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]

Date: Fri, 15 Sep 2023 20:23:50 +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

> Back to mv88e6xxx_g1_read(): should we check whether the EEPROM is
> present and bail out if absent?
> 
> Did a quick hack just to show the idea. Please let me know what you think.
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/global1.c
> b/drivers/net/dsa/mv88e6xxx/global1.c
> index 5848112036b0..0e8b8667e897 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1.c
> +++ b/drivers/net/dsa/mv88e6xxx/global1.c
> @@ -12,6 +12,7 @@
> 
>  #include "chip.h"
>  #include "global1.h"
> +#include "global2.h"
> 
>  int mv88e6xxx_g1_read(struct mv88e6xxx_chip *chip, int reg, u16 *val)
>  {
> @@ -80,6 +81,13 @@ void mv88e6xxx_g1_wait_eeprom_done(struct
> mv88e6xxx_chip *chip)
>         const unsigned long timeout = jiffies + 1 * HZ;
>         u16 val;
>         int err;
> +       u16 data;
> +       u8 addr = 0;
> +
> +       err = mv88e6xxx_g2_eeprom_read16(chip, addr, &data);

The problem with this is that the way to read the contests of the
EEPROM depend on the switch family.

linux/drivers/net/dsa/mv88e6xxx$ grep \.get_eeprom chip.c
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
	.get_eeprom = mv88e6xxx_g2_get_eeprom8,


> +       /* Test whether EEPROM is present and bail out if absent */
> +       if (data == 0xffff)
> +               return;

And how do you know the EEPROM does not in fact contain 0xffff?

What i found interesting in the datasheet for the 6352:

     The EEInt indicates the processing of the EEPROM contents is
     complete and the I/O registers are now available for CPU
     access. A CPU can use this interrupt to know it is OK to start
     accessing the device’s registers. The EEInt will assert the
     device’s INT pin even if not EEPROM is attached unless the EEPROM
     changes the contents of the EEIntMast register (Global 1, offset
     0x04) or if the Test SW_MODE has been configured (see 8888E6352,
     88E6240, 88E6176, and 88E6172 Functional Specification Datasheet,
     Part 1 of 3: Overview, Pinout, Applications, Mechanical and
     Electrical Specifications for details).  The StatsDone, VTUDone
     and ATUDone interrupts de-assert after the Switch Globa

So i would expect that EEInt is set when there is no EEPROM.

What strapping do you have for SW_MODE? Is the switch actually in
standalone mode?

	   Andrew