From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Wed, 03 Dec 2014 17:16:52 +0100 Subject: [U-Boot] [PATCH v3 08/10] dm: Add a simple EEPROM driver In-Reply-To: References: <1416855444-32016-1-git-send-email-sjg@chromium.org> <1416855444-32016-9-git-send-email-sjg@chromium.org> <20141201204835.67C9.AA925319@jp.panasonic.com> Message-ID: <547F3774.9020909@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello, On 12/03/2014 04:18 PM, Simon Glass wrote: > Hi Masahiro, > > On 1 December 2014 at 04:48, Masahiro Yamada wrote: >> Hi Simon, >> >> >> >> >> >> On Mon, 24 Nov 2014 11:57:22 -0700 >> Simon Glass wrote: >> >>> --- /dev/null >>> +++ b/drivers/misc/i2c_eeprom.c >>> @@ -0,0 +1,51 @@ >>> +/* >>> + * Copyright (c) 2014 Google, Inc >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +static int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, >>> + int size) >>> +{ >>> + return -ENODEV; >>> +} >>> + >>> +static int i2c_eeprom_write(struct udevice *dev, int offset, >>> + const uint8_t *buf, int size) >>> +{ >>> + return -ENODEV; >>> +} >> >> >> Isn't there any possibility to reach i2c_eeprom_read/i2c_eeprom_write >> handler? > > No it's just a stub for now. > >> >> Maybe, is it better to fallback to i2c_read()/i2c_write() like this? >> >> >> static int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, >> int size) >> { >> return i2c_read(dev, offset, buf, size); >> } >> >> static int i2c_eeprom_write(struct udevice *dev, int offset, const uint8_t *buf, >> int size) >> { >> return i2c_write(dev, offset, buf, size); >> } >> >> > > Sure we can - I haven't implemented this yet, but if we could have a > generic EEPROM driver like this it would be nice. > >> >> Moreover, can we read data from EEPROM >> without knowing if its parent is I2C bus or SPI bus ? >> >> >> My rough image is like this: >> >> int eeprom_read(struct udevice *dev, int offset, uint8_t buf, int size) >> { >> struct udevice *bus = dev->parent; >> struct generic_bus_operation *ops = bus->uclass->uc_drv->ops; >> >> return ops->read(dev, bus, offset, buf, size); >> } >> >> I am not sure, but if this approach is possible, >> we do not need to have both of "i2c_eeprom uclass" and "spi_eeprom uclass". >> >> struct generic_bus_operation is the operaton some uclasses have. >> >> We can move i2c_read() and i2c_write() >> to struct generic_bus_operation of i2c uclass. >> >> Likewise, we can move spi_read() and spi_write() >> to struct generic_bus_operation of spi uclass. > > Yes we should do join up I2C and SPI. In order to do it we need SPI > uclass to support reading and writing a particular offset (rather than > just the current generic xfer()). > > Perhaps we should have a generic register access uclass. Drivers that > want to support it could add themselves to the I2C and REGISTER > uclass. But it would be better to handle this entirely in the I2C/SPI > uclasses if we can. > > I added Przemyslaw as he needs something like this for the PMIC uclass. > > Regards, > Simon > I send some general comments as a reply for patch number 0 and it also touches the bus accessing problem. Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com