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

[Baruch Siach]

Hi Stefan,

On Thu, Jun 07, 2018 at 08:53:34AM +0200, Stefan Roese wrote:
> On 06.06.2018 16:05, 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>
> > ---
> > v3:
> >    * Fix build for SUNXI (Heiko Schocher)
> > 
> > v2:
> >    * Use clrbits_le32 (Stefan Roese)
> > 
> >    * Apply to Kirkwood (Chris Packham)
> > 
> >    * Add review tags from Stefan and Heiko
> > ---
> >   drivers/i2c/mvtwsi.c | 25 ++++++++++++++++++++++++-
> >   1 file changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> > index f9822e56b894..c4f387992b5d 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,25 @@ static int mvtwsi_i2c_ofdata_to_platdata(struct udevice *bus)
> >   	return 0;
> >   }
> > +static void twsi_disable_i2c_slave(struct mvtwsi_registers *twsi)
> > +{
> > +#if (defined(CONFIG_ARMADA_38X) || defined(CONFIG_KIRKWOOD))
> > +	clrbits_le32(&twsi->debug, BIT(18));
> > +#endif
> > +}
> 
> I would prefer this version:
> 
> #if (defined(CONFIG_ARMADA_38X) || defined(CONFIG_KIRKWOOD))
> static void twsi_disable_i2c_slave(struct mvtwsi_registers *twsi)
> {
> 	clrbits_le32(&twsi->debug, BIT(18));
> }
> #else
> static inline void twsi_disable_i2c_slave(struct mvtwsi_registers *twsi)
> {
> }
> #endif

Should I drop the IS_ENABLED condition from the bind routine, then?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -