On Thu, Jan 02, 2014 at 01:28:22PM -0500, Jason Cooper wrote:
> Wolfram,
> 
> On Thu, Jan 02, 2014 at 05:01:16PM +0100, Gregory CLEMENT wrote:
> > The first variants of Armada XP SoCs (A0 stepping) have issues related
> > to the i2c controller which prevent to use the offload mechanism and
> > lead to a kernel hang during boot.
> > 
> > The driver now check the revision of the SoC. If the revision is not
> > more recent than the A0 or if the driver can't get the SoC revision
> > then it disables the offload mechanism.
> > 
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Gregory CLEMENT <gregory.clem...@free-electrons.com>
> > ---
> >  drivers/i2c/busses/i2c-mv64xxx.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-mv64xxx.c 
> > b/drivers/i2c/busses/i2c-mv64xxx.c
> > index 8be7e42aa4de..089a3663ad86 100644
> > --- a/drivers/i2c/busses/i2c-mv64xxx.c
> > +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/clk.h>
> >  #include <linux/err.h>
> >  #include <linux/delay.h>
> > +#include <linux/mvebu-soc-id.h>
> >  
> >  #define MV64XXX_I2C_ADDR_ADDR(val)                 ((val & 0x7f) << 1)
> >  #define MV64XXX_I2C_BAUD_DIV_N(val)                        (val & 0x7)
> > @@ -779,8 +780,16 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
> >      * Transaction Generator support and the errata fix.
> >      */
> >     if (of_device_is_compatible(np, "marvell,mv78230-i2c")) {
> > -           drv_data->offload_enabled = true;
> > +           u32 dev, rev;
> > +
> >             drv_data->errata_delay = true;
> > +           /*
> > +            * Only revison more recent than A0 support offload
> > +            * mechanism. In case we can't get the SoC revision
> > +            * weplay safe and we don't enable it
> > +            */
> > +           if (!mvebu_get_soc_id(&rev, &dev) && (dev > MV78XX0_A0_REV))

Very minor nits:

I'd prefer (mvebu_get_soc_id == 0) here, since !mvebu_get_soc_id can
easily be read as "if not get soc id" which leads to the assumption the
function failed. And the parantheses around the second comparison are
superfluous.

> > +                   drv_data->offload_enabled = true;
> 
> Since this depends on arch-specific code in the previous patch, I'd like
> to keep the two of them together in a topic branch.  Would you prefer to
> take both with my Ack, or vice-versa?  I'm fine either way.

I'd think you better take it:

Acked-by: Wolfram Sang <w...@the-dreams.de>

Attachment: signature.asc
Description: Digital signature

Reply via email to