[U-Boot] [PATCH v3 0/10] dm: Add I2C support and convert sandbox, tegra
Przemyslaw Marczak
p.marczak at samsung.com
Fri Dec 5 14:09:15 CET 2014
Hello,
On 12/04/2014 03:00 AM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 3 December 2014 at 09:13, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
>> Hello all,
>>
>>
>> On 11/24/2014 07:57 PM, Simon Glass wrote:
>>>
>>>
>>> This series adds 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().
>>>
>>> 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.
>>>
>>> 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, assuming that we want to support this.
>>>
>>> The following I2C CONFIGs are no-longer needed when driver model is used:
>>>
>>> CONFIG_SYS_I2C_INIT_BOARD - each I2C bus is inited in its probe()
>>> method
>>> CONFIG_I2C_MULTI_BUS - we always support multi-bus with driver
>>> model
>>> CONFIG_SYS_MAX_I2C_BUS - the device tree aliases define available
>>> buses
>>> CONFIG_SYS_I2C_SPEED - the device tree specifies the speed for
>>> each bus
>>> CONFIG_SYS_I2C - this is the 'new old' API, now deprecated
>>>
>>> There are a few SPI patches included here due to a dependency on a new
>>> device binding function.
>>>
>>> v3 changes the uclass to driver interface significantly. It is now a list
>>> of
>>> messages to be processed by the driver. This matches the Linux user space
>>> API which may be a benefit. It does introduce one complication, which is
>>> that the protocol does not support separate chip offset and data. I have
>>> enhanced it to do so.
>>>
>>> This series is available at u-boot-dm/i2c-working2.
>>>
>>> Changes in v3:
>>> - Change uclass <=> driver API to use a message list
>>> - Correct bogus address len code (was confused with chip address length)
>>> - Drop extra call to i2c_bind_driver() in i2c_probe()
>>> - Add a helper to query chip flags
>>> - Add support for reading a byte at a time with an address for each byte
>>> - Adjust for slightly altered I2C uclass API
>>> - Add new 'i2c flags' command to get/set chip flags
>>> - Update for new uclass <=> driver interface
>>> - Update emulator for new uclass interface
>>> - Update for new uclass <=> emulateor interface
>>> - Change driver to support new message-based I2C uclass API
>>>
>>> Changes in v2:
>>> - Fix cihp typo
>>> - Implement generic I2C devices to allow 'i2c probe' on unknown devices
>>> - Return the probed device from i2c_probe()
>>> - Set the bus speed after the bus is probed
>>> - Add some debugging for generic I2C device binding
>>> - Add a 'deblock' method to recover an I2C bus stuck in mid-transaction
>>> - Add a helper function to find a chip on a particular bus number
>>> - Change alen to int so that it can be -1 (this was a bug)
>>> - Call the deblock() method for 'i2c reset'
>>> - Update commit message for EEPROM driver
>>> - Add a test for automatic binding of generic I2C devices
>>> - Add a new asm/test.h header for tests in sandbox
>>> - Adjust tegra_i2c_child_pre_probe() to permit generic I2C devices
>>> - Correct the compatible strings for I2C buses
>>> - Don't init if the speed is 0, since this breaks the controller
>>> - Expand coverage to all Tegra boards
>>>
>>> Simon Glass (10):
>>> dm: i2c: Add a uclass for I2C
>>> dm: i2c: Implement driver model support in the i2c command
>>> dm: i2c: Add I2C emulation driver for sandbox
>>> dm: i2c: Add a sandbox I2C driver
>>> dm: i2c: Add an I2C EEPROM simulator
>>> dm: i2c: config: Enable I2C for sandbox using driver model
>>> dm: i2c: dts: Add an I2C bus for sandbox
>>> dm: Add a simple EEPROM driver
>>> dm: i2c: Add tests for I2C
>>> dm: i2c: tegra: Convert to driver model
>>>
>>> arch/arm/cpu/tegra20-common/pmu.c | 21 +-
>>> arch/arm/dts/tegra124-jetson-tk1.dts | 1 -
>>> arch/arm/dts/tegra124-norrin.dts | 1 -
>>> arch/arm/dts/tegra30-tec-ng.dts | 4 +
>>> arch/arm/include/asm/arch-tegra/tegra_i2c.h | 2 +-
>>> arch/sandbox/dts/sandbox.dts | 17 ++
>>> arch/sandbox/include/asm/test.h | 15 +
>>> board/avionic-design/common/tamonten-ng.c | 12 +-
>>> board/nvidia/cardhu/cardhu.c | 13 +-
>>> board/nvidia/common/board.c | 4 -
>>> board/nvidia/dalmore/dalmore.c | 21 +-
>>> board/nvidia/whistler/whistler.c | 29 +-
>>> board/toradex/apalis_t30/apalis_t30.c | 19 +-
>>> common/cmd_i2c.c | 376
>>> +++++++++++++++++++++----
>>> drivers/i2c/Makefile | 2 +
>>> drivers/i2c/i2c-emul-uclass.c | 14 +
>>> drivers/i2c/i2c-uclass.c | 411
>>> ++++++++++++++++++++++++++++
>>> drivers/i2c/sandbox_i2c.c | 142 ++++++++++
>>> drivers/i2c/tegra_i2c.c | 363
>>> ++++++++----------------
>>> drivers/misc/Makefile | 4 +
>>> drivers/misc/i2c_eeprom.c | 51 ++++
>>> drivers/misc/i2c_eeprom_emul.c | 108 ++++++++
>>> drivers/power/tps6586x.c | 27 +-
>>> include/config_fallbacks.h | 6 +
>>> include/configs/apalis_t30.h | 3 -
>>> include/configs/beaver.h | 3 -
>>> include/configs/cardhu.h | 5 -
>>> include/configs/colibri_t30.h | 3 -
>>> include/configs/dalmore.h | 5 -
>>> include/configs/jetson-tk1.h | 5 -
>>> include/configs/norrin.h | 5 -
>>> include/configs/sandbox.h | 6 +
>>> include/configs/seaboard.h | 3 -
>>> include/configs/tec-ng.h | 5 -
>>> include/configs/tegra-common.h | 1 +
>>> include/configs/tegra114-common.h | 3 -
>>> include/configs/tegra124-common.h | 3 -
>>> include/configs/tegra20-common.h | 3 -
>>> include/configs/tegra30-common.h | 3 -
>>> include/configs/trimslice.h | 3 -
>>> include/configs/venice2.h | 5 -
>>> include/configs/whistler.h | 3 -
>>> include/dm/uclass-id.h | 4 +
>>> include/i2c.h | 348
>>> +++++++++++++++++++++++
>>> include/i2c_eeprom.h | 19 ++
>>> include/tps6586x.h | 4 +-
>>> test/dm/Makefile | 1 +
>>> test/dm/i2c.c | 112 ++++++++
>>> test/dm/test.dts | 17 ++
>>> 49 files changed, 1805 insertions(+), 430 deletions(-)
>>> create mode 100644 arch/sandbox/include/asm/test.h
>>> create mode 100644 drivers/i2c/i2c-emul-uclass.c
>>> create mode 100644 drivers/i2c/i2c-uclass.c
>>> create mode 100644 drivers/i2c/sandbox_i2c.c
>>> create mode 100644 drivers/misc/i2c_eeprom.c
>>> create mode 100644 drivers/misc/i2c_eeprom_emul.c
>>> create mode 100644 include/i2c_eeprom.h
>>> create mode 100644 test/dm/i2c.c
>>>
>>
>> This comment will touch driver model, i2c and the pmic.
>>
>> I have ported the s3c24x0_i2c driver to new dm i2c api - will send after dm
>> i2c tree merge.
>>
>> I tested this on Trats2(Exynos4412) and Arndale(Exynos5250) and the second
>> one also with the High Speed I2C on port 0.
>>
>> It works fine for me. I have also rebased my pmic framework on it - which
>> was quite backward, because of various other side tasks like this one.
>>
>> Using dm i2c framework for PMIC actually forces the previous assumed pmic
>> hierarchy, which had look like this:
>> - udev PMIC(parent:root, uclass: PMIC)
>> - udev REGULATOR(parent: PMIC, uclass REGULATOR)
>>
>> then both devices can share some data parent<->child
>>
>> Let's assume that we have some pmic device(more than one functionality in a
>> package) - it uses the same address on I2C0 for both PMIC and REGULATOR
>> drivers.
>>
>> So we have one physically device and two drivers (two uclass devices):
>> - PMIC driver for some not standard settings by raw I/O,
>> - REGULATOR driver for some defined regulator ops
>>
>> Single device, single fdt node, and two (or more) uclasses.
>>
>> With the present dm/i2c assumptions it should looks like:
>> - udev PMIC(parent:I2C0, uclass: PMIC)
>> - udev REGULATOR(parent: I2C0, uclass REGULATOR)
>>
>> This allows use I2C by the regulator and pmic if the second one is probed -
>> i2c device will be created.
>>
>> But in the device-tree we have only one device node like:
>> i2c0 at .. {
>> pmic at .. {
>> };
>> };
>>
>> and for this node we can bind only the ONE driver.
>>
>> The short flow for e.g.i2c0 is:
>> 1. i2c_post_bind() - called after bind i2c0 i2c uclass driver
>> 2. dm_scan_fdt_node() - scan each i2c0 subnode, e.g. mentioned "pmic"
>> 3. lists_bind_fdt() - scan U-Boot drivers linked list
>> 4. device_bind() - bind the compatible driver
>> 4. Have I2C0 bus and pmic i2c chip(bus child)
>>
>> And the child_pre_probe() implementation of i2c driver, takes care of i2c
>> chip which was bind as a I2C bus child.
>>
>> Let's consider this two cases:
>> case 1:
>> # FDT pmic subnode without defined regulators
>> # requires only PMIC uclass driver
>> # situation is clear
>>
>> case 2:
>> # FDT pmic subnode with defined regulators
>> # requires PMIC uclass driver and REGULATOR uclass driver to bind
>> # situation is unclear
>>
>> And now, where is the problem?
>> To bind regulator - device_bind() should be called with I2C bus udevice as a
>> parent.
>> Doing this, we lose the PMIC <-> REGULATOR relationship, and those
>> sub-devices can't share any data.
>>
>> This quite good solution allows to acces the I2C bus also by the regulator,
>> because the device_bind() will be called with PMIC's "of_offset" as an
>> argument.
>>
>> When bind and probe the regulator driver?
>> In the first pmic framework patchset, there was a function pmic_init_dm -
>> and this was for manually bind and probe the drivers - then it was working
>> well.
>> Now driver-model can bind the i2c devices automatically - so the
>> pmic_init_dm() function can be limited to probe the pmic drivers only.
>>
>> Testing:
>> For my tests I solved this by manually bind REGULATOR driver in PMIC driver,
>> with the udev I2C bus as a parent for the REGULATOR.
>>
>> This is very unclear because, the true parent(PMIC) changes it's
>> child(REGULATOR) parent field to the bus udevice which is -> I2C bus.
>> So we have:
>> #|_bus_parent
>> ###|_bus_child ---- parent: #parent_bus
>> #####|_bus_child_child ---- parent: #parent_bus (wrong!)
>>
>> And now, the REGULATOR don't know that PMIC is the parent. So the things are
>> going to be complicated.
>>
>> What do you think about extend the struct udevice by the bus?
>> E.g. :
>> struct udevice {
>> struct driver *driver;
>> const char *name;
>> void *platdata;
>> int of_offset;
>> const struct udevice_id *of_id;
>> struct udevice *parent;
>> + struct udevice *bus;
>> void *priv;
>> ...
>>
>> In such device structure, the true parent device which represents a physical
>> device like PMIC(in this case), could bind few and different uclass devices.
>>
>> To get this more simply, the mentioned PMIC which can consists of few
>> subsystems, like:
>> - regulators
>> - charger
>> - mfd
>> - rtc
>> - adc, and some more
>> as a parent - should be able to bind a proper class driver for each
>> functionality - add a number of childs.
>>
>> The another solution is to add new subnodes with different compatibles,
>> but it also requires some combinations - and still the parent is not a bus.
>>
>> I think that separating a bus from parent<->child relationship make the
>> driver model more flexible.
>
> This (a separate bus link) has been discussed with respect to USB but
> until USB is done we won't know what the requirements is there.
>
> How about you have:
>
> PMIC
> |
> -- REGULATOR
> -- GPIO
> -- BATTERY
>
> or similar. The PMIC itself should be like a multi-function device
> (MFD) with child devices. It is the child devices that actually
> implement functionality - the PMIC is just the gateway.
>
Yes, before the dm i2c it was something like that, the pmic platdata was
shared - and each device could do r/w operations thanks to that. But now
it looks like the pmic platdata (actually the previous struct pmic), can
be removed from the new framework, because all required data are
provided by spi/i2c devices.
> With respect to I2C, it feels dodgy to have child devices talking
> directly to the parent's bus. They should talk to the parent.
Yes, that's truth.
>
> If you implemented some sort of register access mechanism in the PMIC
> driver, then its children could use that. Then the PMIC children don't
> need to know about exactly how the PMIC is accessed, just that they
> can read and write registers. You could implement reg_read() and
> reg_write() in the PMIC uclass.
>
> Regards,
> Simon
>
So in this case, the pmic_read/write could use dev->parent as i2c
device(the pmic gateway) and do the check:
# pmic_read(dev, reg) {
# struct udevice *i2c_dev;
#
# if (dev->parent != root)
# i2c_dev = dev->parent;
# else
# i2c_dev = dev;
# }
Or the second case, the pmic_childs will pass the parent as a device,
with just determine the proper register.
I have some other work to do now. And I will back to this probably on
Thursday.
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list