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: <CAOMZO5AQ6VJi7Qhz4B0VQk5f2_R0bXB_RqipgGMBz9+vtHBMmg@mail.gmail.com>
Date: Fri, 15 Sep 2023 11:13:36 -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 Fri, Sep 15, 2023 at 10:08 AM Andrew Lunn <andrew@...n.ch> wrote:

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

Thanks for confirming with the datasheet.

> Is you device held in reset before the driver loads? As i said, the

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);
       }