On Tue, Sep 03, 2024 at 08:57:24AM +0200, Stefan Roese wrote: > On 8/29/24 10:08, Marek Behún wrote: > > Use the i2c_eeprom miscellaneous driver for reading Turris Omnia EEPROM > > in U-Boot proper. Keep using dm_i2c_read() in SPL build, since adding > > the i2c_eeprom driver to SPL build increases the image by 1.5 KiB. > > > > Signed-off-by: Marek Behún <ka...@kernel.org> > > --- > > arch/arm/mach-mvebu/Kconfig | 1 + > > board/CZ.NIC/turris_omnia/turris_omnia.c | 9 +++++++-- > > configs/turris_omnia_defconfig | 1 - > > 3 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > > index 4a8328760eb..c1a1a333e6c 100644 > > --- a/arch/arm/mach-mvebu/Kconfig > > +++ b/arch/arm/mach-mvebu/Kconfig > > @@ -149,6 +149,7 @@ config TARGET_TURRIS_OMNIA > > select SPL_SYS_MALLOC_SIMPLE > > select SYS_I2C_MVTWSI > > select ATSHA204A > > + select I2C_EEPROM > > select ARMADA_38X_SUPPORT_OLD_DDR3_TRAINING > > config TARGET_TURRIS_MOX > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c > > b/board/CZ.NIC/turris_omnia/turris_omnia.c > > index 392df53a6d8..46f20f05c05 100644 > > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > > @@ -23,6 +23,7 @@ > > #include <dt-bindings/gpio/gpio.h> > > #include <fdt_support.h> > > #include <hexdump.h> > > +#include <i2c_eeprom.h> > > #include <time.h> > > #include <turris-omnia-mcu-interface.h> > > #include <linux/bitops.h> > > @@ -473,9 +474,13 @@ static bool omnia_read_eeprom(struct omnia_eeprom *oep) > > if (!eeprom) > > return false; > > - ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep)); > > + if (IS_ENABLED(CONFIG_SPL_BUILD)) > > + ret = dm_i2c_read(eeprom, 0, (void *)oep, sizeof(*oep)); > > + else > > + ret = i2c_eeprom_read(eeprom, 0, (void *)oep, sizeof(*oep)); > > So whats the advantage using i2c_eeprom_read() here in U-Boot proper? > The only thing that stands out to me is that the code get more complex > because of the SPL/non-SPL distinction.
The reasons are as follows: - it's "more correct" semantically - the i2c_eeprom driver was already enabled for U-Boot proper in defconfig (with this commit selected by the board), because of the `eeprom` commmand, so I thought "why not use it?" (looking at the code now, it goes i2c_eeprom_read() dereference of i2c_eeprom_ops->read method i2c_eeprom_std_read() dm_i2c_read() so the complication is trivial, IMO) - the following commit adds code that writes the eeprom, and this cannot be done with simple dm_i2c_write(), because each eeprom page has to be written by another transaction, which is what i2c_eeprom_write() does. So I knew that I was going to add i2c_eeprom_write(), and I thought that it would seem weird writing with i2c_eeprom_write() but reading with dm_i2c_read(). Marek