If the switch is reset during active EEPROM transactions, as in
just after an SoC reset after power up, the I2C bus transaction
may be cut short leaving the EEPROM internal I2C state machine
in the wrong state. When the switch is reset again, the bad
state machine state may result in data being read from the wrong
memory location causing the switch to enter unexpect mode
rendering it inoperational.
Fixes: 8abbffd27ced ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
Signed-off-by: Alfred Lee <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c7d51a539451..7af2f08a62f1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3034,6 +3034,14 @@ static void mv88e6xxx_hardware_reset(struct mv88e6xxx_chip *chip)
/* If there is a GPIO connected to the reset pin, toggle it */
if (gpiod) {
+ /* If the switch has just been reset and not yet completed
+ * loading EEPROM, the reset may interrupt the I2C transaction
+ * mid-byte, causing the first EEPROM read after the reset
+ * from the wrong location resulting in the switch booting
+ * to wrong mode and inoperable.
+ */
+ mv88e6xxx_g1_wait_eeprom_done(chip);
+
gpiod_set_value_cansleep(gpiod, 1);
usleep_range(10000, 20000);
gpiod_set_value_cansleep(gpiod, 0);
--
2.41.0
Hi Alfred,
On Fri, Aug 11, 2023 at 04:28:32PM -0700, Alfred Lee wrote:
> If the switch is reset during active EEPROM transactions, as in
> just after an SoC reset after power up, the I2C bus transaction
> may be cut short leaving the EEPROM internal I2C state machine
> in the wrong state. When the switch is reset again, the bad
> state machine state may result in data being read from the wrong
> memory location causing the switch to enter unexpect mode
> rendering it inoperational.
>
> Fixes: 8abbffd27ced ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
> Signed-off-by: Alfred Lee <[email protected]>
> ---
I don't think you understand the meaning of the Fixes: tag. It is
supposed to point to the first commit where the issue being fixed
started being visible. But git show 8abbffd27ced shows:
commit 8abbffd27cedd0f89f69e5ee2ff6841ea01511eb
Author: Arseniy Krasnov <[email protected]>
Date: Tue Jan 10 10:18:32 2023 +0000
test/vsock: vsock_perf utility
This adds utility to check vsock rx/tx performance.
Usage as sender:
./vsock_perf --sender <cid> --port <port> --bytes <bytes to send>
Usage as receiver:
./vsock_perf --port <port> --rcvlowat <SO_RCVLOWAT>
Signed-off-by: Arseniy Krasnov <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
Signed-off-by: Paolo Abeni <[email protected]>
So:
1 - that has nothing to do with mv88e6xxx, so it cannot have introduced
the issue here
2 - there is a mismatch between the blamed commit sha1sum and the blamed
commit message. In fact, the blamed commit message is _your_ commit
message, which is not correct.
On Fri, Aug 11, 2023 at 04:28:32PM -0700, Alfred Lee wrote:
> If the switch is reset during active EEPROM transactions, as in
> just after an SoC reset after power up, the I2C bus transaction
> may be cut short leaving the EEPROM internal I2C state machine
> in the wrong state. When the switch is reset again, the bad
> state machine state may result in data being read from the wrong
> memory location causing the switch to enter unexpect mode
nit: unexpect -> unexpected
> rendering it inoperational.
>
> Fixes: 8abbffd27ced ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
> Signed-off-by: Alfred Lee <[email protected]>
On Sun, Aug 13, 2023 at 01:58:04PM +0300, Vladimir Oltean wrote:
> Hi Alfred,
>
> On Fri, Aug 11, 2023 at 04:28:32PM -0700, Alfred Lee wrote:
> > If the switch is reset during active EEPROM transactions, as in
> > just after an SoC reset after power up, the I2C bus transaction
> > may be cut short leaving the EEPROM internal I2C state machine
> > in the wrong state. When the switch is reset again, the bad
> > state machine state may result in data being read from the wrong
> > memory location causing the switch to enter unexpect mode
> > rendering it inoperational.
> >
> > Fixes: 8abbffd27ced ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
> > Signed-off-by: Alfred Lee <[email protected]>
> > ---
>
> I don't think you understand the meaning of the Fixes: tag.
The subject looks correct, but the hash is wrong. The correct hash is
a3dcb3e7e70c72a68a79b30fc3a3adad5612731c.
Fixes: a3dcb3e7e70c ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
Andrew
On Sun, Aug 13, 2023 at 05:09:14PM +0200, Andrew Lunn wrote:
> On Sun, Aug 13, 2023 at 01:58:04PM +0300, Vladimir Oltean wrote:
> > Hi Alfred,
> >
> > On Fri, Aug 11, 2023 at 04:28:32PM -0700, Alfred Lee wrote:
> > > If the switch is reset during active EEPROM transactions, as in
> > > just after an SoC reset after power up, the I2C bus transaction
> > > may be cut short leaving the EEPROM internal I2C state machine
> > > in the wrong state. When the switch is reset again, the bad
> > > state machine state may result in data being read from the wrong
> > > memory location causing the switch to enter unexpect mode
> > > rendering it inoperational.
> > >
> > > Fixes: 8abbffd27ced ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
> > > Signed-off-by: Alfred Lee <[email protected]>
> > > ---
> >
> > I don't think you understand the meaning of the Fixes: tag.
>
> The subject looks correct, but the hash is wrong. The correct hash is
> a3dcb3e7e70c72a68a79b30fc3a3adad5612731c.
>
> Fixes: a3dcb3e7e70c ("net: dsa: mv88e6xxx: Wait for EEPROM done after HW reset")
>
> Andrew
Ok. I didn't notice the "before" vs "after". But the patch still needs
to be resubmitted with the right Fixes: tag.