netdev - Re: mv88e6xxx: Timeout waiting for EEPROM done

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: <5b5f24f4-f98f-4ea1-a4a3-f49c8385559d@lunn.ch>
Date: Fri, 15 Sep 2023 16:34:44 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Fabio Estevam <festevam@...il.com>
Cc: Vladimir Oltean <olteanv@...il.com>, l00g33k@...il.com,
	netdev <netdev@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>,
	sashal@...nel.org
Subject: Re: mv88e6xxx: Timeout waiting for EEPROM done

> Just checked with a scope here and no, the reset pin is not held in
> reset before the driver loads.
> 
> > aim of this code is not to actually reset the switch, but to ensure it
> > is taken out of reset if it was being held in reset. And if it was
> > being held in reset, i would expect that to be for a long time, at
> > least the current Linux boot time.
> 
> That's a point I am concerned about: why don't we follow the datasheet
> with respect to taking the reset pin out of reset?
> 
> Isn't the sequence I used below better suited as it follows the
> datasheet by guaranteeing the 10ms at a low level?
> 
>        chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>        ....
>        if (chip->reset) {
>                 usleep_range(10000, 20000);
>                 gpiod_set_value(chip->reset, 0);
>                 usleep_range(10000, 20000);
>        }

As i said, we do a hardware reset later. All we are trying to do at
this stage is probe the device, does it exist on the bus, and what ID
value does it have. I want to avoid the overhead of doing a reset now,
and then doing it again later.

So you say your device is not held in reset. So ideally there should
not be a change in the GPIO with devm_gpiod_get_optional(), and the
delay afterwards is pointless in your case.

When the device is held in reset, devm_gpiod_get_optional() should
release it from reset, and then we need a delay because experience has
shown the device will not actually respond on the MDIO bus for a short
while.

	Andrew