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 <sr@xxxxxxx>
Thanks for the update, looks mostly good.
> Cc: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
> Cc: Viresh Kumar <viresh.kumar@xxxxxx>
> ---
> 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/ |
Attachment:
signature.asc
Description: Digital signature
