[U-Boot] [PATCH 1/2] i2c: mvtwsi: disable i2c slave on Armada 38x

[Stefan Roese]

On 29.05.2018 19:55, Baruch Siach wrote:
> Hi Stefan,
> 
> On Tue, May 29, 2018 at 08:11:55AM +0200, Stefan Roese wrote:
>> On 29.05.2018 06:45, Baruch Siach wrote:
>>> Equivalent code that disables the hidden i2c0 slave already exists in
>>> the Turris Omnia platform specific code. But this hidden i2c0 slave that
>>> interferes the i2c bus is not board specific. Armada 38x SoCs and at
>>> least some Kirkwood variants are affected as well. Add code to disable
>>> this slave to the i2c bus driver to make it work on all affected
>>> hardware.
>>>
>>> Use the bind callback because we want this to always run at boot,
>>> regardless of whether U-Boot uses the i2c bus.
>>>
>>> Cc: Rabeeh Khoury <rabeeh at solid-run.com>
>>> Cc: Chris Packham <judge.packham at gmail.com>
>>> Reviewed-by: Stefan Roese <sr at denx.de>
>>> Reviewed-by: Heiko Schocher <hs at denx.de>
>>> Signed-off-by: Baruch Siach <baruch at tkos.co.il>
>>> ---
> 
> [snip]
> 
>>> +static int mvtwsi_i2c_bind(struct udevice *bus)
>>> +{
>>> +	struct mvtwsi_registers *twsi = devfdt_get_addr_ptr(bus);
>>> +
>>> +	/* Disable the hidden slave in i2c0 of these platforms */
>>> +	if ((IS_ENABLED(CONFIG_ARMADA_38X) || IS_ENABLED(CONFIG_KIRKWOOD))
>>
>> We could better use the compatible check here:
>>
>> 	if (device_is_compatible(dev, "marvell,mv64xxx-i2c"))
> 
> This is not an equivalent check. marvell,mv64xxx-i2c covers other SoCs that
> might not be affected.

I only see Marvell Kirkwood and Armada XP / A38x boards using this
compatible property.

> Furthermore, this makes a build time test into a run time one. This bloats the
> code for platforms like Allwinner that are unlikely to be affected.

Yes, this is a disadvantage.

> What is the advantage of device_is_compatible()? Is it feasible to build a
> multi-platform U-Boot image?

The main advantage is, that multiple platforms can be supported
in one single image. This is how its done in Linux. I agree, that
this is not so common in U-Boot, e.g. to support Orion and Kirkwood
in one single U-Boot image. So lets keep it this way and apply
this patch version if nobody else objects.

Thanks,
Stefan