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: <96f69525-53e1-f77e-e0f4-3146c2310551@denx.de>
Date:   Thu, 17 Mar 2022 07:43:52 +0100
From:   Stefan Roese <sr@...x.de>
To:     Marek BehĂșn <kabel@...nel.org>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH] net: phy: marvell: Add errata section 5.1 for Alaska PHY

On 3/15/22 17:09, Marek BehĂșn wrote:
> On Tue, 15 Mar 2022 08:48:27 +0100
> Stefan Roese <sr@...x.de> wrote:
> 
>> From: Leszek Polak <lpolak@...i.de>
>>
>> As per Errata Section 5.1, if EEE is intended to be used, some register
>> writes must be done once after every hardware reset. This patch now adds
>> the necessary register writes as listed in the Marvell errata.
>>
>> Without this fix we experience ethernet problems on some of our boards
>> equipped with a new version of this ethernet PHY (different supplier).
>>
>> The fix applies to Marvell Alaska 88E1510/88E1518/88E1512/88E1514
>> Rev. A0.
>>
>> Signed-off-by: Leszek Polak <lpolak@...i.de>
>> Signed-off-by: Stefan Roese <sr@...x.de>
>> Cc: Andrew Lunn <andrew@...n.ch>
>> Cc: Heiner Kallweit <hkallweit1@...il.com>
>> Cc: Russell King <linux@...linux.org.uk>
>> Cc: David S. Miller <davem@...emloft.net>
>> ---
>>   drivers/net/phy/marvell.c | 42 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
>> index 2429db614b59..0f4a3ab4a415 100644
>> --- a/drivers/net/phy/marvell.c
>> +++ b/drivers/net/phy/marvell.c
>> @@ -1179,6 +1179,48 @@ static int m88e1510_config_init(struct phy_device *phydev)
>>   {
>>   	int err;
>>   
>> +	/* As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/
>> +	 * 88E1514 Rev A0, Errata Section 5.1:
>> +	 * If EEE is intended to be used, the following register writes
>> +	 * must be done once after every hardware reset.
>> +	 */
>> +	err = marvell_set_page(phydev, 0x00FF);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 17, 0x214B);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 16, 0x2144);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 17, 0x0C28);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 16, 0x2146);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 17, 0xB233);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 16, 0x214D);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 17, 0xCC0C);
>> +	if (err < 0)
>> +		return err;
>> +	err = phy_write(phydev, 16, 0x2159);
>> +	if (err < 0)
>> +		return err;
> 
> Can you please create a static const struct and then do this in a for
> cycle? Somethign like this
> 
> static const struct { u16 reg17, reg16; } errata_vals = {
>    { 0x214B, 0x2144 }, ...
> };
> 
> for (i = 0; i < ARRAY_SIZE(errata_vals); ++i) {
>    err = phy_write(phydev, 17, errata_vals[i].reg17);
>    if (err)
>      return err;
>    err = phy_write(phydev, 16, errata_vals[i].reg16);
>    if (err)
>      return err;
> }

I could do this, sure. But frankly I'm not so sure, if this really
improves the code much. This list will most likely never be extended.
And in this current version, it's easier to compare the values written
with the one described in the errata.

But again, I have no hard feeling here. If it's the general option to
move to the "loop version", then I'll gladly send an updated patch
version.

Thanks,
Stefan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