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

[Marek Vasut]

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.

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

[...]