Skip to contentSkip to conversation

dm: Add I2C support and convert sandbox, tegra

PATCH Superseded Simon Glass 2 participants 19 messages Dec 5, 2014 – Dec 10, 2014

Subsystems (4)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover dm: Add I2C support and convert sandbox, tegra

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 v4:
- Add a constant for I2C_MAX_OFFSET_LEN
- Add a probe_chip() method for the eeprom
- Add a probe_chip() method to the uclass
- Add a remove() method for the eeprom
- Add an assert for non-null
- Add an eeprom test mode
- Add chip flags to i2c_probe()
- Add comment to i2c_setup_offset()
- Add comments to indicate which methods are optional
- Add comments to set_bus_speed() method
- Add eeprom debugging output
- Add new patch to add newline to debug() messages
- Add probe_chip() method
- Add support for eeprom offset length
- Add tests for offset length
- Adjust i2c_bind_driver() to avoid calloc()/free()
- Adjust tests to match uclass changes
- Drop set_offset_len() method
- Fix copying of chip flags to message in i2c_setup_offset()
- Fix endianness of i2c_setup_offset()
- Fix method comment for xfer()
- Invert return value of i2c_setup_offset()
- Probe with a message length of 0
- Rename i2c_generic_drv to i2c_generic_chip_drv
- Rename i2c_read_bytewise() and implement a real one
- Update call to i2c_probe()

Changes in v3:
- Add a helper to query chip flags
- Add new 'i2c flags' command to get/set chip flags
- Add support for reading a byte at a time with an address for each byte
- Adjust for slightly altered I2C uclass API
- Change driver to support new message-based I2C uclass API
- 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()
- Update emulator for new uclass interface
- Update for new uclass <=> driver interface
- Update for new uclass <=> emulateor interface

Changes in v2:
- 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
- Add a new asm/test.h header for tests in sandbox
- Add a test for automatic binding of generic I2C devices
- Add some debugging for generic I2C device binding
- Adjust tegra_i2c_child_pre_probe() to permit generic I2C devices
- Call the deblock() method for 'i2c reset'
- Change alen to int so that it can be -1 (this was a bug)
- 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
- 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
- Update commit message for EEPROM driver

Simon Glass (11):
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: device: Add newline to debug() messages
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/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             |  26 ++
 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/core/device.c                       |   6 +-
 drivers/i2c/Makefile                        |   2 +
 drivers/i2c/i2c-emul-uclass.c               |  14 +
 drivers/i2c/i2c-uclass.c                    | 457 ++++++++++++++++++++++++++++
 drivers/i2c/sandbox_i2c.c                   | 129 ++++++++
 drivers/i2c/tegra_i2c.c                     | 366 ++++++++--------------
 drivers/misc/Makefile                       |   4 +
 drivers/misc/i2c_eeprom.c                   |  51 ++++
 drivers/misc/i2c_eeprom_emul.c              | 176 +++++++++++
 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/nyan-big.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/dm/ut.h                             |  12 +
 include/i2c.h                               | 349 +++++++++++++++++++++
 include/i2c_eeprom.h                        |  19 ++
 include/tps6586x.h                          |   4 +-
 test/dm/Makefile                            |   1 +
 test/dm/i2c.c                               | 210 +++++++++++++
 test/dm/test.dts                            |  17 ++
 50 files changed, 2036 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
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#1 lore raw

PATCH v4 1/11 dm: i2c: Add a uclass for I2C

The uclass implements the same operations as the current I2C framework but makes some changes to make it fit driver model better:

- Remove the chip address from API calls
- Remove the address length from API calls
- Remove concept of 'current' I2C bus
- Drop all existing init functions

---

Changes in v4:
- Add a constant for I2C_MAX_OFFSET_LEN
- Add a probe_chip() method to the uclass
- Add chip flags to i2c_probe()
- Add comment to i2c_setup_offset()
- Add comments to indicate which methods are optional
- Add comments to set_bus_speed() method
- Adjust i2c_bind_driver() to avoid calloc()/free()
- Drop set_offset_len() method
- Fix copying of chip flags to message in i2c_setup_offset()
- Fix endianness of i2c_setup_offset()
- Fix method comment for xfer()
- Invert return value of i2c_setup_offset()
- Probe with a message length of 0
- Rename i2c_generic_drv to i2c_generic_chip_drv
- Rename i2c_read_bytewise() and implement a real one

Changes in v3:
- Add a helper to query chip flags
- Add support for reading a byte at a time with an address for each byte
- 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()

Changes in v2:
- 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
- Add some debugging for generic I2C device binding
- 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

 drivers/i2c/Makefile       |   1 +
 drivers/i2c/i2c-uclass.c   | 457 +++++++++++++++++++++++++++++++++++++++++++++
 include/config_fallbacks.h |   6 +
 include/dm/uclass-id.h     |   2 +
 include/i2c.h              | 349 ++++++++++++++++++++++++++++++++++
 5 files changed, 815 insertions(+)
 create mode 100644 drivers/i2c/i2c-uclass.c
drivers/i2c/Makefile +1 -0
index dae3d71..063e097 100644
@@ -4,6 +4,7 @@
 #
 # SPDX-License-Identifier:	GPL-2.0+
 #
+obj-$(CONFIG_DM_I2C) += i2c-uclass.o
 
 obj-$(CONFIG_SYS_I2C_ADI) += adi_i2c.o
 obj-$(CONFIG_I2C_MV) += mv_i2c.o
drivers/i2c/i2c-uclass.c new file +457 -0

457 lines not on this page · raw message

include/config_fallbacks.h +6 -0
index 7d8daa2..91a0a43 100644
@@ -87,4 +87,10 @@
 #undef CONFIG_IMAGE_FORMAT_LEGACY
 #endif
 
+#ifdef CONFIG_DM_I2C
+# ifdef CONFIG_SYS_I2C
+#  error "Cannot define CONFIG_SYS_I2C when CONFIG_DM_I2C is used"
+# endif
+#endif
+
 #endif	/* __CONFIG_FALLBACKS_H */
include/dm/uclass-id.h +2 -0
index 202f59b..01866c3 100644
@@ -29,6 +29,8 @@ enum uclass_id {
 	UCLASS_SPI_FLASH,	/* SPI flash */
 	UCLASS_CROS_EC,	/* Chrome OS EC */
 	UCLASS_THERMAL,		/* Thermal sensor */
+	UCLASS_I2C,		/* I2C bus */
+	UCLASS_I2C_GENERIC,	/* Generic I2C device */
 
 	UCLASS_COUNT,
 	UCLASS_INVALID = -1,
include/i2c.h +349 -0

359 lines not on this page · raw message

Signature · 1 line
2.2.0.rc0.207.ga3a616c
#2 lore raw

PATCH v4 1/11 dm: i2c: Add a uclass for I2C

Hi Simon,

Here are some comments on v4.

Quoted 13 lines · On Thu, 4 Dec 2014 21:21:20 -0700 Simon Glass <[email protected]> wrote:
drivers/i2c/i2c-uclass.c new file +13 -0
+#define I2C_MAX_OFFSET_LEN	4
+
+/**
+ * i2c_setup_offset() - Set up a new message with a chip offset
+ *
+ * @chip:	Chip to use
+ * @offset:	Byte offset within chip
+ * @offset_buf:	Place to put byte offset
+ * @msg:	Message buffer
+ * @return 0 if OK, -EADDRNOTAVAIL if the offset length is 0. In that case the
+ * message is still set up but will not contain an offset.
+ */
+static int i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,

drivers/i2c/i2c-uclass.c line 41

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+			    uint8_t offset_buf[], struct i2c_msg *msg)
+{
+	int offset_len;
+
+	msg->addr = chip->chip_addr;
+	msg->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;
+	msg->len = chip->offset_len;
+	msg->buf = offset_buf;
+	if (!chip->offset_len)
+		return -EADDRNOTAVAIL;

I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.

I think it seems a little bit strange.

Instead of the code above, can we put this here?

       if (chip->offset_len > I2C_MAX_OFFSET_LEN)
		return -EADDRNOTAVAIL;

And then, the normal i2c_{read|write} should also check the return code of this.

	if (!chip->offset_len)
		return -EADDRNOTAVAIL;

is specific to *_bytewise ones, so it can be moved to there.

drivers/i2c/i2c-uclass.c line 44

Quoted 3 lines
drivers/i2c/i2c-uclass.c new file +3 -0
+	offset_len = chip->offset_len;
+	while (offset_len--)
+		*offset_buf++ = offset >> (8 * offset_len);

We should be very careful about this code
because buffer overrun happens if too big offset_len is given.

So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)

I know you implemented a similar check check in i2c_set_chip_offset_len() function.

But users can skip i2c_set_chip_offset_len() and
directly invoke i2c_read() or i2c_write().
This is very dangerous.

drivers/i2c/i2c-uclass.c line 143

