[PATCH] eeprom: at24: Change nvmem stride to 1

[PATCH] eeprom: at24: Change nvmem stride to 1

[David Lechner]

This changes the nvmem stride to 1.

I am trying to use the nvram consumer apis to read a MAC address from an
eeprom that has an offset that is not a multiple of 4, which causes an
error currently.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/misc/eeprom/at24.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 305a7a4..f70d14c 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -880,7 +880,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	at24->nvmem_config.reg_read = at24_read;
 	at24->nvmem_config.reg_write = at24_write;
 	at24->nvmem_config.priv = at24;
-	at24->nvmem_config.stride = 4;
+	at24->nvmem_config.stride = 1;
 	at24->nvmem_config.word_size = 1;
 	at24->nvmem_config.size = chip.byte_len;
 
-- 
2.7.4

Re: [PATCH] eeprom: at24: Change nvmem stride to 1

[Bartosz Golaszewski]

2017-12-04 2:54 GMT+01:00 David Lechner <david@lechnology.com>:
> This changes the nvmem stride to 1.
>
> I am trying to use the nvram consumer apis to read a MAC address from an
> eeprom that has an offset that is not a multiple of 4, which causes an
> error currently.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/misc/eeprom/at24.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 305a7a4..f70d14c 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -880,7 +880,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>         at24->nvmem_config.reg_read = at24_read;
>         at24->nvmem_config.reg_write = at24_write;
>         at24->nvmem_config.priv = at24;
> -       at24->nvmem_config.stride = 4;
> +       at24->nvmem_config.stride = 1;
>         at24->nvmem_config.word_size = 1;
>         at24->nvmem_config.size = chip.byte_len;
>
> --
> 2.7.4
>

I can't find any documentation on what the stride config option does
in nvmem, but looking at the code it's only used for alignment checks
in nvmem core, so this patch should be ok. Still: I'm wondering if it
shouldn't depend on the size of the eeprom or if we shouldn't make the
chip you're using a special case.

@David: what is the chip you're using? Is it an at24mac402 by any
chance? Were you affected by the read problem we fixed recently[1][2]
in at24?

@Srinivas: any comments on that?

Thanks,
Bartosz

[1] http://patchwork.ozlabs.org/patch/841852/
[2] http://patchwork.ozlabs.org/patch/841876/

Re: [PATCH] eeprom: at24: Change nvmem stride to 1

[David Lechner]

On 12/04/2017 11:44 AM, Bartosz Golaszewski wrote:
> I can't find any documentation on what the stride config option does
> in nvmem, but looking at the code it's only used for alignment checks
> in nvmem core, so this patch should be ok. Still: I'm wondering if it
> shouldn't depend on the size of the eeprom or if we shouldn't make the
> chip you're using a special case.

I am just guessing on the usage too, but I assume it is for memory 
alignment for other types of nvmem devices that only read words (2, 4 or 
8 bytes) at a time. I don't see anything in the at24 code that says we 
can't read any arbitrary starting I2C register, so I don't think we 
should have any problems.

> 
> @David: what is the chip you're using?

Microchip 24FC128

> Is it an at24mac402 by any
> chance? Were you affected by the read problem we fixed recently[1][2]
> in at24?

No and no.

> 
> @Srinivas: any comments on that?
> 
> Thanks,
> Bartosz
> 
> [1] http://patchwork.ozlabs.org/patch/841852/
> [2] http://patchwork.ozlabs.org/patch/841876/
> 

Re: [PATCH] eeprom: at24: Change nvmem stride to 1

[Srinivas Kandagatla]

On 04/12/17 17:44, Bartosz Golaszewski wrote:
>>          at24->nvmem_config.priv = at24;
>> -       at24->nvmem_config.stride = 4;
>> +       at24->nvmem_config.stride = 1;
>>          at24->nvmem_config.word_size = 1;
>>          at24->nvmem_config.size = chip.byte_len;
>>
>> --
>> 2.7.4
>>
> I can't find any documentation on what the stride config option does
> in nvmem, but looking at the code it's only used for alignment checks
> in nvmem core, so this patch should be ok. Still: I'm wondering if it
> shouldn't depend on the size of the eeprom or if we shouldn't make the
> chip you're using a special case.
> 
> @David: what is the chip you're using? Is it an at24mac402 by any
> chance? Were you affected by the read problem we fixed recently[1][2]
> in at24?
> 
> @Srinivas: any comments on that?
Stride is there to enforce address alignment. As long as there is no 
issue on addresses aligned to 1 byte on at24 I do not see any issue with 
the patch.

Thanks,
srini

Re: [PATCH] eeprom: at24: Change nvmem stride to 1

[Bartosz Golaszewski]

2017-12-04 2:54 GMT+01:00 David Lechner <david@lechnology.com>:
> This changes the nvmem stride to 1.
>
> I am trying to use the nvram consumer apis to read a MAC address from an
> eeprom that has an offset that is not a multiple of 4, which causes an
> error currently.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/misc/eeprom/at24.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 305a7a4..f70d14c 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -880,7 +880,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>         at24->nvmem_config.reg_read = at24_read;
>         at24->nvmem_config.reg_write = at24_write;
>         at24->nvmem_config.priv = at24;
> -       at24->nvmem_config.stride = 4;
> +       at24->nvmem_config.stride = 1;
>         at24->nvmem_config.word_size = 1;
>         at24->nvmem_config.size = chip.byte_len;
>
> --
> 2.7.4
>

Applied, thanks!

Bartosz