drivers: eeprom: add API for EEPROM devices

[henrikbrixandersen]

Add API for accessing Electrically Erasable Programmable Read-Only Memory (EEPROM) devices.

EEPROMs have an erase block size of 1 byte, a long lifetime, and allows overwriting data on byte-by-byte access.

Signed-off-by: Henrik Brix Andersen [email protected]

[henrikbrixandersen]

The idea behind this is a simple, byte-oriented API for EEPROM devices.

I have been implementing drivers for generic SPI EEPROMs (AT25xxx compatible) and I2C EEPROMs (AT24xxx compatible) under the Zephyr flash API, but it just doesn't feel right.

The EEPROM drivers need to implement the full flash API (What to do on erase? NO-OP? Write 0xFF to "erased" bytes thus affecting lifetime negatively? Something else?) and this type of devices generally aren't fit as backends for the various subsystems in Zephyr currently supporting the flash API as backend (e.g. the settings subsystem) due to assumptions about page layout and erase.

I would like to hear any comments and/or suggestions on how to approach integrating EEPROM devices in as first-class devices in Zephyr.

[Laczen]

@henrikbrixandersen, nice idea to add EEPROM devices support to zephyr. All backend systems for zephyr have been written to use flash devices for storage and they will probably not be an ideal fit for EEPROM devices. But even a non ideal fit would allow these devices to be used:

a. The flash storage solutions try to do everything to avoid erasing pages, so even writing 0xff (I would do this) as a erase operation would not influence lifetime a lot.
b. The flash page size can be selected smaller (e.g. 512 bytes).

Since page erases and page sizes are not EEPROM properties I would separate these properties in a "eeprom_flash" library, so not directly in the EEPROM library. The "glue" between eeprom and flash could be similar to the flash simulator (of course simplified).

This proposed solution would not take full advantage of the EEPROM devices, to do that a storage method for EEPROM devices needs to be created.

[nvlsianpu]

@henrikbrixandersen
Maybe worth to add a flash driver which will be a wrapper onto eeprom API.
Storage system dedicated to eeprom should be designed slight different than for flash.

[nvlsianpu]

EEPROMs typically have an erase block size of 1 byte and a long

have you any example of an EEPROM with different garrulity - If so, need to add API for fetch this.

[pabigot]

It makes sense to do something simpler than the Zephyr flash API, and this seems like a reasonable approach.

We do need devicetree bindings for the EEPROMs. There's an existing atmel,at24 binding in Zephyr under mtd that supports a hand-coded sample tests/drivers/i2c/i2c_slave_api for I2C eeprom. It's ancient and should be replaced by one closer to Linux's, with a companion atmel,25 binding added.

Is it true that all the eeprom variation points can be parameterized in devicetree, so there would be a single implementation of this driver?

