[U-Boot] [PATCH v3 0/10] dm: Add I2C support and convert sandbox, tegra
Simon Glass
sjg at chromium.org
Fri Dec 5 18:47:47 CET 2014
Hi Prazemyslaw,
On 5 December 2014 at 06:09, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> 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.
Sounds plausible.
>
>> 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;
> # }
I don't like that very much. It would be better to require a PMIC
device to be passed in, then have children use dev_get_parent() to
find it.
>
> Or the second case, the pmic_childs will pass the parent as a device, with
> just determine the proper register.
Yes
>
> I have some other work to do now. And I will back to this probably on
> Thursday.
OK sounds good.
Regards,
Simon
More information about the U-Boot
mailing list