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: <CAOMZO5BDWFtYu5iae7Gk-bF6Q6d1TV4dYZ=GtW_L_-CV8HapBg@mail.gmail.com>
Date: Thu, 14 Sep 2023 21:40:05 -0300
From: Fabio Estevam <festevam@...il.com>
To: Andrew Lunn <andrew@...n.ch>
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

Hi Andrew,

On Thu, Sep 14, 2023 at 6:38 PM Andrew Lunn <andrew@...n.ch> wrote:

> Unfortunately, none of my boards appear to have the reset pin wired to
> a GPIO.
>
> The 6352 data sheet documents the reset pin is active low. So i can
> understand using GPIO_ACTIVE_LOW.

What does the datasheet say about the minimal duration for the reset
pin being asserted?

Unfortunately, I don't have access to it.

Usually, the datasheets specify a minimum duration for the reset to be
at logic level low.

For example:
https://www.marvell.com/content/dam/marvell/en/public-collateral/phys-transceivers/marvell-phys-transceivers-fast-ethernet-88e301x-functional-specifications.pdf

(4.6.1 Reset and Configuration Timing - says the reset pin should be
low for at least 10 ms).

> In probe, we want to ensure the switch is taken out of reset, if the
> bootloader etc has left it in reset. We don't actually perform a reset
> here. That is done later. So we want the pin to have a high value. I

My concern is that the implemented method to bring the reset pin out
of reset may violate the datasheet by not waiting the required amount
of time.

Someone who has access to the datasheet may confirm, please.

> know gpiod_set_value() takes into account if the DT node has
> GPIO_ACTIVE_LOW. So setting a value of 0 disables it, which means it
> goes high. This is what we want. But the intention of the code is that
> the actual devm_gpiod_get_optional() should set the GPIO to output a
> high. But does devm_gpiod_get_optional() do the same mapping as
> gpiod_set_value()? gpiod_direction_output() documents says:

Yes, this is my understanding.

>  * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
>  * be called safely on it. The initial value of the output must be specified
>  * as the logical value of the GPIO, i.e. taking its ACTIVE_LOW status into
>  * account.
>
> I don't know how to interpret this.
>
> Is the first change on its own sufficient to make it work? As i said,

No, it is not. On my tests, I needed to force the reset GPIO to be low
for a certain duration,