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

[Chris Packham]

On Wed, May 30, 2018 at 5:55 AM Baruch Siach <baruch at tkos.co.il> 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 agree with Baruch on this. The CONFIG_ARMADA_38X/CONFIG_KIRKWOOD singles
out platforms that we know have this feature and work. Using the compatible
string Is likely to flush out platforms that this hasn't been tested on
and/or where it doesn't work.

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

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

> > > +                   && bus->req_seq == 0)
> > > +           twsi_disable_i2c_slave(twsi);
> > > +
> > > +   return 0;
> > > +}

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