On Wed, Apr 18, 2012 at 09:33:19AM +0200, Stefan Roese wrote: > The STM SPEAr platform can only access the i2c controller register > via 16bit read/write functions. This patch adds support to > automatically detect this 16bit access mode. > > Signed-off-by: Stefan Roese <[email protected]>
Thanks for the update, looks mostly good. > Cc: Wolfram Sang <[email protected]> > Cc: Viresh Kumar <[email protected]> > --- > v2: > - Removed parenthesis for single-statement block > - Moved "swab" and "access_16bit" into "accessor_flags" > > drivers/i2c/busses/i2c-designware-core.c | 27 ++++++++++++++++++++++----- > drivers/i2c/busses/i2c-designware-core.h | 5 ++++- > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c > index df87992..b3c5cfa 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -164,9 +164,15 @@ static char *abort_sources[] = { > > u32 dw_readl(struct dw_i2c_dev *dev, int offset) > { > - u32 value = readl(dev->base + offset); > + u32 value; > > - if (dev->swab) > + if (dev->accessor_flags & ACCESS_16BIT) > + value = readw(dev->base + offset) | > + (readw(dev->base + offset + 2) << 16); > + else > + value = readl(dev->base + offset); > + > + if (dev->accessor_flags & ACCESS_SWAP) > return swab32(value); > else > return value; > @@ -174,10 +180,15 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset) > > void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset) > { > - if (dev->swab) > + if (dev->accessor_flags & ACCESS_SWAP) > b = swab32(b); > > - writel(b, dev->base + offset); > + if (dev->accessor_flags & ACCESS_16BIT) { > + writew((u16)b, dev->base + offset); > + writew((u16)(b >> 16), dev->base + offset + 2); > + } else { > + writel(b, dev->base + offset); > + } > } > > static u32 > @@ -254,7 +265,13 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > /* Configure register endianess access */ > reg = dw_readl(dev, DW_IC_COMP_TYPE); > if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) { > - dev->swab = 1; > + dev->accessor_flags |= ACCESS_SWAP; > + reg = DW_IC_COMP_TYPE_VALUE; May I ask you to use proper if/elseif/else blocks instead of overwriting reg? I know you didn't come up with the mechanism, yet it looks too fragile to be extended IMO. > + } > + > + /* Configure register access mode 16bit */ > + if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) { Does it make sense to check reg + 2 for the upper part of the signature? > + dev->accessor_flags |= ACCESS_16BIT; > reg = DW_IC_COMP_TYPE_VALUE; > } > > diff --git a/drivers/i2c/busses/i2c-designware-core.h > b/drivers/i2c/busses/i2c-designware-core.h > index 02d1a2d..9c1840e 100644 > --- a/drivers/i2c/busses/i2c-designware-core.h > +++ b/drivers/i2c/busses/i2c-designware-core.h > @@ -82,7 +82,7 @@ struct dw_i2c_dev { > unsigned int status; > u32 abort_source; > int irq; > - int swab; > + u32 accessor_flags; > struct i2c_adapter adapter; > u32 functionality; > u32 master_cfg; > @@ -90,6 +90,9 @@ struct dw_i2c_dev { > unsigned int rx_fifo_depth; > }; > > +#define ACCESS_SWAP 0x00000001 > +#define ACCESS_16BIT 0x00000002 > + > extern u32 dw_readl(struct dw_i2c_dev *dev, int offset); > extern void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset); > extern int i2c_dw_init(struct dw_i2c_dev *dev); > -- > 1.7.10 > Thanks, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
