[U-Boot] [PATCH] ARM: imx6: DHCOM i.MX6 PDK: Switch to DM for I2C

[Ludwig Zenz]

Marek Vasut <marex at 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. 

> >  	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.

> > +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?

> [...]
> 

Thanks and regards,
Ludwig Zenz