Re: [PATCH v2 2/4] nvmem: NXP LPC18xx EEPROM memory NVMEM driver

[Ariel D'Alessandro <ariel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>]

Stefan,

El 17/11/15 a las 07:01, Stefan Wahren escribió:
> Hi Ariel,
> 
>> Ariel D'Alessandro <ariel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org> hat am 16. November 2015 um
>> 16:29 geschrieben:
>>
>>
>> Hi Stefan,
>>
>> Sorry for the delay.
>>
>> El 03/11/15 a las 05:20, Stefan Wahren escribió:
>>> Hi Ariel,
>>>
>>> Am 19.10.2015 um 19:32 schrieb Ariel D'Alessandro:
>>>> This commit adds support for NXP LPC18xx EEPROM memory found in NXP
>>>> LPC185x/3x and LPC435x/3x/2x/1x devices.
>>>>
>>>> EEPROM size is 16384 bytes and it can be entirely read and
>>>> written/erased with 1 word (4 bytes) granularity. The last page
>>>> (128 bytes) contains the EEPROM initialization data and is not writable.
>>>>
>>>> Erase/program time is less than 3ms. The EEPROM device requires a
>>>> ~1500 kHz clock (min 800 kHz, max 1600 kHz) that is generated dividing
>>>> the system bus clock by the division factor, contained in the divider
>>>> register (minus 1 encoded).
>>>>
>>>> Signed-off-by: Ariel D'Alessandro <ariel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
>>>> ---
>>>> drivers/nvmem/Kconfig | 9 ++
>>>> drivers/nvmem/Makefile | 2 +
>>>> drivers/nvmem/lpc18xx_eeprom.c | 266
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 277 insertions(+)
>>>> create mode 100644 drivers/nvmem/lpc18xx_eeprom.c
>>>> [...]
>>>> +
>>>> +static int lpc18xx_eeprom_gather_write(void *context, const void *reg,
>>>> + size_t reg_size, const void *val,
>>>> + size_t val_size)
>>>> +{
>>>> + struct lpc18xx_eeprom_dev *eeprom = context;
>>>> + unsigned int offset = *(u32 *)reg;
>>>> +
>>>> + /* 3 ms of erase/program time between each writing */
>>>> + while (val_size) {
>>>> + writel(*(u32 *)val, eeprom->mem_base + offset);
>>>> + usleep_range(3000, 4000);
>>>
>>> i think it would be good to verify that the EEPROM write operation has
>>> really finished.
>>
>> I'm not sure what are you proposing. Why could the write operation not
>> finish?
> 
> it's always good to keep in sync with the hardware. Here is an extract of
> chapter 
> "46.6.2.1 Writing and erase/programming" from the datasheet [1]:
> 
>   During programming, the EEPROM is not available for other operations. To
> prevent
>   undesired loss in performance which would be caused by stalling the bus, the
> EEPROM
>   instead generates an error for AHB read/writes and APB writes when programming
> is
>   busy. In order to prevent the error response, the program operation finished
> interrupt
>   can be enabled or the interrupt status bit can be polled.
> 
> Please blame me if it doesn't apply.
> 
> It's only a suggestion: How about checking the interrupt status bit for
> END_OF_PROG after the 3 ms sleep?

Yeah, I think that's a good idea, pretty simple and it will ensure
(although 3ms should be enough) erase/program operation has effectively
finished.

Thanks for the explanation and suggestion. I'm submitting patchset v4
with this.

-- 
Ariel D'Alessandro, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html