Quoted 12 lines
drivers/i2c/i2c-uclass.c new file +12 -0
+int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, int len)
+{
+	struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+	struct udevice *bus = dev_get_parent(dev);
+	struct dm_i2c_ops *ops = i2c_get_ops(bus);
+	struct i2c_msg msg[1];
+
+	if (!ops->xfer)
+		return -ENOSYS;
+
+	if (chip->flags & DM_I2C_CHIP_RE_ADDRESS)
+		return i2c_write_bytewise(dev, offset, buffer, len);

Have you noticed that do_i2c_write() function in common/cmd_i2c.c always does the bytewise-equivalent behavior?

(It uses a while() loop and in each loop it calls i2c_write with length=1)

On the other hand, do_i2c_read() does not split it into small chunks.

At first I was wondering why only do_i2c_write() must split into many small transactions, but I got an answer when I was testing it on my board.

My on-board EEPROM chip does not work with long-burst write, but it works with any long-burst read.

It turned out some chips (at least mine) require DM_I2C_CHIP_RE_ADDRESS only for write.

Maybe, do we need to have a _RE_ADDRESS flag for each of write/read?

drivers/i2c/i2c-uclass.c line 188

Quoted 2 lines
drivers/i2c/i2c-uclass.c new file +2 -0
+static int i2c_probe_chip(struct udevice *bus, uint chip_addr,
+			  enum dm_i2c_chip_flags flags)

I notice you added "flags" argument in v4.

When and how do we use this flag ?

for DM_I2C_CHIP_10BIT ??

Quoted 8 lines
drivers/i2c/i2c-uclass.c new file +8 -0
+{
+	struct dm_i2c_ops *ops = i2c_get_ops(bus);
+	struct i2c_msg msg[1];
+	uint8_t ch = 0;
+	int ret;
+
+	if (ops->probe_chip) {
+		ret = ops->probe_chip(bus, chip_addr, flags);

drivers/i2c/i2c-uclass.c line 206

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+		if (!ret || ret != -ENOSYS)
+			return ret;
+	}
+
+	if (!ops->xfer)
+		return -ENOSYS;
+
+	/* Probe with a zero-length message */
+	msg->addr = chip_addr;
+	msg->flags = 0;

If my guess is correct, this line should be like this?

   ptr->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;

I am not sure..

Quoted 6 lines
drivers/i2c/i2c-uclass.c new file +6 -0
+
+/*
+ * i2c_get_bus_speed:
+ *
+ *  Returns speed of selected I2C bus in Hz
+ */

drivers/i2c/i2c-uclass.c line 353

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+int i2c_get_bus_speed(struct udevice *bus)
+{
+	struct dm_i2c_ops *ops = i2c_get_ops(bus);
+	struct dm_i2c_bus *i2c = bus->uclass_priv;
+
+	if (!ops->set_bus_speed)
+		return i2c->speed_hz;
+
+	return ops->get_bus_speed(bus);
+}

If ops->set_bus_speed is missing, ops->get_bus_speed is never called even if it exists. Isn't it a bit strange?

It is not clear to me why set_bus_speed must be checked.

Why isn't it like this?

int i2c_get_bus_speed(struct udevice *bus)

{
	struct dm_i2c_ops *ops = i2c_get_ops(bus);
	struct dm_i2c_bus *i2c = bus->uclass_priv;

	if (ops->get_bus_speed(bus)
		return ops->get_bus_speed(bus);

	return i2c->speed_hz;
}

When opt->set_bus_speed is missing, I think there are two possibilities:
[1] Changing the bus speed is not supported
[2] Hardware registers are set in .xfer handler

In case [2], somebody still might want to read the bus speed from the hardware register.

include/i2c.h line 274

Quoted 12 lines
include/i2c.h +12 -0
+	/**
+	 * set_bus_speed() - set the speed of a bus (optional)
+	 *
+	 * The bus speed value will be updated by the uclass if this function
+	 * does not return an error. This method is optional - if it is not
+	 * provided then the driver can read the speed from
+	 * bus->uclass_priv->speed_hz.
+	 *
+	 * @bus:	Bus to adjust
+	 * @speed:	Requested speed in Hz
+	 * @return 0 if OK, -INVAL for invalid values
+	 */

I am afraid you missed what I pointed out in v3.

s/-INVAL/-EINVAL/

Best Regards
Masahiro Yamada

#3 lore raw

PATCH v4 1/11 dm: i2c: Add a uclass for I2C

Hi Masahiro,

Quoted 45 lines · Masahiro Yamada

Hi Simon,

Here are some comments on v4.

Quoted 13 lines · On Thu, 4 Dec 2014 21:21:20 -0700 Simon Glass <[email protected]> wrote:
drivers/i2c/i2c-uclass.c new file +13 -0
+#define I2C_MAX_OFFSET_LEN   4
+
+/**
+ * i2c_setup_offset() - Set up a new message with a chip offset
+ *
+ * @chip:    Chip to use
+ * @offset:  Byte offset within chip
+ * @offset_buf:      Place to put byte offset
+ * @msg:     Message buffer
+ * @return 0 if OK, -EADDRNOTAVAIL if the offset length is 0. In that case the
+ * message is still set up but will not contain an offset.
+ */
+static int i2c_setup_offset(struct dm_i2c_chip *chip, uint offset,

drivers/i2c/i2c-uclass.c line 41

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+                         uint8_t offset_buf[], struct i2c_msg *msg)
+{
+     int offset_len;
+
+     msg->addr = chip->chip_addr;
+     msg->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;
+     msg->len = chip->offset_len;
+     msg->buf = offset_buf;
+     if (!chip->offset_len)
+             return -EADDRNOTAVAIL;

I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.

I think it seems a little bit strange.

Instead of the code above, can we put this here?

       if (chip->offset_len > I2C_MAX_OFFSET_LEN)
                return -EADDRNOTAVAIL;
Quoted 10 lines

And then, the normal i2c_{read|write} should also check the return code of this.

        if (!chip->offset_len)
                return -EADDRNOTAVAIL;

is specific to *_bytewise ones, so it can be moved to there.

We are not trying to check that the offset length is too large - that has already been done in ic_set_chip_offset_len(). I will add an assert and a comment. The purpose of the return value is to indicate whether an offset was added at all. In the case of a chip with a zero offset length, when doing a read, we want to omit the 'write' cycle entirely since there is no offset to write. The return value makes that possible.

Quoted 10 lines

drivers/i2c/i2c-uclass.c line 44

Quoted 6 lines
drivers/i2c/i2c-uclass.c new file +3 -0
+     offset_len = chip->offset_len;
+     while (offset_len--)
+             *offset_buf++ = offset >> (8 * offset_len);

We should be very careful about this code
because buffer overrun happens if too big offset_len is given.

Quoted 9 lines

So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)

I know you implemented a similar check check in i2c_set_chip_offset_len() function.

But users can skip i2c_set_chip_offset_len() and
directly invoke i2c_read() or i2c_write().
This is very dangerous.

If they skip it, then the value will be 1, the default. I've added an assert() but I don't really want to bloat the code which duplicate checks. Of course assert() is only useful for debugging.

Quoted 27 lines
Quoted 11 lines
drivers/i2c/i2c-uclass.c new file +2 -0
+int i2c_write(struct udevice *dev, uint offset, const uint8_t *buffer, int len)
+{

drivers/i2c/i2c-uclass.c line 143

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+     struct dm_i2c_chip *chip = dev_get_parentdata(dev);
+     struct udevice *bus = dev_get_parent(dev);
+     struct dm_i2c_ops *ops = i2c_get_ops(bus);
+     struct i2c_msg msg[1];
+
+     if (!ops->xfer)
+             return -ENOSYS;
+
+     if (chip->flags & DM_I2C_CHIP_RE_ADDRESS)
+             return i2c_write_bytewise(dev, offset, buffer, len);

Have you noticed that do_i2c_write() function in common/cmd_i2c.c always does the bytewise-equivalent behavior?

Quoted 1 line

(It uses a while() loop and in each loop it calls i2c_write with length=1)

Yes, in fact someone submitted a patch for this recently.

http://patchwork.ozlabs.org/patch/415117/

I'd like to use the flags to support this at some point.

Quoted 6 lines

On the other hand, do_i2c_read() does not split it into small chunks.

At first I was wondering why only do_i2c_write() must split into many small transactions, but I got an answer when I was testing it on my board.

Quoted 7 lines

My on-board EEPROM chip does not work with long-burst write, but it works with any long-burst read.

It turned out some chips (at least mine) require DM_I2C_CHIP_RE_ADDRESS only for write.

Maybe, do we need to have a _RE_ADDRESS flag for each of write/read?

I was heading that way originally but then stopped when I wasn't sure of the use case. I think I will do this.

Quoted 7 lines
Quoted 8 lines

drivers/i2c/i2c-uclass.c line 188

Quoted 2 lines
drivers/i2c/i2c-uclass.c new file +2 -0
+static int i2c_probe_chip(struct udevice *bus, uint chip_addr,
+                       enum dm_i2c_chip_flags flags)

I notice you added "flags" argument in v4.

When and how do we use this flag ?

for DM_I2C_CHIP_10BIT ??

I commented the exported function i2c_probe() but not this, I'll add a comment.

Quoted 24 lines
Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +8 -0
+{
+     struct dm_i2c_ops *ops = i2c_get_ops(bus);
+     struct i2c_msg msg[1];
+     uint8_t ch = 0;
+     int ret;
+
+     if (ops->probe_chip) {
+             ret = ops->probe_chip(bus, chip_addr, flags);

drivers/i2c/i2c-uclass.c line 206

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+             if (!ret || ret != -ENOSYS)
+                     return ret;
+     }
+
+     if (!ops->xfer)
+             return -ENOSYS;
+
+     /* Probe with a zero-length message */
+     msg->addr = chip_addr;
+     msg->flags = 0;

If my guess is correct, this line should be like this?

Quoted 5 lines
   ptr->flags = chip->flags & DM_I2C_CHIP_10BIT ? I2C_M_TEN : 0;

I am not sure..

Yes we should do that, I'll add it.

Quoted 42 lines
Quoted 9 lines
drivers/i2c/i2c-uclass.c new file +6 -0
+
+/*
+ * i2c_get_bus_speed:
+ *
+ *  Returns speed of selected I2C bus in Hz
+ */

drivers/i2c/i2c-uclass.c line 353

Quoted 10 lines
drivers/i2c/i2c-uclass.c new file +10 -0
+int i2c_get_bus_speed(struct udevice *bus)
+{
+     struct dm_i2c_ops *ops = i2c_get_ops(bus);
+     struct dm_i2c_bus *i2c = bus->uclass_priv;
+
+     if (!ops->set_bus_speed)
+             return i2c->speed_hz;
+
+     return ops->get_bus_speed(bus);
+}

If ops->set_bus_speed is missing, ops->get_bus_speed is never called even if it exists. Isn't it a bit strange?

It is not clear to me why set_bus_speed must be checked.

Why isn't it like this?

int i2c_get_bus_speed(struct udevice *bus)

{
        struct dm_i2c_ops *ops = i2c_get_ops(bus);
        struct dm_i2c_bus *i2c = bus->uclass_priv;

        if (ops->get_bus_speed(bus)
                return ops->get_bus_speed(bus);

        return i2c->speed_hz;
}
Quoted 8 lines

When opt->set_bus_speed is missing, I think there are two possibilities:
[1] Changing the bus speed is not supported
[2] Hardware registers are set in .xfer handler

In case [2], somebody still might want to read the bus speed from the hardware register.

Well that's a strange case. But I think the simplest thing is to make these two completely independent as you suggest. I'll also document the return value for xfer() when the speed is wrong.

I'm not 100% sure of the use case for get_bus_speed() but I have seen hardware where 100KHz cannot be honoured exactly, or where clock change problems may cause the rate to change under your feet. So I'd like to keep it here.

Quoted 17 lines
Quoted 6 lines
include/i2c.h +2 -0
+     /**
+      * set_bus_speed() - set the speed of a bus (optional)

include/i2c.h line 274

Quoted 10 lines
include/i2c.h +10 -0
+      *
+      * The bus speed value will be updated by the uclass if this function
+      * does not return an error. This method is optional - if it is not
+      * provided then the driver can read the speed from
+      * bus->uclass_priv->speed_hz.
+      *
+      * @bus:        Bus to adjust
+      * @speed:      Requested speed in Hz
+      * @return 0 if OK, -INVAL for invalid values
+      */
Quoted 5 lines

I am afraid you missed what I pointed out in v3.

s/-INVAL/-EINVAL/

Fixed thanks.

Regards,
Simon

#4 lore raw

PATCH v4 1/11 dm: i2c: Add a uclass for I2C

Hi Simon,
Sorry for my late reply.

On Fri, 5 Dec 2014 07:41:50 -0700
Simon Glass <[email protected]> wrote:
n -EADDRNOTAVAIL;

Quoted 23 lines
Quoted 12 lines

I notice i2c_{read|write}_bytewise checks the return code of this function, but the normal one does not.

I think it seems a little bit strange.

Instead of the code above, can we put this here?

       if (chip->offset_len > I2C_MAX_OFFSET_LEN)
                return -EADDRNOTAVAIL;
Quoted 10 lines

And then, the normal i2c_{read|write} should also check the return code of this.

        if (!chip->offset_len)
                return -EADDRNOTAVAIL;

is specific to *_bytewise ones, so it can be moved to there.

Quoted 7 lines

We are not trying to check that the offset length is too large - that has already been done in ic_set_chip_offset_len(). I will add an assert and a comment. The purpose of the return value is to indicate whether an offset was added at all. In the case of a chip with a zero offset length, when doing a read, we want to omit the 'write' cycle entirely since there is no offset to write. The return value makes that possible.

I missed this.
OK, I agreed that we should keep this as is.

Quoted 15 lines
Quoted 15 lines

drivers/i2c/i2c-uclass.c line 44

Quoted 6 lines
drivers/i2c/i2c-uclass.c new file +3 -0
+     offset_len = chip->offset_len;
+     while (offset_len--)
+             *offset_buf++ = offset >> (8 * offset_len);

We should be very careful about this code
because buffer overrun happens if too big offset_len is given.

So, we should make sure that offset_len is no larger than I2C_MAX_OFFSET_LEN. (Or, you can pass the length of offset_buf[] to this function.)

I know you implemented a similar check check in i2c_set_chip_offset_len() function.

Quoted 8 lines
Quoted 4 lines

But users can skip i2c_set_chip_offset_len() and
directly invoke i2c_read() or i2c_write().
This is very dangerous.

If they skip it, then the value will be 1, the default. I've added an assert() but I don't really want to bloat the code which duplicate checks. Of course assert() is only useful for debugging.

You are right, thanks.

Best Regards
Masahiro Yamada

#5 lore raw

PATCH v4 2/11 dm: i2c: Implement driver model support in the i2c command

In reply to Simon Glass

The concept of a 'current bus' is now implemented in the command line rather than in the uclass. Also the address length does not need to be specified with each command - really we should consider dropping this from most commands but it works OK for now.

---

Changes in v4:
- Update call to i2c_probe()

Changes in v3:
- Add new 'i2c flags' command to get/set chip flags
- Adjust for slightly altered I2C uclass API

Changes in v2:
- Call the deblock() method for 'i2c reset'
- Change alen to int so that it can be -1 (this was a bug)

 common/cmd_i2c.c | 376 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 323 insertions(+), 53 deletions(-)
common/cmd_i2c.c +323 -53
index c266b88..22db1bb 100644
@@ -69,8 +69,10 @@
 #include <bootretry.h>
 #include <cli.h>
 #include <command.h>
+#include <dm.h>
 #include <edid.h>
 #include <environment.h>
+#include <errno.h>
 #include <i2c.h>
 #include <malloc.h>
 #include <asm/byteorder.h>
@@ -117,6 +119,60 @@ static uchar i2c_no_probes[] = CONFIG_SYS_I2C_NOPROBES;
 
 #define DISP_LINE_LEN	16
 
+/*
+ * Default for driver model is to use the chip's existing address length.
+ * For legacy code, this is not stored, so we need to use a suitable
+ * default.
+ */
+#ifdef CONFIG_DM_I2C
+#define DEFAULT_ADDR_LEN	(-1)
+#else
+#define DEFAULT_ADDR_LEN	1
+#endif
+
+#ifdef CONFIG_DM_I2C
+static struct udevice *i2c_cur_bus;
+
+static int i2c_set_bus_num(unsigned int busnum)
+{
+	struct udevice *bus;
+	int ret;
+
+	ret = uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus);
+	if (ret) {
+		debug("%s: No bus %d\n", __func__, busnum);
+		return ret;
+	}
+	i2c_cur_bus = bus;
+
+	return 0;
+}
+
+static int i2c_get_cur_bus(struct udevice **busp)
+{
+	if (!i2c_cur_bus) {
+		puts("No I2C bus selected\n");
+		return -ENODEV;
+	}
+	*busp = i2c_cur_bus;
+
+	return 0;
+}
+
+static int i2c_get_cur_bus_chip(uint chip_addr, struct udevice **devp)
+{
+	struct udevice *bus;
+	int ret;
+
+	ret = i2c_get_cur_bus(&bus);
+	if (ret)
+		return ret;
+
+	return i2c_get_chip(bus, chip_addr, devp);
+}
+
+#endif
+
 /**
  * i2c_init_board() - Board-specific I2C bus init
  *
@@ -143,7 +199,7 @@ void i2c_init_board(void)
  *
  * Returns I2C bus speed in Hz.
  */
-#if !defined(CONFIG_SYS_I2C)
+#if !defined(CONFIG_SYS_I2C) && !defined(CONFIG_DM_I2C)
 /*
  * TODO: Implement architecture-specific get/set functions
  * Should go away, if we switched completely to new multibus support
@@ -182,12 +238,12 @@ int i2c_set_bus_speed(unsigned int speed)
  *
  * Returns the address length.
  */
-static uint get_alen(char *arg)
+static uint get_alen(char *arg, int default_len)
 {
 	int	j;
 	int	alen;
 
-	alen = 1;
+	alen = default_len;
 	for (j = 0; j < 8; j++) {
 		if (arg[j] == '.') {
 			alen = arg[j+1] - '0';
@@ -227,8 +283,13 @@ static int i2c_report_err(int ret, enum i2c_err_op op)
 static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	u_char	chip;
-	uint	devaddr, alen, length;
+	uint	devaddr, length;
+	int alen;
 	u_char  *memaddr;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc != 5)
 		return CMD_RET_USAGE;
@@ -243,7 +304,7 @@ static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 	 * 2 bytes long.  Some day it might be 3 bytes long :-).
 	 */
 	devaddr = simple_strtoul(argv[2], NULL, 16);
-	alen = get_alen(argv[2]);
+	alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 	if (alen > 3)
 		return CMD_RET_USAGE;
 
@@ -257,18 +318,31 @@ static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 	 */
 	memaddr = (u_char *)simple_strtoul(argv[4], NULL, 16);
 
-	if (i2c_read(chip, devaddr, alen, memaddr, length) != 0) {
-		i2c_report_err(-1, I2C_ERR_READ);
-		return 1;
-	}
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (!ret)
+		ret = i2c_read(dev, devaddr, memaddr, length);
+#else
+	ret = i2c_read(chip, devaddr, alen, memaddr, length);
+#endif
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
+
 	return 0;
 }
 
 static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	u_char	chip;
-	uint	devaddr, alen, length;
+	uint	devaddr, length;
+	int alen;
 	u_char  *memaddr;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc != 5)
 		return cmd_usage(cmdtp);
@@ -288,7 +362,7 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 	 * 2 bytes long.  Some day it might be 3 bytes long :-).
 	 */
 	devaddr = simple_strtoul(argv[3], NULL, 16);
-	alen = get_alen(argv[3]);
+	alen = get_alen(argv[3], DEFAULT_ADDR_LEN);
 	if (alen > 3)
 		return cmd_usage(cmdtp);
 
@@ -297,10 +371,22 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 	 */
 	length = simple_strtoul(argv[4], NULL, 16);
 
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_WRITE);
+#endif
+
 	while (length-- > 0) {
-		if (i2c_write(chip, devaddr++, alen, memaddr++, 1) != 0) {
-			return i2c_report_err(-1, I2C_ERR_WRITE);
-		}
+#ifdef CONFIG_DM_I2C
+		ret = i2c_write(dev, devaddr++, memaddr++, 1);
+#else
+		ret = i2c_write(chip, devaddr++, alen, memaddr++, 1);
+#endif
+		if (ret)
+			return i2c_report_err(ret, I2C_ERR_WRITE);
 /*
  * No write delay with FRAM devices.
  */
@@ -311,6 +397,38 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 	return 0;
 }
 
+#ifdef CONFIG_DM_I2C
+static int do_i2c_flags(cmd_tbl_t *cmdtp, int flag, int argc,
+			char *const argv[])
+{
+	struct udevice *dev;
+	uint flags;
+	int chip;
+	int ret;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	chip = simple_strtoul(argv[1], NULL, 16);
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
+
+	if (argc > 2) {
+		flags = simple_strtoul(argv[2], NULL, 16);
+		ret = i2c_set_chip_flags(dev, flags);
+	} else  {
+		ret = i2c_get_chip_flags(dev, &flags);
+		if (!ret)
+			printf("%x\n", flags);
+	}
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
+
+	return 0;
+}
+#endif
+
 /**
  * do_i2c_md() - Handle the "i2c md" command-line command
  * @cmdtp:	Command data struct pointer
@@ -327,8 +445,13 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
 static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	u_char	chip;
-	uint	addr, alen, length;
+	uint	addr, length;
+	int alen;
 	int	j, nbytes, linebytes;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	/* We use the last specified parameters, unless new ones are
 	 * entered.
@@ -356,7 +479,7 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		 * 2 bytes long.  Some day it might be 3 bytes long :-).
 		 */
 		addr = simple_strtoul(argv[2], NULL, 16);
-		alen = get_alen(argv[2]);
+		alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 		if (alen > 3)
 			return CMD_RET_USAGE;
 
@@ -368,6 +491,14 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 			length = simple_strtoul(argv[3], NULL, 16);
 	}
 
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
+#endif
+
 	/*
 	 * Print the lines.
 	 *
@@ -381,8 +512,13 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 
 		linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes;
 
-		if (i2c_read(chip, addr, alen, linebuf, linebytes) != 0)
-			i2c_report_err(-1, I2C_ERR_READ);
+#ifdef CONFIG_DM_I2C
+		ret = i2c_read(dev, addr, linebuf, linebytes);
+#else
+		ret = i2c_read(chip, addr, alen, linebuf, linebytes);
+#endif
+		if (ret)
+			i2c_report_err(ret, I2C_ERR_READ);
 		else {
 			printf("%04x:", addr);
 			cp = linebuf;
@@ -429,9 +565,13 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	uchar	byte;
 	int	count;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if ((argc < 4) || (argc > 5))
 		return CMD_RET_USAGE;
@@ -445,10 +585,17 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	 * Address is always specified.
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
-	alen = get_alen(argv[2]);
+	alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 	if (alen > 3)
 		return CMD_RET_USAGE;
 
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_WRITE);
+#endif
 	/*
 	 * Value to write is always specified.
 	 */
@@ -463,8 +610,13 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		count = 1;
 
 	while (count-- > 0) {
-		if (i2c_write(chip, addr++, alen, &byte, 1) != 0)
-			i2c_report_err(-1, I2C_ERR_WRITE);
+#ifdef CONFIG_DM_I2C
+		ret = i2c_write(dev, addr++, &byte, 1);
+#else
+		ret = i2c_write(chip, addr++, alen, &byte, 1);
+#endif
+		if (ret)
+			i2c_report_err(ret, I2C_ERR_WRITE);
 		/*
 		 * Wait for the write to complete.  The write can take
 		 * up to 10mSec (we allow a little more time).
@@ -499,11 +651,15 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	int	count;
 	uchar	byte;
 	ulong	crc;
 	ulong	err;
+	int ret = 0;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc < 4)
 		return CMD_RET_USAGE;
@@ -517,10 +673,17 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	 * Address is always specified.
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
-	alen = get_alen(argv[2]);
+	alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 	if (alen > 3)
 		return CMD_RET_USAGE;
 
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
+#endif
 	/*
 	 * Count is always specified
 	 */
@@ -534,13 +697,18 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	crc = 0;
 	err = 0;
 	while (count-- > 0) {
-		if (i2c_read(chip, addr, alen, &byte, 1) != 0)
+#ifdef CONFIG_DM_I2C
+		ret = i2c_read(dev, addr, &byte, 1);
+#else
+		ret = i2c_read(chip, addr, alen, &byte, 1);
+#endif
+		if (ret)
 			err++;
 		crc = crc32 (crc, &byte, 1);
 		addr++;
 	}
 	if (err > 0)
-		i2c_report_err(-1, I2C_ERR_READ);
+		i2c_report_err(ret, I2C_ERR_READ);
 	else
 		printf ("%08lx\n", crc);
 
@@ -568,10 +736,14 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	ulong	data;
 	int	size = 1;
 	int	nbytes;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc != 3)
 		return CMD_RET_USAGE;
@@ -601,19 +773,32 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg
 		 * Address is always specified.
 		 */
 		addr = simple_strtoul(argv[2], NULL, 16);
-		alen = get_alen(argv[2]);
+		alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 		if (alen > 3)
 			return CMD_RET_USAGE;
 	}
 
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_WRITE);
+#endif
+
 	/*
 	 * Print the address, followed by value.  Then accept input for
 	 * the next value.  A non-converted value exits.
 	 */
 	do {
 		printf("%08lx:", addr);
-		if (i2c_read(chip, addr, alen, (uchar *)&data, size) != 0)
-			i2c_report_err(-1, I2C_ERR_READ);
+#ifdef CONFIG_DM_I2C
+		ret = i2c_read(dev, addr, (uchar *)&data, size);
+#else
+		ret = i2c_read(chip, addr, alen, (uchar *)&data, size);
+#endif
+		if (ret)
+			i2c_report_err(ret, I2C_ERR_READ);
 		else {
 			data = cpu_to_be32(data);
 			if (size == 1)
@@ -655,8 +840,15 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const arg
 				 * good enough to not time out
 				 */
 				bootretry_reset_cmd_timeout();
-				if (i2c_write(chip, addr, alen, (uchar *)&data, size) != 0)
-					i2c_report_err(-1, I2C_ERR_WRITE);
+#ifdef CONFIG_DM_I2C
+				ret = i2c_write(dev, addr, (uchar *)&data,
+						size);
+#else
+				ret = i2c_write(chip, addr, alen,
+						(uchar *)&data, size);
+#endif
+				if (ret)
+					i2c_report_err(ret, I2C_ERR_WRITE);
 #ifdef CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS
 				udelay(CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS * 1000);
 #endif
@@ -697,6 +889,13 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 	int k, skip;
 	unsigned int bus = GET_BUS_NUM;
 #endif	/* NOPROBES */
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *bus, *dev;
+
+	if (i2c_get_cur_bus(&bus))
+		return CMD_RET_FAILURE;
+#endif
 
 	if (argc == 2)
 		addr = simple_strtol(argv[1], 0, 16);
@@ -717,7 +916,12 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		if (skip)
 			continue;
 #endif
-		if (i2c_probe(j) == 0) {
+#ifdef CONFIG_DM_I2C
+		ret = i2c_probe(bus, j, 0, &dev);
+#else
+		ret = i2c_probe(j);
+#endif
+		if (ret == 0) {
 			printf(" %02X", j);
 			found++;
 		}
@@ -754,11 +958,15 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	u_char	chip;
-	ulong	alen;
+	int alen;
 	uint	addr;
 	uint	length;
 	u_char	bytes[16];
 	int	delay;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc < 3)
 		return CMD_RET_USAGE;
@@ -772,9 +980,16 @@ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	 * Address is always specified.
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
-	alen = get_alen(argv[2]);
+	alen = get_alen(argv[2], DEFAULT_ADDR_LEN);
 	if (alen > 3)
 		return CMD_RET_USAGE;
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret && alen != -1)
+		ret = i2c_set_chip_offset_len(dev, alen);
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_WRITE);
+#endif
 
 	/*
 	 * Length is the number of objects, not number of bytes.
@@ -794,8 +1009,13 @@ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 	 * Run the loop...
 	 */
 	while (1) {
-		if (i2c_read(chip, addr, alen, bytes, length) != 0)
-			i2c_report_err(-1, I2C_ERR_READ);
+#ifdef CONFIG_DM_I2C
+		ret = i2c_read(dev, addr, bytes, length);
+#else
+		ret = i2c_read(chip, addr, alen, bytes, length);
+#endif
+		if (ret)
+			i2c_report_err(ret, I2C_ERR_READ);
 		udelay(delay);
 	}
 
@@ -1345,6 +1565,10 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
 	u_char chip;
 	struct edid1_info edid;
+	int ret;
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#endif
 
 	if (argc < 2) {
 		cmd_usage(cmdtp);
@@ -1352,10 +1576,15 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 	}
 
 	chip = simple_strtoul(argv[1], NULL, 16);
-	if (i2c_read(chip, 0, 1, (uchar *)&edid, sizeof(edid)) != 0) {
-		i2c_report_err(-1, I2C_ERR_READ);
-		return 1;
-	}
+#ifdef CONFIG_DM_I2C
+	ret = i2c_get_cur_bus_chip(chip, &dev);
+	if (!ret)
+		ret = i2c_read(dev, 0, (uchar *)&edid, sizeof(edid));
+#else
+	ret = i2c_read(chip, 0, 1, (uchar *)&edid, sizeof(edid));
+#endif
+	if (ret)
+		return i2c_report_err(ret, I2C_ERR_READ);
 
 	if (edid_check_info(&edid)) {
 		puts("Content isn't valid EDID.\n");
@@ -1437,17 +1666,28 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
  * Returns zero on success, CMD_RET_USAGE in case of misuse and negative
  * on error.
  */
-#if defined(CONFIG_SYS_I2C) || defined(CONFIG_I2C_MULTI_BUS)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_I2C_MULTI_BUS) || \
+		defined(CONFIG_DM_I2C)
 static int do_i2c_bus_num(cmd_tbl_t *cmdtp, int flag, int argc,
 				char * const argv[])
 {
 	int		ret = 0;
-	unsigned int	bus_no;
+	int	bus_no;
 
-	if (argc == 1)
+	if (argc == 1) {
 		/* querying current setting */
-		printf("Current bus is %d\n", i2c_get_bus_num());
-	else {
+#ifdef CONFIG_DM_I2C
+		struct udevice *bus;
+
+		if (!i2c_get_cur_bus(&bus))
+			bus_no = bus->seq;
+		else
+			bus_no = -1;
+#else
+		bus_no = i2c_get_bus_num();
+#endif
+		printf("Current bus is %d\n", bus_no);
+	} else {
 		bus_no = simple_strtoul(argv[1], NULL, 10);
 #if defined(CONFIG_SYS_I2C)
 		if (bus_no >= CONFIG_SYS_NUM_I2C_BUSES) {
@@ -1478,13 +1718,28 @@ static int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char * const
 {
 	int speed, ret=0;
 
-	if (argc == 1)
+#ifdef CONFIG_DM_I2C
+	struct udevice *bus;
+
+	if (i2c_get_cur_bus(&bus))
+		return 1;
+#endif
+	if (argc == 1) {
+#ifdef CONFIG_DM_I2C
+		speed = i2c_get_bus_speed(bus);
+#else
+		speed = i2c_get_bus_speed();
+#endif
 		/* querying current speed */
-		printf("Current bus speed=%d\n", i2c_get_bus_speed());
-	else {
+		printf("Current bus speed=%d\n", speed);
+	} else {
 		speed = simple_strtoul(argv[1], NULL, 10);
 		printf("Setting bus speed to %d Hz\n", speed);
+#ifdef CONFIG_DM_I2C
+		ret = i2c_set_bus_speed(bus, speed);
+#else
 		ret = i2c_set_bus_speed(speed);
+#endif
 		if (ret)
 			printf("Failure changing bus speed (%d)\n", ret);
 	}
@@ -1532,7 +1787,16 @@ static int do_i2c_nm(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
  */
 static int do_i2c_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 {
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_DM_I2C)
+	struct udevice *bus;
+
+	if (i2c_get_cur_bus(&bus))
+		return CMD_RET_FAILURE;
+	if (i2c_deblock(bus)) {
+		printf("Error: Not supported by the driver\n");
+		return CMD_RET_FAILURE;
+	}
+#elif defined(CONFIG_SYS_I2C)
 	i2c_init(I2C_ADAP->speed, I2C_ADAP->slaveaddr);
 #else
 	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
@@ -1546,7 +1810,7 @@ static cmd_tbl_t cmd_i2c_sub[] = {
 #endif
 	U_BOOT_CMD_MKENT(crc32, 3, 1, do_i2c_crc, "", ""),
 #if defined(CONFIG_SYS_I2C) || \
-	defined(CONFIG_I2C_MULTI_BUS)
+	defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C)
 	U_BOOT_CMD_MKENT(dev, 1, 1, do_i2c_bus_num, "", ""),
 #endif  /* CONFIG_I2C_MULTI_BUS */
 #if defined(CONFIG_I2C_EDID)
@@ -1560,6 +1824,9 @@ static cmd_tbl_t cmd_i2c_sub[] = {
 	U_BOOT_CMD_MKENT(probe, 0, 1, do_i2c_probe, "", ""),
 	U_BOOT_CMD_MKENT(read, 5, 1, do_i2c_read, "", ""),
 	U_BOOT_CMD_MKENT(write, 5, 0, do_i2c_write, "", ""),
+#ifdef CONFIG_DM_I2C
+	U_BOOT_CMD_MKENT(flags, 2, 1, do_i2c_flags, "", ""),
+#endif
 	U_BOOT_CMD_MKENT(reset, 0, 1, do_i2c_reset, "", ""),
 #if defined(CONFIG_CMD_SDRAM)
 	U_BOOT_CMD_MKENT(sdram, 1, 1, do_sdram, "", ""),
@@ -1610,7 +1877,7 @@ static char i2c_help_text[] =
 #endif
 	"crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n"
 #if defined(CONFIG_SYS_I2C) || \
-	defined(CONFIG_I2C_MULTI_BUS)
+	defined(CONFIG_I2C_MULTI_BUS) || defined(CONFIG_DM_I2C)
 	"i2c dev [dev] - show or set current I2C bus\n"
 #endif  /* CONFIG_I2C_MULTI_BUS */
 #if defined(CONFIG_I2C_EDID)
@@ -1622,8 +1889,11 @@ static char i2c_help_text[] =
 	"i2c mw chip address[.0, .1, .2] value [count] - write to I2C device (fill)\n"
 	"i2c nm chip address[.0, .1, .2] - write to I2C device (constant address)\n"
 	"i2c probe [address] - test for and show device(s) on the I2C bus\n"
-	"i2c read chip address[.0, .1, .2] length memaddress - read to memory \n"
+	"i2c read chip address[.0, .1, .2] length memaddress - read to memory\n"
 	"i2c write memaddress chip address[.0, .1, .2] length - write memory to i2c\n"
+#ifdef CONFIG_DM_I2C
+	"i2c flags chip [flags] - set or get chip flags\n"
+#endif
 	"i2c reset - re-init the I2C Controller\n"
 #if defined(CONFIG_CMD_SDRAM)
 	"i2c sdram chip - print SDRAM configuration information\n"
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#6 lore raw

PATCH v4 3/11 dm: i2c: Add I2C emulation driver for sandbox

In reply to Simon Glass

In order to test I2C we need some sort of emulation interface. Add hooks to allow a driver to emulate an I2C device for sandbox.

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/i2c/Makefile          |  1 +
 drivers/i2c/i2c-emul-uclass.c | 14 ++++++++++++++
 include/dm/uclass-id.h        |  1 +
 3 files changed, 16 insertions(+)
 create mode 100644 drivers/i2c/i2c-emul-uclass.c
drivers/i2c/Makefile +1 -0
index 063e097..5a93473 100644
@@ -27,6 +27,7 @@ obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o
 obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o
 obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o
 obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o
+obj-$(CONFIG_SYS_I2C_SANDBOX) += i2c-emul-uclass.o
 obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o
 obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o
 obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o
drivers/i2c/i2c-emul-uclass.c new file +14 -0
new file mode 100644
index 0000000..aa89f95
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <i2c.h>
+
+UCLASS_DRIVER(i2c_emul) = {
+	.id		= UCLASS_I2C_EMUL,
+	.name		= "i2c_emul",
+};
include/dm/uclass-id.h +1 -0
index 01866c3..16e4224 100644
@@ -19,6 +19,7 @@ enum uclass_id {
 	UCLASS_TEST_FDT,
 	UCLASS_TEST_BUS,
 	UCLASS_SPI_EMUL,	/* sandbox SPI device emulator */
+	UCLASS_I2C_EMUL,	/* sandbox I2C device emulator */
 	UCLASS_SIMPLE_BUS,
 
 	/* U-Boot uclasses start here */
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#7 lore raw

PATCH v4 4/11 dm: i2c: Add a sandbox I2C driver

In reply to Simon Glass

This driver includes some test features such as only supporting certain bus speeds. It passes its I2C traffic through to an emulator.

---

Changes in v4:
- Drop set_offset_len() method

Changes in v3:
- Update for new uclass <=> driver interface

Changes in v2: None

 drivers/i2c/Makefile      |   2 +-
 drivers/i2c/sandbox_i2c.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i2c/sandbox_i2c.c
drivers/i2c/Makefile +1 -1
index 5a93473..6f3c86c 100644
@@ -27,7 +27,7 @@ obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o
 obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o
 obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o
 obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o
-obj-$(CONFIG_SYS_I2C_SANDBOX) += i2c-emul-uclass.o
+obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o
 obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o
 obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o
 obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o
drivers/i2c/sandbox_i2c.c new file +129 -0

129 lines not on this page · raw message

Signature · 1 line
2.2.0.rc0.207.ga3a616c
#8 lore raw

PATCH v4 5/11 dm: i2c: Add an I2C EEPROM simulator

In reply to Simon Glass

To enable testing of I2C, add a simple I2C EEPROM simulator for sandbox. It supports reading and writing from a small data store.

---

Changes in v4:
- Add a probe_chip() method for the eeprom
- Add a remove() method for the eeprom
- Add an eeprom test mode
- Add eeprom debugging output
- Add support for eeprom offset length

Changes in v3:
- Update emulator for new uclass interface

Changes in v2: None

 arch/sandbox/include/asm/test.h |  26 ++++++
 drivers/misc/Makefile           |   3 +
 drivers/misc/i2c_eeprom_emul.c  | 176 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 205 insertions(+)
 create mode 100644 arch/sandbox/include/asm/test.h
 create mode 100644 drivers/misc/i2c_eeprom_emul.c
arch/sandbox/include/asm/test.h new file +26 -0
new file mode 100644
index 0000000..25a0c85
@@ -0,0 +1,26 @@
+/*
+ * Test-related constants for sandbox
+ *
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_TEST_H
+#define __ASM_TEST_H
+
+/* The sandbox driver always permits an I2C device with this address */
+#define SANDBOX_I2C_TEST_ADDR	0x59
+
+enum sandbox_i2c_eeprom_test_mode {
+	SIE_TEST_MODE_NONE,
+	/* Permits read/write of only one byte per I2C transaction */
+	SIE_TEST_MODE_SINGLE_BYTE,
+};
+
+void sandbox_i2c_eeprom_set_test_mode(struct udevice *dev,
+				      enum sandbox_i2c_eeprom_test_mode mode);
+
+void sandbox_i2c_eeprom_set_offset_len(struct udevice *dev, int offset_len);
+
+#endif
drivers/misc/Makefile +3 -0
index 2f2e48f..ff02184 100644
@@ -20,6 +20,9 @@ obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o
 obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o
 obj-$(CONFIG_NS87308) += ns87308.o
 obj-$(CONFIG_PDSP188x) += pdsp188x.o
+ifdef CONFIG_DM_I2C
+obj-$(CONFIG_SANDBOX) += i2c_eeprom_emul.o
+endif
 obj-$(CONFIG_STATUS_LED) += status_led.o
 obj-$(CONFIG_TWL4030_LED) += twl4030_led.o
 obj-$(CONFIG_FSL_IFC) += fsl_ifc.o
drivers/misc/i2c_eeprom_emul.c new file +176 -0

176 lines not on this page · raw message

Signature · 1 line
2.2.0.rc0.207.ga3a616c
#9 lore raw

PATCH v4 6/11 dm: i2c: config: Enable I2C for sandbox using driver model

In reply to Simon Glass

Enable the options to bring up I2C on sandbox. Also enable all the available I2C commands for testing purposes.

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 include/configs/sandbox.h | 6 ++++++
 1 file changed, 6 insertions(+)
include/configs/sandbox.h +6 -0
index 2b03841..657f751 100644
@@ -112,6 +112,12 @@
 #define CONFIG_SPI_FLASH_STMICRO
 #define CONFIG_SPI_FLASH_WINBOND
 
+#define CONFIG_DM_I2C
+#define CONFIG_CMD_I2C
+#define CONFIG_SYS_I2C_SANDBOX
+#define CONFIG_I2C_EDID
+#define CONFIG_I2C_EEPROM
+
 /* Memory things - we don't really want a memory test */
 #define CONFIG_SYS_LOAD_ADDR		0x00000000
 #define CONFIG_SYS_MEMTEST_START	0x00100000
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#10 lore raw

PATCH v4 7/11 dm: i2c: dts: Add an I2C bus for sandbox

In reply to Simon Glass

Add an I2C bus to the device tree, with an EEPROM emulator attached to one of the addresses.

---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/sandbox/dts/sandbox.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
arch/sandbox/dts/sandbox.dts +17 -0
index 7614715..11748ae 100644
@@ -134,6 +134,23 @@
 		num-gpios = <20>;
 	};
 
+	i2c at 0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0>;
+		compatible = "sandbox,i2c";
+		clock-frequency = <400000>;
+		eeprom at 2c {
+			reg = <0x2c>;
+			compatible = "i2c-eeprom";
+			emul {
+				compatible = "sandbox,i2c-eeprom";
+				sandbox,filename = "i2c.bin";
+				sandbox,size = <128>;
+			};
+		};
+	};
+
 	spi at 0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#11 lore raw

PATCH v4 8/11 dm: Add a simple EEPROM driver

In reply to Simon Glass

There seem to be a few EEPROM drivers around - perhaps we should have a single standard one? This simple driver is used for sandbox testing, but could be pressed into more active service.

---

Changes in v4: None
Changes in v3: None
Changes in v2:
- Update commit message for EEPROM driver

 drivers/misc/Makefile     |  1 +
 drivers/misc/i2c_eeprom.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h    |  1 +
 include/i2c_eeprom.h      | 19 ++++++++++++++++++
 4 files changed, 72 insertions(+)
 create mode 100644 drivers/misc/i2c_eeprom.c
 create mode 100644 include/i2c_eeprom.h
drivers/misc/Makefile +1 -0
index ff02184..6fa836f 100644
@@ -15,6 +15,7 @@ obj-$(CONFIG_CROS_EC_SANDBOX) += cros_ec_sandbox.o
 obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
 obj-$(CONFIG_FSL_IIM) += fsl_iim.o
 obj-$(CONFIG_GPIO_LED) += gpio_led.o
+obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o
 obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o
 obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o
 obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o
drivers/misc/i2c_eeprom.c new file +51 -0

51 lines not on this page · raw message

include/dm/uclass-id.h +1 -0
index 16e4224..f17c3c2 100644
@@ -32,6 +32,7 @@ enum uclass_id {
 	UCLASS_THERMAL,		/* Thermal sensor */
 	UCLASS_I2C,		/* I2C bus */
 	UCLASS_I2C_GENERIC,	/* Generic I2C device */
+	UCLASS_I2C_EEPROM,	/* I2C EEPROM device */
 
 	UCLASS_COUNT,
 	UCLASS_INVALID = -1,
include/i2c_eeprom.h new file +19 -0
new file mode 100644
index 0000000..ea6c962
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __I2C_EEPROM
+#define __I2C_EEPROM
+
+struct i2c_eeprom_ops {
+	int (*read)(struct udevice *dev, int offset, uint8_t *buf, int size);
+	int (*write)(struct udevice *dev, int offset, const uint8_t *buf,
+		     int size);
+};
+
+struct i2c_eeprom {
+};
+
+#endif
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#12 lore raw

PATCH v4 9/11 dm: i2c: Add tests for I2C

In reply to Simon Glass

Add some basic tests to check that the system works as expected.

---

Changes in v4:
- Add an assert for non-null
- Add tests for offset length
- Adjust tests to match uclass changes

Changes in v3:
- Update for new uclass <=> emulateor interface

Changes in v2:
- Add a new asm/test.h header for tests in sandbox
- Add a test for automatic binding of generic I2C devices

 include/dm/ut.h  |  12 ++++
 test/dm/Makefile |   1 +
 test/dm/i2c.c    | 210 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/dm/test.dts |  17 +++++
 4 files changed, 240 insertions(+)
 create mode 100644 test/dm/i2c.c
include/dm/ut.h +12 -0
index fa9eac0..ec61465 100644
@@ -89,6 +89,18 @@ void ut_failf(struct dm_test_state *dms, const char *fname, int line,
 	}								\
 }
 
+/* Assert that a pointer is not NULL */
+#define ut_assertnonnull(expr) {					\
+	const void *val = (expr);					\
+									\
+	if (val == NULL) {						\
+		ut_failf(dms, __FILE__, __LINE__, __func__,		\
+			 #expr " = NULL",				\
+			 "Expected non-null, got NULL");		\
+		return -1;						\
+	}								\
+}
+
 /* Assert that an operation succeeds (returns 0) */
 #define ut_assertok(cond)	ut_asserteq(0, cond)
 
test/dm/Makefile +1 -0
index 75d3d41..612aa95 100644
@@ -20,4 +20,5 @@ ifneq ($(CONFIG_SANDBOX),)
 obj-$(CONFIG_DM_GPIO) += gpio.o
 obj-$(CONFIG_DM_SPI) += spi.o
 obj-$(CONFIG_DM_SPI_FLASH) += sf.o
+obj-$(CONFIG_DM_I2C) += i2c.o
 endif
test/dm/i2c.c new file +210 -0

210 lines not on this page · raw message

test/dm/test.dts +17 -0
index 1fba792..fb0272a 100644
@@ -93,6 +93,23 @@
 		num-gpios = <10>;
 	};
 
+	i2c at 0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0>;
+		compatible = "sandbox,i2c";
+		clock-frequency = <100000>;
+		eeprom at 2c {
+			reg = <0x2c>;
+			compatible = "i2c-eeprom";
+			emul {
+				compatible = "sandbox,i2c-eeprom";
+				sandbox,filename = "i2c.bin";
+				sandbox,size = <256>;
+			};
+		};
+	};
+
 	spi at 0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#13 lore raw

PATCH v4 10/11 dm: device: Add newline to debug() messages

In reply to Simon Glass

Some of these are missing a newline. Add it.

---

Changes in v4:
- Add new patch to add newline to debug() messages

Changes in v3: None
Changes in v2: None

 drivers/core/device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/core/device.c +3 -3
index 6793e1c..963b16f 100644
@@ -234,7 +234,7 @@ int device_probe(struct udevice *dev)
 void *dev_get_platdata(struct udevice *dev)
 {
 	if (!dev) {
-		dm_warn("%s: null device", __func__);
+		dm_warn("%s: null device\n", __func__);
 		return NULL;
 	}
 
@@ -244,7 +244,7 @@ void *dev_get_platdata(struct udevice *dev)
 void *dev_get_priv(struct udevice *dev)
 {
 	if (!dev) {
-		dm_warn("%s: null device", __func__);
+		dm_warn("%s: null device\n", __func__);
 		return NULL;
 	}
 
@@ -254,7 +254,7 @@ void *dev_get_priv(struct udevice *dev)
 void *dev_get_parentdata(struct udevice *dev)
 {
 	if (!dev) {
-		dm_warn("%s: null device", __func__);
+		dm_warn("%s: null device\n", __func__);
 		return NULL;
 	}
 
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#14 lore raw

PATCH v4 11/11 dm: i2c: tegra: Convert to driver model

In reply to Simon Glass

This converts all Tegra boards over to use driver model for I2C. The driver is adjusted to use driver model and the following obsolete CONFIGs are removed:

- CONFIG_SYS_I2C_INIT_BOARD
- CONFIG_I2C_MULTI_BUS
- CONFIG_SYS_MAX_I2C_BUS
- CONFIG_SYS_I2C_SPEED
- CONFIG_SYS_I2C

This has been tested on:
- trimslice (no I2C)
- beaver
- Jetson-TK1

It has not been tested on Tegra 114 as I don't have that board.

---

Changes in v4:
- Add probe_chip() method

Changes in v3:
- Change driver to support new message-based I2C uclass API

Changes in v2:
- 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

 arch/arm/cpu/tegra20-common/pmu.c           |  21 +-
 arch/arm/dts/tegra124-jetson-tk1.dts        |   1 -
 arch/arm/dts/tegra30-tec-ng.dts             |   4 +
 arch/arm/include/asm/arch-tegra/tegra_i2c.h |   2 +-
 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 +-
 drivers/i2c/tegra_i2c.c                     | 366 +++++++++-------------------
 drivers/power/tps6586x.c                    |  27 +-
 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/nyan-big.h                  |   5 -
 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/tps6586x.h                          |   4 +-
 30 files changed, 210 insertions(+), 374 deletions(-)
arch/arm/cpu/tegra20-common/pmu.c +16 -5
index c595f70..36a76a2 100644
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <i2c.h>
 #include <tps6586x.h>
 #include <asm/io.h>
 #include <asm/arch/tegra.h>
@@ -23,9 +24,13 @@
 #define VDD_TRANSITION_STEP	0x06	/* 150mv */
 #define VDD_TRANSITION_RATE	0x06	/* 3.52mv/us */
 
+#define PMI_I2C_ADDRESS	0x34	/* chip requires this address */
+
 int pmu_set_nominal(void)
 {
-	int core, cpu, bus;
+	struct udevice *bus, *dev;
+	int core, cpu;
+	int ret;
 
 	/* by default, the table has been filled with T25 settings */
 	switch (tegra_get_chip_sku()) {
@@ -42,12 +47,18 @@ int pmu_set_nominal(void)
 		return -1;
 	}
 
-	bus = tegra_i2c_get_dvc_bus_num();
-	if (bus == -1) {
+	ret = tegra_i2c_get_dvc_bus(&bus);
+	if (ret) {
 		debug("%s: Cannot find DVC I2C bus\n", __func__);
-		return -1;
+		return ret;
 	}
-	tps6586x_init(bus);
+	ret = i2c_get_chip(bus, PMI_I2C_ADDRESS, &dev);
+	if (ret) {
+		debug("%s: Cannot find DVC I2C chip\n", __func__);
+		return ret;
+	}
+
+	tps6586x_init(dev);
 	tps6586x_set_pwm_mode(TPS6586X_PWM_SM1);
 	return tps6586x_adjust_sm0_sm1(core, cpu, VDD_TRANSITION_STEP,
 				VDD_TRANSITION_RATE, VDD_RELATION);
arch/arm/dts/tegra124-jetson-tk1.dts +0 -1
index ffad116..f6fe9a0 100644
@@ -16,7 +16,6 @@
 		i2c2 = "/i2c at 7000c400";
 		i2c3 = "/i2c at 7000c500";
 		i2c4 = "/i2c at 7000c700";
-		i2c5 = "/i2c at 7000d100";
 		sdhci0 = "/sdhci at 700b0600";
 		sdhci1 = "/sdhci at 700b0400";
 		spi0 = "/spi at 7000d400";
arch/arm/dts/tegra30-tec-ng.dts +4 -0
index 8a69e81..e924acc 100644
@@ -6,6 +6,10 @@
 	model = "Avionic Design Tamonten? NG Evaluation Carrier";
 	compatible = "ad,tec-ng", "nvidia,tegra30";
 
+	aliases {
+		i2c0 = "/i2c at 7000c400";
+	};
+
 	/* GEN2 */
 	i2c at 7000c400 {
 		status = "okay";
arch/arm/include/asm/arch-tegra/tegra_i2c.h +1 -1
index 7ca6907..eeeb247 100644
@@ -167,6 +167,6 @@ struct i2c_ctlr {
  *
  * @return number of bus, or -1 if there is no DVC active
  */
-int tegra_i2c_get_dvc_bus_num(void);
+int tegra_i2c_get_dvc_bus(struct udevice **busp);
 
 #endif	/* _TEGRA_I2C_H_ */
board/avionic-design/common/tamonten-ng.c +10 -2
index 5870b95..86a0844 100644
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/gp_padctrl.h>
 #include <asm/arch/gpio.h>
@@ -51,8 +52,15 @@ void gpio_early_init(void)
 
 void pmu_write(uchar reg, uchar data)
 {
-	i2c_set_bus_num(4);	/* PMU is on bus 4 */
-	i2c_write(PMU_I2C_ADDRESS, reg, 1, &data, 1);
+	struct udevice *dev;
+	int ret;
+
+	ret = i2c_get_chip_for_busnum(4, PMU_I2C_ADDRESS, &dev);
+	if (ret) {
+		debug("%s: Cannot find PMIC I2C chip\n", __func__);
+		return;
+	}
+	i2c_write(dev, reg, &data, 1);
 }
 
 /*
board/nvidia/cardhu/cardhu.c +10 -3
index cc0e5e1..026f45c 100644
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/gp_padctrl.h>
 #include "pinmux-config-cardhu.h"
@@ -37,17 +38,23 @@ void pinmux_init(void)
  */
 void board_sdmmc_voltage_init(void)
 {
+	struct udevice *dev;
 	uchar reg, data_buffer[1];
+	int ret;
 	int i;
 
-	i2c_set_bus_num(0);	/* PMU is on bus 0 */
+	ret = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev);
+	if (ret) {
+		debug("%s: Cannot find PMIC I2C chip\n", __func__);
+		return;
+	}
 
 	/* TPS659110: LDO5_REG = 3.3v, ACTIVE to SDMMC1 */
 	data_buffer[0] = 0x65;
 	reg = 0x32;
 
 	for (i = 0; i < MAX_I2C_RETRY; ++i) {
-		if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1))
+		if (i2c_write(dev, reg, data_buffer, 1))
 			udelay(100);
 	}
 
@@ -56,7 +63,7 @@ void board_sdmmc_voltage_init(void)
 	reg = 0x67;
 
 	for (i = 0; i < MAX_I2C_RETRY; ++i) {
-		if (i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1))
+		if (i2c_write(dev, reg, data_buffer, 1))
 			udelay(100);
 	}
 }
board/nvidia/common/board.c +0 -4
index 0e4a65a..4bdbf01 100644
@@ -113,10 +113,6 @@ int board_init(void)
 	power_det_init();
 
 #ifdef CONFIG_SYS_I2C_TEGRA
-#ifndef CONFIG_SYS_I2C_INIT_BOARD
-#error "You must define CONFIG_SYS_I2C_INIT_BOARD to use i2c on Nvidia boards"
-#endif
-	i2c_init_board();
 # ifdef CONFIG_TEGRA_PMU
 	if (pmu_set_nominal())
 		debug("Failed to select nominal voltages\n");
board/nvidia/dalmore/dalmore.c +15 -6
index f2d05af..2a73746 100644
@@ -15,6 +15,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/gp_padctrl.h>
 #include "pinmux-config-dalmore.h"
@@ -50,18 +51,21 @@ void pinmux_init(void)
  */
 void board_sdmmc_voltage_init(void)
 {
+	struct udevice *dev;
 	uchar reg, data_buffer[1];
 	int ret;
 
-	ret = i2c_set_bus_num(0);/* PMU is on bus 0 */
-	if (ret)
-		printf("%s: i2c_set_bus_num returned %d\n", __func__, ret);
+	ret = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev);
+	if (ret) {
+		debug("%s: Cannot find PMIC I2C chip\n", __func__);
+		return;
+	}
 
 	/* TPS65913: LDO9_VOLTAGE = 3.3V */
 	data_buffer[0] = 0x31;
 	reg = 0x61;
 
-	ret = i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1);
+	ret = i2c_write(dev, reg, data_buffer, 1);
 	if (ret)
 		printf("%s: PMU i2c_write %02X<-%02X returned %d\n",
 			__func__, reg, data_buffer[0], ret);
@@ -70,7 +74,7 @@ void board_sdmmc_voltage_init(void)
 	data_buffer[0] = 0x01;
 	reg = 0x60;
 
-	ret = i2c_write(PMU_I2C_ADDRESS, reg, 1, data_buffer, 1);
+	ret = i2c_write(dev, reg, data_buffer, 1);
 	if (ret)
 		printf("%s: PMU i2c_write %02X<-%02X returned %d\n",
 			__func__, reg, data_buffer[0], ret);
@@ -79,7 +83,12 @@ void board_sdmmc_voltage_init(void)
 	data_buffer[0] = 0x03;
 	reg = 0x14;
 
-	ret = i2c_write(BAT_I2C_ADDRESS, reg, 1, data_buffer, 1);
+	ret = i2c_get_chip_for_busnum(0, BAT_I2C_ADDRESS, &dev);
+	if (ret) {
+		debug("%s: Cannot find charger I2C chip\n", __func__);
+		return;
+	}
+	ret = i2c_write(dev, reg, data_buffer, 1);
 	if (ret)
 		printf("%s: BAT i2c_write %02X<-%02X returned %d\n",
 			__func__, reg, data_buffer[0], ret);
board/nvidia/whistler/whistler.c +18 -11
index 3e9d3d9..3114b20 100644
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/io.h>
 #include <asm/arch/tegra.h>
 #include <asm/arch/clock.h>
@@ -21,23 +22,26 @@
  */
 void pin_mux_mmc(void)
 {
+	struct udevice *dev;
 	uchar val;
 	int ret;
 
 	/* Turn on MAX8907B LDO12 to 2.8V for J40 power */
-	ret = i2c_set_bus_num(0);
-	if (ret)
-		printf("i2c_set_bus_num failed: %d\n", ret);
+	ret = i2c_get_chip_for_busnum(0, 0x3c, &dev);
+	if (ret) {
+		printf("%s: Cannot find MAX8907B I2C chip\n", __func__);
+		return;
+	}
 	val = 0x29;
-	ret = i2c_write(0x3c, 0x46, 1, &val, 1);
+	ret = i2c_write(dev, 0x46, &val, 1);
 	if (ret)
 		printf("i2c_write 0 0x3c 0x46 failed: %d\n", ret);
 	val = 0x00;
-	ret = i2c_write(0x3c, 0x45, 1, &val, 1);
+	ret = i2c_write(dev, 0x45, &val, 1);
 	if (ret)
 		printf("i2c_write 0 0x3c 0x45 failed: %d\n", ret);
 	val = 0x1f;
-	ret = i2c_write(0x3c, 0x44, 1, &val, 1);
+	ret = i2c_write(dev, 0x44, &val, 1);
 	if (ret)
 		printf("i2c_write 0 0x3c 0x44 failed: %d\n", ret);
 
@@ -49,6 +53,7 @@ void pin_mux_mmc(void)
 /* this is a weak define that we are overriding */
 void pin_mux_usb(void)
 {
+	struct udevice *dev;
 	uchar val;
 	int ret;
 
@@ -59,15 +64,17 @@ void pin_mux_usb(void)
 	 */
 
 	/* Turn on TAC6416's GPIO 0+1 for USB1/3's VBUS */
-	ret = i2c_set_bus_num(0);
-	if (ret)
-		printf("i2c_set_bus_num failed: %d\n", ret);
+	ret = i2c_get_chip_for_busnum(0, 0x20, &dev);
+	if (ret) {
+		printf("%s: Cannot find TAC6416 I2C chip\n", __func__);
+		return;
+	}
 	val = 0x03;
-	ret = i2c_write(0x20, 2, 1, &val, 1);
+	ret = i2c_write(dev, 2, &val, 1);
 	if (ret)
 		printf("i2c_write 0 0x20 2 failed: %d\n", ret);
 	val = 0xfc;
-	ret = i2c_write(0x20, 6, 1, &val, 1);
+	ret = i2c_write(dev, 6, &val, 1);
 	if (ret)
 		printf("i2c_write 0 0x20 6 failed: %d\n", ret);
 }
board/toradex/apalis_t30/apalis_t30.c +7 -12
index b9d694a..5d2c024 100644
@@ -6,7 +6,7 @@
  */
 
 #include <common.h>
-
+#include <dm.h>
 #include <asm/arch/gp_padctrl.h>
 #include <asm/arch/pinmux.h>
 #include <asm/gpio.h>
@@ -38,23 +38,20 @@ void pinmux_init(void)
 #ifdef CONFIG_PCI_TEGRA
 int tegra_pcie_board_init(void)
 {
-	unsigned int old_bus;
+	struct udevice *dev;
 	u8 addr, data[1];
 	int err;
 
-	old_bus = i2c_get_bus_num();
-
-	err = i2c_set_bus_num(0);
+	err = i2c_get_chip_for_busnum(0, PMU_I2C_ADDRESS, &dev);
 	if (err) {
-		debug("failed to set I2C bus\n");
+		debug("%s: Cannot find PMIC I2C chip\n", __func__);
 		return err;
 	}
-
 	/* TPS659110: VDD2_OP_REG = 1.05V */
 	data[0] = 0x27;
 	addr = 0x25;
 
-	err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1);
+	err = i2c_write(dev, addr, data, 1);
 	if (err) {
 		debug("failed to set VDD supply\n");
 		return err;
@@ -64,7 +61,7 @@ int tegra_pcie_board_init(void)
 	data[0] = 0x0D;
 	addr = 0x24;
 
-	err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1);
+	err = i2c_write(dev, addr, data, 1);
 	if (err) {
 		debug("failed to enable VDD supply\n");
 		return err;
@@ -74,14 +71,12 @@ int tegra_pcie_board_init(void)
 	data[0] = 0x0D;
 	addr = 0x35;
 
-	err = i2c_write(PMU_I2C_ADDRESS, addr, 1, data, 1);
+	err = i2c_write(dev, addr, data, 1);
 	if (err) {
 		debug("failed to set AVDD supply\n");
 		return err;
 	}
 
-	i2c_set_bus_num(old_bus);
-
 	return 0;
 }
 
drivers/i2c/tegra_i2c.c +119 -247

511 lines not on this page · raw message

drivers/power/tps6586x.c +7 -20
index d29d969..29bab4c 100644
@@ -10,9 +10,7 @@
 #include <asm/io.h>
 #include <i2c.h>
 
-static int bus_num;		/* I2C bus we are on */
-#define I2C_ADDRESS		0x34	/* chip requires this address */
-static char inited;		/* 1 if we have been inited */
+static struct udevice *tps6586x_dev;
 
 enum {
 	/* Registers that we access */
@@ -37,13 +35,9 @@ static int tps6586x_read(int reg)
 	int	i;
 	uchar	data;
 	int	retval = -1;
-	int	old_bus_num;
-
-	old_bus_num = i2c_get_bus_num();
-	i2c_set_bus_num(bus_num);
 
 	for (i = 0; i < MAX_I2C_RETRY; ++i) {
-		if (!i2c_read(I2C_ADDRESS, reg, 1, &data, 1)) {
+		if (!i2c_read(tps6586x_dev, reg,  &data, 1)) {
 			retval = (int)data;
 			goto exit;
 		}
@@ -53,7 +47,6 @@ static int tps6586x_read(int reg)
 	}
 
 exit:
-	i2c_set_bus_num(old_bus_num);
 	debug("pmu_read %x=%x\n", reg, retval);
 	if (retval < 0)
 		debug("%s: failed to read register %#x: %d\n", __func__, reg,
@@ -65,13 +58,9 @@ static int tps6586x_write(int reg, uchar *data, uint len)
 {
 	int	i;
 	int	retval = -1;
-	int	old_bus_num;
-
-	old_bus_num = i2c_get_bus_num();
-	i2c_set_bus_num(bus_num);
 
 	for (i = 0; i < MAX_I2C_RETRY; ++i) {
-		if (!i2c_write(I2C_ADDRESS, reg, 1, data, len)) {
+		if (!i2c_write(tps6586x_dev, reg, data, len)) {
 			retval = 0;
 			goto exit;
 		}
@@ -81,7 +70,6 @@ static int tps6586x_write(int reg, uchar *data, uint len)
 	}
 
 exit:
-	i2c_set_bus_num(old_bus_num);
 	debug("pmu_write %x=%x: ", reg, retval);
 	for (i = 0; i < len; i++)
 		debug("%x ", data[i]);
@@ -163,7 +151,7 @@ int tps6586x_set_pwm_mode(int mask)
 	uchar val;
 	int ret;
 
-	assert(inited);
+	assert(tps6586x_dev);
 	ret = tps6586x_read(PFM_MODE);
 	if (ret != -1) {
 		val = (uchar)ret;
@@ -184,7 +172,7 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate,
 	int sm0, sm1;
 	int bad;
 
-	assert(inited);
+	assert(tps6586x_dev);
 
 	/* get current voltage settings */
 	if (read_voltages(&sm0, &sm1)) {
@@ -255,10 +243,9 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate,
 	return bad ? -1 : 0;
 }
 
-int tps6586x_init(int bus)
+int tps6586x_init(struct udevice *dev)
 {
-	bus_num = bus;
-	inited = 1;
+	tps6586x_dev = dev;
 
 	return 0;
 }
include/configs/apalis_t30.h +0 -3
index 3cde923..61809fc 100644
@@ -26,10 +26,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/beaver.h +0 -3
index 164b2dd..5d765f3 100644
@@ -40,10 +40,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/cardhu.h +0 -5
index 09129c7..758b7ad 100644
@@ -43,12 +43,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/colibri_t30.h +0 -3
index a582e25..ce6f23b 100644
@@ -25,10 +25,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/dalmore.h +0 -5
index ff7ec4a..0b04ee6 100644
@@ -36,12 +36,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/jetson-tk1.h +0 -5
index d67c025..a7d7665 100644
@@ -25,12 +25,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/nyan-big.h +0 -5
index a62bc4e..cf331ab 100644
@@ -25,12 +25,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/seaboard.h +0 -3
index 04e4f82..5f77051 100644
@@ -37,10 +37,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/tec-ng.h +0 -5
index 51f87da..e37b233 100644
@@ -23,12 +23,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/tegra-common.h +1 -0
index d690045..0685328 100644
@@ -26,6 +26,7 @@
 #endif
 #define CONFIG_DM_SPI
 #define CONFIG_DM_SPI_FLASH
+#define CONFIG_DM_I2C
 
 #define CONFIG_SYS_TIMER_RATE		1000000
 #define CONFIG_SYS_TIMER_COUNTER	NV_PA_TMRUS_BASE
include/configs/tegra114-common.h +0 -3
index 555c237..9eba5d5 100644
@@ -76,9 +76,6 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
-/* Total I2C ports on Tegra114 */
-#define TEGRA_I2C_NUM_CONTROLLERS	5
-
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
include/configs/tegra124-common.h +0 -3
index 61e5026..f2b3774 100644
@@ -68,9 +68,6 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
-/* Total I2C ports on Tegra124 */
-#define TEGRA_I2C_NUM_CONTROLLERS	5
-
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
include/configs/tegra20-common.h +0 -3
index 21bf977..6330281 100644
@@ -97,9 +97,6 @@
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 1
 
-/* Total I2C ports on Tegra20 */
-#define TEGRA_I2C_NUM_CONTROLLERS	4
-
 #define CONFIG_SYS_NAND_SELF_INIT
 #define CONFIG_SYS_NAND_ONFI_DETECTION
 
include/configs/tegra30-common.h +0 -3
index 443c842..bfdbeb7 100644
@@ -73,9 +73,6 @@
 #define CONFIG_SYS_SPL_MALLOC_START	0x80090000
 #define CONFIG_SPL_STACK		0x800ffffc
 
-/* Total I2C ports on Tegra30 */
-#define TEGRA_I2C_NUM_CONTROLLERS	5
-
 /* For USB EHCI controller */
 #define CONFIG_EHCI_IS_TDI
 #define CONFIG_USB_EHCI_TXFIFO_THRESH	0x10
include/configs/trimslice.h +0 -3
index 7c00642..a254f86 100644
@@ -34,10 +34,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/venice2.h +0 -5
index 6897aa8..8880de8 100644
@@ -25,12 +25,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_I2C_MULTI_BUS
-#define CONFIG_SYS_MAX_I2C_BUS		TEGRA_I2C_NUM_CONTROLLERS
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/configs/whistler.h +0 -3
index 10e70d2..e083cbd 100644
@@ -26,10 +26,7 @@
 
 /* I2C */
 #define CONFIG_SYS_I2C_TEGRA
-#define CONFIG_SYS_I2C_INIT_BOARD
-#define CONFIG_SYS_I2C_SPEED		100000
 #define CONFIG_CMD_I2C
-#define CONFIG_SYS_I2C
 
 /* SD/MMC */
 #define CONFIG_MMC
include/tps6586x.h +2 -2
index 78ce428..eefc95f 100644
@@ -44,9 +44,9 @@ int tps6586x_adjust_sm0_sm1(int sm0_target, int sm1_target, int step, int rate,
  * Set up the TPS6586X I2C bus number. This will be used for all operations
  * on the device. This function must be called before using other functions.
  *
- * @param bus	I2C bus number containing the TPS6586X chip
+ * @param bus	I2C bus containing the TPS6586X chip
  * @return 0 (always succeeds)
  */
-int tps6586x_init(int bus);
+int tps6586x_init(struct udevice *bus);
 
 #endif	/* _TPS6586X_H_ */
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#15 lore raw

PATCH v4 11/11 dm: i2c: tegra: Convert to driver model

Hi Simon,

Quoted 10 lines · On Thu, 4 Dec 2014 21:21:30 -0700 Simon Glass <[email protected]> wrote:
drivers/i2c/tegra_i2c.c +7 -2
+static const struct dm_i2c_ops tegra_i2c_ops = {
+	.xfer		= tegra_i2c_xfer,
+	.probe_chip	= tegra_i2c_probe_chip,
+	.set_bus_speed	= tegra_i2c_set_bus_speed,
+};
 
-	for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; i++) {
-		struct i2c_bus *bus = &i2c_controllers[i];
+static int tegra_i2c_child_pre_probe(struct udevice *dev)
+{

drivers/i2c/tegra_i2c.c static int i2c_read_data(struct i2c_bus *bus, uchar chip, uchar *buffer, · line 495

Quoted 10 lines
drivers/i2c/tegra_i2c.c +6 -3
+	struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
 
-		if (bus->inited && bus->is_dvc)
-			return i;
-	}
+	if (dev->of_offset == -1)
+		return 0;
+	return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
+					   i2c_chip);
+}

As I said in v3, it is unfortunate that uclass does not support .child_pre_probe.

sandbox_i2c.c and tegra-i2c.c have a similar .child_pre_probe. I copied tegra_i2c_child_pre_probe() to my i2c driver verbatim.

I guess every driver will do so.

Is it a good idea to move .child_pre_probe to uclass_driver?

Best Regards
Masahiro Yamada

#16 lore raw

PATCH v4 11/11 dm: i2c: tegra: Convert to driver model

Hi Masahiro,

Quoted 32 lines · Masahiro Yamada

Hi Simon,

Quoted 10 lines · On Thu, 4 Dec 2014 21:21:30 -0700 Simon Glass <[email protected]> wrote:
drivers/i2c/tegra_i2c.c +7 -2
+static const struct dm_i2c_ops tegra_i2c_ops = {
+     .xfer           = tegra_i2c_xfer,
+     .probe_chip     = tegra_i2c_probe_chip,
+     .set_bus_speed  = tegra_i2c_set_bus_speed,
+};
 
-     for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; i++) {
-             struct i2c_bus *bus = &i2c_controllers[i];
+static int tegra_i2c_child_pre_probe(struct udevice *dev)
+{

drivers/i2c/tegra_i2c.c static int i2c_read_data(struct i2c_bus *bus, uchar chip, uchar *buffer, · line 495

Quoted 10 lines
drivers/i2c/tegra_i2c.c +6 -3
+     struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
 
-             if (bus->inited && bus->is_dvc)
-                     return i;
-     }
+     if (dev->of_offset == -1)
+             return 0;
+     return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
+                                        i2c_chip);
+}

As I said in v3, it is unfortunate that uclass does not support .child_pre_probe.

Quoted 7 lines

sandbox_i2c.c and tegra-i2c.c have a similar .child_pre_probe. I copied tegra_i2c_child_pre_probe() to my i2c driver verbatim.

I guess every driver will do so.

Is it a good idea to move .child_pre_probe to uclass_driver?

Sorry I didn't reply on this.

I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.

For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).

With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.

For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.

Let's revisit when I come back to PCI.

Regards,
Simon

#17 lore raw

PATCH v4 11/11 dm: i2c: tegra: Convert to driver model

Hi Simon,

Sorry for my late reply.

Quoted 15 lines · On Fri, 5 Dec 2014 07:50:33 -0700 Simon Glass <[email protected]> wrote:

Sorry I didn't reply on this.

I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.

For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).

Quoted 7 lines

With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.

For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.

Let's revisit when I come back to PCI.

I hope you will come back to here before it gets too late.

If we have lots of drivers converted to DM and some of them start to have various deviations, it will be difficult to merge the common parts.

Best Regards
Masahiro Yamada

#18 lore raw

PATCH v4 11/11 dm: i2c: tegra: Convert to driver model

Hi Masahiro,

Quoted 29 lines · Masahiro Yamada

Hi Simon,

Sorry for my late reply.

Quoted 21 lines · On Fri, 5 Dec 2014 07:50:33 -0700 Simon Glass <[email protected]> wrote:

Sorry I didn't reply on this.

I have been thinking about this for a while. For SPI we have the same problem and I think I mentioned it in the code somewhere. For the PCI RFC I have already adjusted the platdata lifecycle a little since I think it will simplify all buses, and have added a another data pointer for a different purpose.

For the case you raise the problem as you say is that each driver is required to provide a certain structure for its children so that its uclass is happy. On the other hand the driver may want to add additional information We already have a lot of flexibility so I was waiting to see how much this would really matter (there is after all a complexity trade-off).

With PCI I've made some changes and I think I'll end up adding another pointer. At that point I will also want to change SPI and I2C.

For now, I'd like to leave this alone. It does work and avoids adding new features at this late stage.

Quoted 8 lines
Quoted 1 line

Let's revisit when I come back to PCI.

I hope you will come back to here before it gets too late.

If we have lots of drivers converted to DM and some of them start to have various deviations, it will be difficult to merge the common parts.

First priority is to move everything to Kconfig. I sent an RFC.

Second I need to figure out the platdata size of things and the discussion above.

I am not worried about the task. So far it only affects I2C and SPI (and PCI RFC). There are only 3-4 drivers I think. It should be fine. The good news is that with this many uclasses we can be a lot more confident that the design is solid and can cover U-Boot's needs.

Regards,
Simon