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

[Ludwig Zenz]

On 7/4/19 5:20 PM, Marek Vasut wrote:
> On 7/4/19 9:09 AM, Ludwig Zenz wrote:
> > 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.
> 
> 
> +CC Heiko, he's the i2c maintainer.

I've invested some more time. Please see my patch V2.

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

I agree. The user must know what he is doing and choose the appropriate bus.

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

Thanks to your hint I came to the conclusion that the SPL build generates the warnings (no DM and no I2C use). 
Solved by deactivating I2C in SPL.

Thank you.
Best regards,
Ludwig Zenz