[U-Boot] [PATCH 06/55] dm: i2c: Add support for multiplexed I2C buses

[Simon Glass]

Hi Heiko,

On 5 July 2015 at 00:10, Heiko Schocher <hs at denx.de> wrote:
> Hello Simon,
>
> Am 03.07.2015 um 02:15 schrieb Simon Glass:
>>
>> Add a new I2C_MUX uclass. Devices in this class can multiplex between
>> several I2C buses, selecting them one at a time for use by the system.
>> The multiplexing mechanism is left to the driver to decide - it may be
>> controlled by GPIOs, for example.
>>
>> The uclass supports only two methods: select() and deselect().
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>   doc/device-tree-bindings/i2c/i2c-mux.txt |  60 ++++++++++
>>   drivers/i2c/Kconfig                      |   2 +
>>   drivers/i2c/Makefile                     |   2 +
>>   drivers/i2c/muxes/Kconfig                |   8 ++
>>   drivers/i2c/muxes/Makefile               |   6 +
>>   drivers/i2c/muxes/i2c-mux-uclass.c       | 198
>> +++++++++++++++++++++++++++++++
>>   include/dm/uclass-id.h                   |   1 +
>>   include/i2c.h                            |  32 +++++
>>   8 files changed, 309 insertions(+)
>>   create mode 100644 doc/device-tree-bindings/i2c/i2c-mux.txt
>>   create mode 100644 drivers/i2c/muxes/Kconfig
>>   create mode 100644 drivers/i2c/muxes/Makefile
>>   create mode 100644 drivers/i2c/muxes/i2c-mux-uclass.c
>>
> [...]
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c
>> b/drivers/i2c/muxes/i2c-mux-uclass.c
>> new file mode 100644
>> index 0000000..3f52bff
>> --- /dev/null
>> +++ b/drivers/i2c/muxes/i2c-mux-uclass.c
>> @@ -0,0 +1,198 @@
>
> [...]
>>
>> +static int i2c_mux_bus_xfer(struct udevice *dev, struct i2c_msg *msg,
>> +                           int nmsgs)
>> +{
>> +       struct udevice *mux = dev->parent;
>> +       struct i2c_mux *priv = dev_get_uclass_priv(mux);
>> +       struct dm_i2c_ops *ops = i2c_get_ops(priv->i2c_bus);
>> +       int ret, ret2;
>> +
>> +       debug("%s: %s, bus %s\n", __func__, dev->name,
>> priv->i2c_bus->name);
>> +       if (!ops->xfer)
>> +               return -ENOSYS;
>> +       ret = i2c_mux_select(dev);
>> +       if (ret)
>> +               return ret;
>> +       ret = ops->xfer(priv->i2c_bus, msg, nmsgs);
>> +       ret2 = i2c_mux_deselect(dev);
>> +
>> +       return ret ? ret : ret2;
>> +}
>
>
> So we have for each transaction a select() and deselect() call.
> Is it possible to store the last accessed "way", and only switch
> with deselect(), select() if this is really needed?
>
> Beside of this "nitpick", it looks good, thanks for the great work!

I think it is best to have that state in the mux since it is the only
thing that knows how to make things work.

The mux can record the current state and then avoid switching unless
it is necessary. It can make deselect a nop if required.

Also for the GPIO arbitrator, we must deselect and re-select each
time. This is the same API used by Linux and I think it is more
flexible.

Regards,
Simon