Re: [PATCH 2/2] i2c-ocore: support 16 and 32-bit wide registers

["Jayachandran C." <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>]

On Tue, May 08, 2012 at 10:28:34PM +0530, Shubhrajyoti Datta wrote:
> Hi,
> 
> On Tue, May 8, 2012 at 6:55 PM, Jayachandran C
> <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org> wrote:
> > From: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >
> > Some architectures supports only 16-bit or 32-bit read/write access
> > to their iospace. Add a 'regwidth' platform and OF parameter which
> > specifies the IO width to support these platforms.
> >
> > regwidth can be specified as 1, 2 or 4, and has a default value
> > of 1 if it is unspecified.
> >
> > Signed-off-by: Ganesan Ramalingam <ganesanr-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Jayachandran C <jayachandranc-oSioyQM9ZPnuBjGU1YDckgC/G2K4zDHf@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-ocores.c |   32 ++++++++++++++++++++++++++++++--
> >  include/linux/i2c-ocores.h      |    1 +
> >  2 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> > index ebd2700..1313145 100644
> > --- a/drivers/i2c/busses/i2c-ocores.c
> > +++ b/drivers/i2c/busses/i2c-ocores.c
> > @@ -60,6 +64,7 @@
> >  struct ocores_i2c {
> >        void __iomem *base;
> >        int regstep;
> > +       int regwidth;
> 
> Do we need it to be signed?

regstep and regwidth can be both unsigned, but since regstep is already 
defined as int, we just followed that convention.

> 
> >        wait_queue_head_t wait;
> >        struct i2c_adapter adap;
> >        struct i2c_msg *msg;
> > @@ -102,12 +107,22 @@ struct ocores_i2c {
> >
> >  static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
> >  {
> > -       iowrite8(value, i2c->base + reg * i2c->regstep);
> > +       if (i2c->regwidth == 4)
> > +               iowrite32(value, i2c->base + reg * i2c->regstep);
> > +       else if (i2c->regwidth == 2)
> > +               iowrite16(value, i2c->base + reg * i2c->regstep);
> > +       else
> > +               iowrite8(value, i2c->base + reg * i2c->regstep);
> >  }
> >
> >  static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
>
> Shouldnt the return type also change?
>

No, the registers have only 8bit of data, and we do a 16/32 bit read and
return the lowest 8 bits.

Regards,
JC.