[U-Boot] [RFC PATCH 0/12] RFC: dm: Add I2C support

[Simon Glass]

Hi Heiko,

On 10 November 2014 00:16, Heiko Schocher <hs at denx.de> wrote:
> Hello Simon,
>
> sorry for the long delay...
>
> Am 13.10.2014 07:39, schrieb Simon Glass:
>>
>> (Note this is RFC since the uclass interface needs discussion and also
>> because only sandbox is implemented so far. But I thought it best to get
>> this out there as soon as I wrote it as it may influence the PMIC library,
>> etc.)
>>
>> This series is an initial attempt to add I2C support to driver model. It
>> has become apparent that this is a high priority as it is widely used. It
>> follows along to some extent from the SPI conversion.
>>
>> Several changes are made from the original I2C implementations.
>>
>> Firstly It is not necessary to specify the chip address with every call,
>> since each chip knows its own address - it is stored in struct dm_i2c_chip
>> which is attached to each chip on the I2C bus. However, this information
>> *is* passed to the driver since I presume most drivers need it and it
>> would
>> be cumbersome to look up in every call.
>>
>> Secondly there is no concept of a 'current' I2C bus so all associated
>> logic
>> is removed. With driver model i2c_set_bus_num() and i2c_get_bus_num() are
>> not available. Since the chip device specifies both the bus and the chip
>> address, there is no need for this concept. It also causes problems when
>> one driver changes the current bus and forgets to change it back.
>>
>> Thirdly initialisation is handled by driver model's normal probe() method
>> on each device so there should be no need for i2c_init_all(), i2c_init(),
>> i2c_init_board(), i2c_board_late_init() and board_i2c_init().
>
>
> Great!
>
>> I2C muxes are not yet supported. To support these we will need to maintain
>> state of the current mux settings to avoid resetting every mux every time.
>> Probably we need to add a sandbox I2C mux driver to permit testing of
>> this.
>> This can probably be done later.
>
>
> Currently only the keymile boards really use i2c muxes, so I am fine
> with doing this in a second step.
>
>> Platform data is not yet supported either, only device tree. The
>> U_BOOT_I2C_MKENT_COMPLETE() and U_BOOT_I2C_ADAP_COMPLETE() macros are not
>> used. Also struct i2c_adapter is not defined anymore. This will need to be
>> addressed, perhaps as part of converting over a board that does not use
>> device tree.
>
>
> Ok for this in the first step... The question raised if we only would
> support Device tree with DM ... so maybe we do not need to do this step.
>
> I am not really sure, if we should really support Device Tree only with
> DM, because:
>
> - do all archs switch to Device Tree in the near future?
>
> - in SPL we have really on some SoCs small memory (like I just work
>   on some AT91 boards which have 4k only!) To get DM with Device
>   Tree into 4k is a big challenge ... so in my opinion, it would be
>   good to have the possibility of Platform data ... so we prevent
>   to make dirty hacks for the  SPL case (I hope) ...
>
>> This series is available at u-boot-dm/i2c-working.
>
>
> Thanks for your great work.
>
> I looked through your patchset and have no real objection against it ...
> To the "i2c deblocking" subject ... we should add at least the
> "deblock()" in "struct dm_i2c_ops" and call it "do_i2c_reset"
> if defined ... beside of this, you can add my
>
> Acked-by: Heiko Schocher <hs at denx.de>
>
> to the hole series.

Thanks for reviewing this. I have added the deblock() method and also
the generic I2C support (allows you to use a device on the command
line without it having a proper driver). There are a few changes so if
you have time to take another look I will definitely add your
Acked-by.

Regards,
Simon