On 7/4/19 9:09 AM, Ludwig Zenz wrote:

Marek Vasut marex@denx.de Wednesday 3rd July 2019 14:06:

...

On 7/3/19 10:20 AM, Ludwig Zenz wrote: [...]

...

static int setup_dhcom_mac_from_fuse(void) {

• struct udevice *dev; unsigned char enetaddr[6]; int ret;

@@ -228,13 +145,14 @@ static int setup_dhcom_mac_from_fuse(void) return 0; }

• ret = i2c_set_bus_num(2);

+#ifdef CONFIG_SYS_I2C_MXC_I2C3

• ret = i2c_get_chip_for_busnum(2, EEPROM_I2C_ADDRESS, 1, &dev);

Isn't there some DM way which avoids using bus sequence numbers ? If there is, subsequent patch is fine.

I searched the U-Boot source tree in particular also drivers/i2c/i2c-uclass.c and unfortunately found no better variant. Of course I am open for suggestions.

+CC Heiko, he's the i2c maintainer.

...

...

if (ret) {

• printf("Error switching I2C bus!\n");
  

• printf("Cannot find EEPROM!\n");
  

  return ret; }

• ret = i2c_read(EEPROM_I2C_ADDRESS, 0xfa, 0x1, enetaddr, 0x6);

• ret = dm_i2c_read(dev, 0xfa, enetaddr, 0x6); if (ret) { printf("Error reading configuration EEPROM!\n"); return ret;

[...]

...

diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig index 4734ed76e5..b0749a054f 100644 --- a/configs/dh_imx6_defconfig +++ b/configs/dh_imx6_defconfig @@ -48,6 +48,13 @@ CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_DWC_AHSATA=y CONFIG_BOOTCOUNT_LIMIT=y CONFIG_DM_GPIO=y +CONFIG_DM_I2C=y +CONFIG_I2C_SET_DEFAULT_BUS_NUM=y +CONFIG_I2C_DEFAULT_BUS_NUMBER=0x2

Are these two ^ needed ?

Bus number 2 is the i2c bus for the system on module components. The first thought was that it would add some comfort. On the other hand, of course, there is also additional overhead. I will leave it out in v2.

Surely the user should pick the correct bus, right ?

...

...

+CONFIG_SYS_I2C_MXC=y +CONFIG_SYS_I2C_MXC_I2C1=y +CONFIG_SYS_I2C_MXC_I2C2=y +CONFIG_SYS_I2C_MXC_I2C3=y

Are these three ^ really needed ?

You're right. It is not needed (I did test it). I was irritated by the fact that this is used on many boards.

Unfortunately I get the following warnings when compiling (maybe that is the reason why so many boards use it):

/work/dhcom/imx6/u-boot-imx/drivers/i2c/mxc_i2c.c:793:12: warning: ‘mxc_i2c_set_bus_speed’ defined but not used [-Wunused-function] static u32 mxc_i2c_set_bus_speed(struct i2c_adapter *adap, uint speed) ^~~~~~~~~~~~~~~~~~~~~ /work/dhcom/imx6/u-boot-imx/drivers/i2c/mxc_i2c.c:785:13: warning: ‘mxc_i2c_init’ defined but not used [-Wunused-function] static void mxc_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) ^~~~~~~~~~~~ /work/dhcom/imx6/u-boot-imx/drivers/i2c/mxc_i2c.c:721:12: warning: ‘mxc_i2c_probe’ defined but not used [-Wunused-function] static int mxc_i2c_probe(struct i2c_adapter *adap, uint8_t chip) ^~~~~~~~~~~~~ /work/dhcom/imx6/u-boot-imx/drivers/i2c/mxc_i2c.c:711:12: warning: ‘mxc_i2c_write’ defined but not used [-Wunused-function] static int mxc_i2c_write(struct i2c_adapter *adap, uint8_t chip, ^~~~~~~~~~~~~ /work/dhcom/imx6/u-boot-imx/drivers/i2c/mxc_i2c.c:704:12: warning: ‘mxc_i2c_read’ defined but not used [-Wunused-function] static int mxc_i2c_read(struct i2c_adapter *adap, uint8_t chip,

with CONFIG_SYS_I2C_MXC_I2Cx in the dh_imx6_defconfig everything is fine.

Can you make a suggestion on how to solve it?

Is DM_I2C defined ? Those ^ functions are defined if !CONFIG_DM_I2C according to the driver source.

[...]