[PATCH] board: ti: common: board_detect: Fix EEPROM read quirk

[Nishanth Menon]

On 19:05-20220823, Matwey V. Kornilov wrote:
> There are three different kinds of EEPROM possibly present on boards.
>   1. 1byte address. For those we should avoid 2byte address in order
>      not to rewrite the data. Second byte of the address can potentially
>      be interpreted as the data to write.
>   2. 2byte address with defined behaviour. When we try to use 1byte
>      address they just return "FF FF FF FF ... FF"
>   3. 2byte address with undefined behaviour (for instance, 24LC32AI).
>      When we try to use 1byte address, then their internal read
>      pointer is changed to some value. Subsequential reads may be
>      broken.
> 
> To gracefully handle both case #1 and case #3 we read all required
> data from EEPROM at once (about 80 bytes). So either all the data is
> valid or we fallback to 2byte address.

I would suggest to add a note that this was measured as adding extra
time in startup time.

With that:
Acked-by: Nishanth Menon <nm at ti.com>
> 
> Cc: Nishanth Menon <nm at ti.com>
> Fixes: a58147c2dbbf ("board: ti: common: board_detect: Do 1byte address checks first.")
> Reference: https://lore.kernel.org/all/CAJs94Ebdd4foOjhGFu9Bop0v=B1US9neDLxfhgcY23ukgLzFOQ@mail.gmail.com/
> Signed-off-by: Matwey V. Kornilov <matwey.kornilov at gmail.com>
> ---
>  board/ti/common/board_detect.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index ed34991377..fdf83fcfb0 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -86,7 +86,6 @@ __weak void gpi2c_init(void)
>  static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  					    u32 header, u32 size, uint8_t *ep)
>  {
> -	u32 hdr_read = 0xdeadbeef;
>  	int rc;
>  
>  #if CONFIG_IS_ENABLED(DM_I2C)
> @@ -113,10 +112,10 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  	 * We must allow for fall through to check the data if 2 byte
>  	 * addressing works
>  	 */
> -	(void)dm_i2c_read(dev, 0, (uint8_t *)&hdr_read, 4);
> +	(void)dm_i2c_read(dev, 0, ep, size);
>  
>  	/* Corrupted data??? */
> -	if (hdr_read != header) {
> +	if (*((u32 *)ep) != header) {
>  		/*
>  		 * read the eeprom header using i2c again, but use only a
>  		 * 2 byte address (some newer boards need this..)
> @@ -125,16 +124,12 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  		if (rc)
>  			return rc;
>  
> -		rc = dm_i2c_read(dev, 0, (uint8_t *)&hdr_read, 4);
> +		rc = dm_i2c_read(dev, 0, ep, size);
>  		if (rc)
>  			return rc;
>  	}
> -	if (hdr_read != header)
> +	if (*((u32 *)ep) != header)
>  		return -1;
> -
> -	rc = dm_i2c_read(dev, 0, ep, size);
> -	if (rc)
> -		return rc;
>  #else
>  	u32 byte;
>  
> @@ -154,26 +149,21 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  	 * We must allow for fall through to check the data if 2 byte
>  	 * addressing works
>  	 */
> -	(void)i2c_read(dev_addr, 0x0, byte, (uint8_t *)&hdr_read, 4);
> +	(void)i2c_read(dev_addr, 0x0, byte, ep, size);
>  
>  	/* Corrupted data??? */
> -	if (hdr_read != header) {
> +	if (*((u32 *)ep) != header) {
>  		/*
>  		 * read the eeprom header using i2c again, but use only a
>  		 * 2 byte address (some newer boards need this..)
>  		 */
>  		byte = 2;
> -		rc = i2c_read(dev_addr, 0x0, byte, (uint8_t *)&hdr_read,
> -			      4);
> +		rc = i2c_read(dev_addr, 0x0, byte, ep, size);
>  		if (rc)
>  			return rc;
>  	}
> -	if (hdr_read != header)
> +	if (*((u32 *)ep) != header)
>  		return -1;
> -
> -	rc = i2c_read(dev_addr, 0x0, byte, ep, size);
> -	if (rc)
> -		return rc;
>  #endif
>  	return 0;
>  }
> -- 
> 2.26.2
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D