public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Eugen.Hristev at microchip.com <Eugen.Hristev@microchip.com>
To: u-boot@lists.denx.de
Subject: [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset
Date: Thu, 7 May 2020 08:55:43 +0000	[thread overview]
Message-ID: <05e52cfa-77b0-d463-00a8-39a42704eaf3@microchip.com> (raw)
In-Reply-To: <87ftcd6mdk.fsf@tarshish>

On 06.05.2020 19:47, Baruch Siach wrote:
> Hi Eugen,
> 
> On Wed, May 06 2020, Eugen Hristev wrote:
>> Because of this commit :
>> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at probe()")
>> at probe time, each eeprom is tested for read at offset 0.
>>
>> The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is
>> used for the lower 0x80 bytes and another i2c slave address is used for the
>> upper 0x80 bytes. Because of this basically the i2c master sees 2 different
>> slaves. We need the upper bytes because we read the unique MAC address from
>> this EEPROM area.
>>
>> However this implies that our slave address will return error on reads
>> from address 0x0 to 0x80.
>>
>> To solve this, implemented a way to figure out from the compatible udevice_id
>> 'data' field which offset we should attempt to read.
>> Added the offset as one byte in the data field (byte number 3).
>> Created two macros that will convert offset to 'data' field and back.
>>
>> The probe function will now read this offset and use it, instead of blindly
>> checking offset 0.
>>
>> This will fix the regression noticed on these EEPROMs since the commit
>> abovementioned that introduces the probe failed issue.
> 
> Judging from the Linux at24 driver that commit appears to break a number
> of other eeprom chips. I suggest to just revert it.
> 
> baruch
> 

Hi,

Please disregard this patch as it's done for u-boot 2020.01 . I did not 
notice driver changed in later versions. I sent a v2 that is done for 
the master u-boot.

Thanks !

>>
>> The offset field can for sure be updated later at a 2byte offset if needed
>> by anyone.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/misc/i2c_eeprom.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
>> index 3755dbf74b..79805cc46e 100644
>> --- a/drivers/misc/i2c_eeprom.c
>> +++ b/drivers/misc/i2c_eeprom.c
>> @@ -11,6 +11,13 @@
>>   #include <i2c.h>
>>   #include <i2c_eeprom.h>
>>
>> +/* These macros are used to encode/decode the starting EEPROM offset into the
>> + * udevice_id structure's data field 3rd byte.
>> + * Lower 2 bytes of the data field are used for pagewidth.
>> + */
>> +#define I2C_EEPROM_OFFSET_TO_DATA(v) ((v) << 16)
>> +#define I2C_EEPROM_DATA_TO_OFFSET(v) ((v) >> 16)
>> +
>>   int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int size)
>>   {
>>        const struct i2c_eeprom_ops *ops = device_get_ops(dev);
>> @@ -85,11 +92,17 @@ static int i2c_eeprom_std_ofdata_to_platdata(struct udevice *dev)
>>
>>   static int i2c_eeprom_std_probe(struct udevice *dev)
>>   {
>> +     u64 data = dev_get_driver_data(dev);
>>        u8 test_byte;
>>        int ret;
>>
>>        /* Verify that the chip is functional */
>> -     ret = i2c_eeprom_read(dev, 0, &test_byte, 1);
>> +     /*
>> +      * Not all eeproms start from offset 0. Valid offset is encoded in
>> +      * upper bits of the data (byte 3).
>> +      */
>> +     ret = i2c_eeprom_read(dev, I2C_EEPROM_DATA_TO_OFFSET(data) & 0xFF,
>> +                           &test_byte, 1);
>>        if (ret)
>>                return -ENODEV;
>>
>> @@ -105,7 +118,8 @@ static const struct udevice_id i2c_eeprom_std_ids[] = {
>>        { .compatible = "atmel,24c08", .data = 4 },
>>        { .compatible = "atmel,24c08a", .data = 4 },
>>        { .compatible = "atmel,24c16a", .data = 4 },
>> -     { .compatible = "atmel,24mac402", .data = 4 },
>> +     { .compatible = "atmel,24mac402",
>> +             .data = (4 | I2C_EEPROM_OFFSET_TO_DATA(0x80))},
>>        { .compatible = "atmel,24c32", .data = 5 },
>>        { .compatible = "atmel,24c64", .data = 5 },
>>        { .compatible = "atmel,24c128", .data = 6 },
> 
> 
> --
>                                                       ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>     - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
> 

  reply	other threads:[~2020-05-07  8:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-06 16:08 [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset Eugen Hristev
2020-05-06 16:47 ` Baruch Siach
2020-05-07  8:55   ` Eugen.Hristev at microchip.com [this message]
2020-05-07 10:00   ` Heiko Schocher
2020-05-07 10:11     ` Eugen.Hristev at microchip.com

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05e52cfa-77b0-d463-00a8-39a42704eaf3@microchip.com \
    --to=eugen.hristev@microchip.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox