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

[Stefan Roese]

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>
> ---
> v2:
>    * Use clrbits_le32 (Stefan Roese)
> 
>    * Apply to Kirkwood (Chris Packham)
> 
>    * Add review tags from Stefan and Heiko

Please add the patch revision also to the patch subject next time:

[PATCH 1/2 v2] i2c: mvtwsi: disable i2c slave on Armada 38x

One more comment below...

> ---
>   drivers/i2c/mvtwsi.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index f9822e56b894..c0d9a71627db 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -11,6 +11,7 @@
>   #include <i2c.h>
>   #include <linux/errno.h>
>   #include <asm/io.h>
> +#include <linux/bitops.h>
>   #include <linux/compat.h>
>   #ifdef CONFIG_DM_I2C
>   #include <dm.h>
> @@ -70,8 +71,10 @@ struct  mvtwsi_registers {
>   		u32 baudrate;	/* When writing */
>   	};
>   	u32 xtnd_slave_addr;
> -	u32 reserved[2];
> +	u32 reserved0[2];
>   	u32 soft_reset;
> +	u32 reserved1[27];
> +	u32 debug;
>   };
>   
>   #endif
> @@ -795,6 +798,23 @@ static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
>   	return 0;
>   }
>   
> +static void twsi_disable_i2c_slave(struct mvtwsi_registers *twsi)
> +{
> +	clrbits_le32(&twsi->debug, BIT(18));
> +}
> +
> +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"))

> +			&& bus->req_seq == 0)
> +		twsi_disable_i2c_slave(twsi);
> +
> +	return 0;
> +}
> +
>   static int mvtwsi_i2c_probe(struct udevice *bus)
>   {
>   	struct mvtwsi_i2c_dev *dev = dev_get_priv(bus);
> @@ -850,6 +870,7 @@ U_BOOT_DRIVER(i2c_mvtwsi) = {
>   	.name = "i2c_mvtwsi",
>   	.id = UCLASS_I2C,
>   	.of_match = mvtwsi_i2c_ids,
> +	.bind = mvtwsi_i2c_bind,
>   	.probe = mvtwsi_i2c_probe,
>   	.ofdata_to_platdata = mvtwsi_i2c_ofdata_to_platdata,
>   	.priv_auto_alloc_size = sizeof(struct mvtwsi_i2c_dev),
> 

Thanks,
Stefan