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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <333e23ae-fe75-48e1-a2fb-65b127ec9b3e@lunn.ch>
Date: Fri, 15 Sep 2023 15:08:33 +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

On Thu, Sep 14, 2023 at 09:40:05PM -0300, Fabio Estevam wrote:
> 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?

It is a bit ambiguous. RESETn is both an input and an output. It says
this about input for the For the 6352:

     As in input, when RESETn is driven low by an external device,
     this device will then driver RESETn low as an output for 8 to
     14ms (10ms typically). In this mode RESETn can be used to
     debounce a hardware reset switch.

So i would say it needs to be low long enough not to be a glitch, but
can be short.

> > 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,

Is you device held in reset before the driver loads? As i said, the
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.

   Andrew