lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <71d4a673-73b6-4ebe-a669-de3ae6c9af5f@gmx.net>
Date: Wed, 8 May 2024 21:30:25 +0200
From: Hans-Frieder Vogt <hfdevel@....net>
To: Andrew Lunn <andrew@...n.ch>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
 horms@...nel.org, kuba@...nel.org, jiri@...nulli.us, pabeni@...hat.com
Subject: Re: [PATCH net-next v5 5/6] net: tn40xx: add mdio bus support

On 08.05.2024 20.25, Andrew Lunn wrote:
>>> +    writel(((1 << 15) | i), regs + TN40_REG_MDIO_CMD);
>> similarly here:
>>
>> writel((MDIO_PHY_ID_C45 | i), regs + TN40_REG_MDIO_CMD);
> This one i don't agree with. It happens to work, but there is no
> reason to think the hardware has been designed around how Linux
> combines the different parts of a C45 address into one word, using the
> top bit to indicate it is actually a C45 address, not a C22.
>
> I would much prefer a TN40_ define is added for this bit.
OK, yes, very valid point.
>
>>> +    writel(((device & 0x1F) | ((port & 0x1F) << 5)),
>> and also here, similarly:
>>
>> writel((device & MDIO_PHY_ID_DEVAD) | ((port << 5) & MDIO_PHY_ID_PRTAD),
> Similarly here, this happens to work, but that is just because the
> hardware matches a software construct Linux uses. It would be better
> to add TN40_ macros to describe the hardware.
agreed, I assume I just interpreted too much into the constants.
>
>     Andrew

Thanks!
Hans


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