[U-Boot] [PATCH v3 08/10] dm: Add a simple EEPROM driver

[Przemyslaw Marczak]

Hello,
On 12/03/2014 04:18 PM, Simon Glass wrote:
> Hi Masahiro,
>
> On 1 December 2014 at 04:48, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
>> Hi Simon,
>>
>>
>>
>>
>>
>> On Mon, 24 Nov 2014 11:57:22 -0700
>> Simon Glass <sjg at chromium.org> 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 <common.h>
>>> +#include <dm.h>
>>> +#include <i2c.h>
>>> +#include <i2c_eeprom.h>
>>> +
>>> +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