Re: [RFC PATCH 2/3] drivers: misc: eeprom: at24: support reading mac eeprom from different addresses

[m18063 <Claudiu.Beznea@microchip.com>]

On 29.06.2017 18:32, David Lechner wrote:
> On 06/29/2017 06:39 AM, Claudiu Beznea wrote:
>> Add support for reading from different offsets of EEPROM.
>> The offset is initialized via device tree. If nothing is
>> given as input the old value, 0x90, is used. In this way
>> the driver could be used as generic driver for different
>> vendor memories by only changing the reading offset via
>> device tree.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>   drivers/misc/eeprom/at24.c         | 5 ++++-
>>   include/linux/platform_data/at24.h | 1 +
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
>> index 764ff5df..53c5fb8 100644
>> --- a/drivers/misc/eeprom/at24.c
>> +++ b/drivers/misc/eeprom/at24.c
>> @@ -365,7 +365,7 @@ static ssize_t at24_eeprom_read_mac(struct at24_data *at24, char *buf,
>>       memset(msg, 0, sizeof(msg));
>>       msg[0].addr = client->addr;
>>       msg[0].buf = addrbuf;
>> -    addrbuf[0] = 0x90 + offset;
>> +    addrbuf[0] = at24->chip.offset + offset;
>>       msg[0].len = 1;
>>       msg[1].addr = client->addr;
>>       msg[1].flags = I2C_M_RD;
>> @@ -581,6 +581,9 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
>>            */
>>           chip->page_size = 1;
>>       }
>> +    err = device_property_read_u8(dev, "start-offset", &chip->offset);
> 
> Why use u8 here? I have an at24 EEPROM that uses 16-bit addressing where the MAC address is stored at 0x3F06. Seems like it would be better to just use u32, then you don't have to mess with /bits/ in the device tree binding.
Agree! I will use u32 instead. It is better that way.

Thanks,
Claudiu
> 
>> +    if (err)
>> +        chip->offset = 0x90;
>>   }
>>     static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>> diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
>> index 271a4e2..d55c454 100644
>> --- a/include/linux/platform_data/at24.h
>> +++ b/include/linux/platform_data/at24.h
>> @@ -50,6 +50,7 @@ struct at24_platform_data {
>>   #define AT24_FLAG_TAKE8ADDR    BIT(4)    /* take always 8 addresses (24c00) */
>>   #define AT24_FLAG_SERIAL    BIT(3)    /* factory-programmed serial number */
>>   #define AT24_FLAG_MAC        BIT(2)    /* factory-programmed mac address */
>> +    u8        offset;
>>         void        (*setup)(struct nvmem_device *nvmem, void *context);
>>       void        *context;
>>
>