It should support multiple instances (though that may be difficult until Zephyr's devicetree becomes more mature), as well as support both I2C and SPI as the back-end. There are some sensor drivers that support both busses that could be used as an example, or we could consider take advantage of the opportunity and provide a generic block back-end that's independent of bus.

[aunsbjerg]

Looks good!

Some EEPROM chip's have dedicated erase commands. Maybe the API could support this by having something like erase(struct *dev, off_t offset, size_t size) method as well?

[henrikbrixandersen]

Looks good!

Thank you for reviewing 👍

Some EEPROM chip's have dedicated erase commands. Maybe the API could support this by having something like erase(struct *dev, off_t offset, size_t size) method as well?

Wouldn't they fit under our existing flash API, then?

[henrikbrixandersen]

It makes sense to do something simpler than the Zephyr flash API, and this seems like a reasonable approach.

Thank you for reviewing 👍

We do need devicetree bindings for the EEPROMs. There's an existing atmel,at24 binding in Zephyr under mtd that supports a hand-coded sample tests/drivers/i2c/i2c_slave_api for I2C eeprom. It's ancient and should be replaced by one closer to Linux's, with a companion atmel,25 binding added.

Yes, I have those bindings here. I will add them for review.

Is it true that all the eeprom variation points can be parameterized in devicetree, so there would be a single implementation of this driver?

Well, yes. But I would prefer having a dedicated eeprom-at24.c and a dedicated eeprom-at25.c driver, since the underlying bus access is what makes up most of the driver. Putting both I2C and SPI support into the same driver does not give any real benefit, I think.

[henrikbrixandersen]

have you any example of an EEPROM with different garrulity - If so, need to add API for fetch this.

No, I haven't come across any. I will reword the sentence to reflect that.

[henrikbrixandersen]

Regarding a flash API adapter layer: Good idea, if anybody has a use for that. Another option could be to implement EEPROM-specific backends for the various subsystems that could use EEPROMs as backends when the need arises.

I would like to keep the first implementation as simple as possible and focus on the hardware-near API.

[Laczen]

@henrikbrixandersen, yes I think you should keep the first implementation as simple as possible and focus on the hardware-near API. An EEPROM has different properties than a flash device and the EEPROM API should reflect this -> so I think your proposal is the correct way.

When you would like to use it for (settings) storage you could define a flash_eeprom API layer, that is just a wrapper to your read/write methods but also defines a virtual page size (e.g 512 bytes) to define a "flash_erase()" and also to allow the storage backends to get a page size. This layer could then be used as a "flash" interface to store all settings in EEPROM without any modifications.

[nvlsianpu]

Some EEPROM chip's have dedicated erase commands.

@aunsbjerg Can you point such a part? Does it men that for such a device need to erase an area before write to it, or it is just a helper feature for increase write speed.

[nvlsianpu]

I would like to keep the first implementation as simple as possible and focus on the hardware-near API.

@henrikbrixandersen Right. My comment was suited for pointing direction which can satisfy flash API client - this can be added in subsequent PR by anyone who needs such a solution.

[aunsbjerg]

Some EEPROM chip's have dedicated erase commands.

@aunsbjerg Can you point such a part? Does it men that for such a device need to erase an area before write to it, or it is just a helper feature for increase write speed.

@nvlsianpu
The Microchip 25LC1024 has chip/page/sector erase commands. They are only helper features for quicker erase operation. The same result could be achieved by manually erasing using write operations.

@henrikbrixandersen
I don't consider an erase() method in the API to be crucial at all, but for those EEPROM devices that do support it, it would be nice to have without having to "hide" an EEPROM behind the flash interface.

[henrikbrixandersen]

@aunsbjerg

I don't consider an erase() method in the API to be crucial at all, but for those EEPROM devices that do support it, it would be nice to have without having to "hide" an EEPROM behind the flash interface.

That leads to another questions: If we have to have an erase() API method in the EEPROM API (which most of the EEPROM device drivers would likely not implement or just provide a dummy method for), would we be better off just "hiding" all EEPROM devices behind the flash API?

Personally, I still believe a dedicated, simple EEPROM API is the way to go. For devices which have the concept of explicit erase, I would prefer to use the flash API.

[pabigot]

The Microchip 25LC1024 has chip/page/sector erase commands. They are only helper features for quicker erase operation. The same result could be achieved by manually erasing using write operations.

That chip implements the full JEDEC API so is already supported by the flash spi_nor driver. My understanding is that this API targets much more limited chips that don't have such erase operations.

I tend to agree with @henrikbrixandersen that there is no need for an erase function in this API. There may be a value in a library function that implements "write sequence S at offset O repeating for L bytes" which can support erase protocols of varying types (write 0xFF, write patterns, etc.).

[nvlsianpu]

@aunsbjerg Agree - with erase API it will be almost flash API. I'm in line with that this should be covered by flash API - eeprom api should be simpler.

[henrikbrixandersen]

I have added a unified driver to AT24 I2C and AT25 SPI EEPROMs. Please re-review as needed.

[henrikbrixandersen]

From a quick grep though the master source tree, the following supported boards already contain an EEPROM of some sorts:

• 96b_avenger96
• b_l072z_lrwan1
• sam_e70_xplained
• stm3210c_eval
• stm32373c_eval
• stm32f072b_disco
• stm32l476g_disco
• udoo_neo_full_m4

Unfortunately, I do not have access to any of these boards, but if somebody else is looking to test the EEPROM subsystem and driver...

[andrewboie]

the new subsystem API, syscalls, shell integrayion, etc all look good to me.

But we're missing tests of this subsystem, just adding a case to build_all doesn't give use any code coverage. It especially makes sense to create tests since we can actually exercise it in CI because you provided a native_posix implementation. Could you please implement some test code under tests/drivers/eeprom/ which exercises all these new subsystem APIs, from user mode for the system calls?

[andrewboie]

the new subsystem API, syscalls, shell integrayion, etc all look good to me.

But we're missing tests of this subsystem, just adding a case to build_all doesn't give use any code coverage. It especially makes sense to create tests since we can actually exercise it in CI because you provided a native_posix implementation. Could you please implement some test code under tests/drivers/eeprom/ which exercises all these new subsystem APIs, from user mode for the system calls? the tests should exercise the full set of return values supported by each API.

[henrikbrixandersen]

@andrewboie Good point, thank you. I have added a test suite. It runs to completion when running without userspace, but fails under userspace. Still trying to figure out what I am doing wrong...