i2c: imx: use struct representing i2c clk{div, val} pair
PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair
using struct representing the i2c clk{div, val} pair would make the i2c_clk_div array more clear.
--- drivers/i2c/busses/i2c-imx.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-imx.c +13 -7
@@ -30,6 +30,8 @@* Copyright (C) 2007 RightHand Technologies, Inc. * Copyright (C) 2008 Darius Augulis <[email protected]> * +* Copyright 2013 Freescale Semiconductor, Inc. +* */ /** Includes *******************************************************************
@@ -95,8 +97,12 @@* * Duplicated divider values removed from list */ +struct imx_i2c_clk_pair { +u16 div; +u16 val; +}; -static u16 __initdata i2c_clk_div[50 ][2 ] = { +static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = { {22 ,0x20 }, {24 ,0x21 }, {26 ,0x22 }, {28 ,0x23 }, {30 ,0x00 }, {32 ,0x24 }, {36 ,0x25 }, {40 ,0x26 }, {42 ,0x03 }, {44 ,0x27 }, {48 ,0x28 }, {52 ,0x05 },
@@ -274,15 +280,15 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,/* Divider value calculation */ i2c_clk_rate =clk_get_rate (i2c_imx->clk); div = (i2c_clk_rate + rate -1 ) / rate; -if (div < i2c_clk_div[0 ][0 ]) +if (div < i2c_clk_div[0 ].div) i =0 ; -else if (div > i2c_clk_div[ARRAY_SIZE (i2c_clk_div) -1 ][0 ]) +else if (div > i2c_clk_div[ARRAY_SIZE (i2c_clk_div) -1 ].div) i =ARRAY_SIZE (i2c_clk_div) -1 ;else -for (i =0 ; i2c_clk_div[i][0 ] < div; i++); +for (i =0 ; i2c_clk_div[i].div < div; i++);/* Store divider value */ - i2c_imx->ifdr = i2c_clk_div[i][1 ]; + i2c_imx->ifdr = i2c_clk_div[i].val;/* * There dummy delay is calculated.
@@ -290,7 +296,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,* This delay is used in I2C bus disable function * to fix chip hardware bug. */ - i2c_imx->disable_delay = (500000U * i2c_clk_div[i][0 ] + i2c_imx->disable_delay = (500000U * i2c_clk_div[i].div + (i2c_clk_rate /2 ) -1 ) / (i2c_clk_rate /2 );/* dev_dbg() can't be used, because adapter is not yet registered */
@@ -298,7 +304,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,dev_dbg (&i2c_imx->adapter.dev,"<%s> I2C_CLK=%d, REQ DIV=%d\n" , __func__, i2c_clk_rate, div);dev_dbg (&i2c_imx->adapter.dev,"<%s> IFDR[IC]=0x%x, REAL DIV=%d\n" , - __func__, i2c_clk_div[i][1 ], i2c_clk_div[i][0 ]); + __func__, i2c_clk_div[i].val, i2c_clk_div[i].div);#endif }
Signature · 1 line
1.8.0
PATCH 2/8 i2c: imx: enable clk before write to registers
The module clk should be enabled before write to its register in probe(), and the clk can be disabled after.
--- drivers/i2c/busses/i2c-imx.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/i2c/busses/i2c-imx.c +6 -0
@@ -543,6 +543,11 @@ static int __init i2c_imx_probe(struct platform_device *pdev)return PTR_ERR (i2c_imx->clk); } + ret =clk_prepare_enable (i2c_imx->clk); +if (ret) { +dev_err (&pdev->dev,"can't enable I2C clock\n" ); +return ret; + }/* Request IRQ */ ret =devm_request_irq (&pdev->dev, irq, i2c_imx_isr,0 , pdev->name, i2c_imx);
@@ -580,6 +585,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)/* Set up platform driver data */ platform_set_drvdata (pdev, i2c_imx); +clk_disable_unprepare (i2c_imx->clk);dev_dbg (&i2c_imx->adapter.dev,"claimed irq %d\n" , irq); dev_dbg(&i2c_imx->adapter.dev, "device resources from 0x%x to 0x%x\n",
Signature · 1 line
1.8.0
PATCH 3/8 i2c: imx: don't change platform device id_entry directly
In reply to Jingchang LuThe id_entry field should be changed by platform driver core, driver should prevent changing it derectly. Use local variable to save and extract platform_device_id info of the dts devices instead.
--- drivers/i2c/busses/i2c-imx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-imx.c +6 -2
@@ -499,6 +499,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)struct imx_i2c_struct *i2c_imx;struct resource *res;struct imxi2c_platform_data *pdata = pdev->dev.platform_data; +const struct platform_device_id *imx_id;void __iomem *base;int irq, ret;u32 bitrate;
@@ -524,8 +525,11 @@ static int __init i2c_imx_probe(struct platform_device *pdev) }if (of_id) - pdev->id_entry = of_id->data; - i2c_imx->devtype = pdev->id_entry->driver_data; + imx_id = of_id->data; +else + imx_id =platform_get_device_id (pdev); + + i2c_imx->devtype = imx_id->driver_data;/* Setup i2c_imx driver structure */ strlcpy (i2c_imx->adapter.name, pdev->name,sizeof (i2c_imx->adapter.name));
Signature · 1 line
1.8.0
PATCH 4/8 i2c: imx: wrap registers read/write to inline function
In reply to Jingchang Luwrap the readb(), writeb() into inline function calls. It would make the driver more clearer to support platform with different register offset.
--- drivers/i2c/busses/i2c-imx.c | 80 +++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 34 deletions(-)
drivers/i2c/busses/i2c-imx.c +46 -34
@@ -160,6 +160,18 @@ static inline int is_imx1_i2c(struct imx_i2c_struct *i2c_imx)return i2c_imx->devtype ==IMX1_I2C ; } +static inline void imx_i2c_write_reg (unsigned int val, +struct imx_i2c_struct *i2c_imx,unsigned int reg) +{ +writeb (val, i2c_imx->base + reg); +} + +static inline unsigned char imx_i2c_read_reg (struct imx_i2c_struct *i2c_imx, +unsigned int reg) +{ +return readb (i2c_imx->base + reg); +} +/** Functions for IMX I2C adapter driver *************************************** *******************************************************************************/
@@ -171,7 +183,7 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)dev_dbg (&i2c_imx->adapter.dev,"<%s>\n" , __func__);while (1 ) { - temp =readb (i2c_imx->base +IMX_I2C_I2SR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2SR );if (for_busy && (temp &I2SR_IBB ))break ;if (!for_busy && !(temp &I2SR_IBB ))
@@ -202,7 +214,7 @@ static int i2c_imx_trx_complete(struct imx_i2c_struct *i2c_imx)static int i2c_imx_acked (struct imx_i2c_struct *i2c_imx) { -if (readb (i2c_imx->base +IMX_I2C_I2SR ) &I2SR_RXAK ) { +if (imx_i2c_read_reg (i2c_imx,IMX_I2C_I2SR ) &I2SR_RXAK ) {dev_dbg (&i2c_imx->adapter.dev,"<%s> No ACK\n" , __func__);return -EIO ;/* No ACK */ }
@@ -219,25 +231,25 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)dev_dbg (&i2c_imx->adapter.dev,"<%s>\n" , __func__);clk_prepare_enable (i2c_imx->clk); -writeb (i2c_imx->ifdr, i2c_imx->base +IMX_I2C_IFDR ); +imx_i2c_write_reg (i2c_imx->ifdr, i2c_imx,IMX_I2C_IFDR );/* Enable I2C controller */ -writeb (0 , i2c_imx->base +IMX_I2C_I2SR ); -writeb (I2CR_IEN , i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2SR ); +imx_i2c_write_reg (I2CR_IEN , i2c_imx,IMX_I2C_I2CR );/* Wait controller to be stable */ udelay (50 );/* Start I2C transaction */ - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp |=I2CR_MSTA ; -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR ); result =i2c_imx_bus_busy (i2c_imx,1 );if (result)return result; i2c_imx->stopped =0 ; temp |=I2CR_IIEN |I2CR_MTX |I2CR_TXAK ; -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR );return result; }
@@ -248,9 +260,9 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)if (!i2c_imx->stopped) {/* Stop I2C transaction */ dev_dbg (&i2c_imx->adapter.dev,"<%s>\n" , __func__); - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp &= ~(I2CR_MSTA |I2CR_MTX ); -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR ); }if (is_imx1_i2c (i2c_imx)) {/*
@@ -266,7 +278,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) }/* Disable I2C controller */ -writeb (0 , i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2CR );clk_disable_unprepare (i2c_imx->clk); }
@@ -313,12 +325,12 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)struct imx_i2c_struct *i2c_imx = dev_id;unsigned int temp; - temp =readb (i2c_imx->base +IMX_I2C_I2SR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2SR );if (temp &I2SR_IIF ) {/* save status register */ i2c_imx->i2csr = temp; temp &= ~I2SR_IIF ; -writeb (temp, i2c_imx->base +IMX_I2C_I2SR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2SR );wake_up (&i2c_imx->queue);return IRQ_HANDLED ; }
@@ -334,7 +346,7 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs) __func__, msgs->addr << 1);/* write slave address */ -writeb (msgs->addr <<1 , i2c_imx->base +IMX_I2C_I2DR ); +imx_i2c_write_reg (msgs->addr <<1 , i2c_imx,IMX_I2C_I2DR ); result =i2c_imx_trx_complete (i2c_imx);if (result)return result;
@@ -348,7 +360,7 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)dev_dbg (&i2c_imx->adapter.dev,"<%s> write byte: B%d=0x%X\n" , __func__, i, msgs->buf[i]); -writeb (msgs->buf[i], i2c_imx->base +IMX_I2C_I2DR ); +imx_i2c_write_reg (msgs->buf[i], i2c_imx,IMX_I2C_I2DR ); result =i2c_imx_trx_complete (i2c_imx);if (result)return result;
@@ -369,7 +381,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs) __func__, (msgs->addr << 1) | 0x01);/* write slave address */ -writeb ((msgs->addr <<1 ) |0x01 , i2c_imx->base +IMX_I2C_I2DR ); +imx_i2c_write_reg ((msgs->addr <<1 ) |0x01 , i2c_imx,IMX_I2C_I2DR ); result =i2c_imx_trx_complete (i2c_imx);if (result)return result;
@@ -380,12 +392,12 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)dev_dbg (&i2c_imx->adapter.dev,"<%s> setup bus\n" , __func__);/* setup bus to read data */ - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp &= ~I2CR_MTX ;if (msgs->len -1 ) temp &= ~I2CR_TXAK ; -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); -readb (i2c_imx->base +IMX_I2C_I2DR );/* dummy read */ +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR ); +imx_i2c_read_reg (i2c_imx,IMX_I2C_I2DR );/* dummy read */ dev_dbg (&i2c_imx->adapter.dev,"<%s> read data\n" , __func__);
@@ -399,19 +411,19 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)controller from generating another clock cycle */ dev_dbg (&i2c_imx->adapter.dev,"<%s> clear MSTA\n" , __func__); - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp &= ~(I2CR_MSTA |I2CR_MTX ); -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR );i2c_imx_bus_busy (i2c_imx,0 ); i2c_imx->stopped =1 ; }else if (i == (msgs->len -2 )) {dev_dbg (&i2c_imx->adapter.dev,"<%s> set TXAK\n" , __func__); - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp |=I2CR_TXAK ; -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR ); } - msgs->buf[i] =readb (i2c_imx->base +IMX_I2C_I2DR ); + msgs->buf[i] =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2DR );dev_dbg (&i2c_imx->adapter.dev,"<%s> read byte: B%d=0x%X\n" , __func__, i, msgs->buf[i]);
@@ -438,9 +450,9 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,if (i) {dev_dbg (&i2c_imx->adapter.dev,"<%s> repeated start\n" , __func__); - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR ); temp |=I2CR_RSTA ; -writeb (temp, i2c_imx->base +IMX_I2C_I2CR ); +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR ); result =i2c_imx_bus_busy (i2c_imx,1 );if (result) goto fail0;
@@ -449,13 +461,13 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter, "<%s> transfer message: %d\n", __func__, i);/* write/read data */ #ifdef CONFIG_I2C_DEBUG_BUS - temp =readb (i2c_imx->base +IMX_I2C_I2CR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2CR );dev_dbg (&i2c_imx->adapter.dev,"<%s> CONTROL: IEN=%d, IIEN=%d, " "MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n" , __func__, (temp &I2CR_IEN ?1 :0 ), (temp &I2CR_IIEN ?1 :0 ), (temp &I2CR_MSTA ?1 :0 ), (temp &I2CR_MTX ?1 :0 ), (temp &I2CR_TXAK ?1 :0 ), (temp &I2CR_RSTA ?1 :0 )); - temp =readb (i2c_imx->base +IMX_I2C_I2SR ); + temp =imx_i2c_read_reg (i2c_imx,IMX_I2C_I2SR ); dev_dbg(&i2c_imx->adapter.dev, "<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, " "IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
@@ -575,8 +587,8 @@ static int __init i2c_imx_probe(struct platform_device *pdev)i2c_imx_set_clk (i2c_imx, bitrate);/* Set up chip registers to defaults */ -writeb (0 , i2c_imx->base +IMX_I2C_I2CR ); -writeb (0 , i2c_imx->base +IMX_I2C_I2SR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2SR );/* Add I2C adapter */ ret =i2c_add_numbered_adapter (&i2c_imx->adapter);
@@ -612,10 +624,10 @@ static int __exit i2c_imx_remove(struct platform_device *pdev)i2c_del_adapter (&i2c_imx->adapter);/* setup chip registers to defaults */ -writeb (0 , i2c_imx->base +IMX_I2C_IADR ); -writeb (0 , i2c_imx->base +IMX_I2C_IFDR ); -writeb (0 , i2c_imx->base +IMX_I2C_I2CR ); -writeb (0 , i2c_imx->base +IMX_I2C_I2SR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_IADR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_IFDR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2SR );return 0 ; }
Signature · 1 line
1.8.0
PATCH 5/8 i2c: imx: change register offset representation
In reply to Jingchang Lu the I2C register offset may different between SoCs,
to provid support for all these chips, split the register offset into a fixed base address and a variable shift value, then the full register offset will be calculated byreg_off = ( reg_base_addr << reg_shift)
--- drivers/i2c/busses/i2c-imx.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-imx.c +16 -7
@@ -64,12 +64,21 @@/* Default value */ #define IMX_I2C_BIT_RATE 100000/* 100kHz */ -/* IMX I2C registers */ +/* IMX I2C registers: +* the I2C register offset is different between SoCs, +* to provid support for all these chips, split the +* register offset into a fixed base address and a +* variable shift value, then the full register offset +* will be calculated by +* reg_off = ( reg_base_addr << reg_shift) +*/ #define IMX_I2C_IADR 0x00/* i2c slave address */ -#define IMX_I2C_IFDR 0x04/* i2c frequency divider */ -#define IMX_I2C_I2CR 0x08/* i2c control */ -#define IMX_I2C_I2SR 0x0C/* i2c status */ -#define IMX_I2C_I2DR 0x10/* i2c transfer data */ +#define IMX_I2C_IFDR 0x01/* i2c frequency divider */ +#define IMX_I2C_I2CR 0x02/* i2c control */ +#define IMX_I2C_I2SR 0x03/* i2c status */ +#define IMX_I2C_I2DR 0x04/* i2c transfer data */ + +#define IMX_I2C_REGSHIFT 2/* Bits of IMX I2C registers */ #define I2SR_RXAK 0x01
@@ -163,13 +172,13 @@ static inline int is_imx1_i2c(struct imx_i2c_struct *i2c_imx)static inline void imx_i2c_write_reg (unsigned int val,struct imx_i2c_struct *i2c_imx,unsigned int reg) { -writeb (val, i2c_imx->base + reg); +writeb (val, i2c_imx->base + (reg <<IMX_I2C_REGSHIFT )); }static inline unsigned char imx_i2c_read_reg (struct imx_i2c_struct *i2c_imx,unsigned int reg) { -return readb (i2c_imx->base + reg); +return readb (i2c_imx->base + (reg <<IMX_I2C_REGSHIFT )); }/** Functions for IMX I2C adapter driver ***************************************
Signature · 1 line
1.8.0
PATCH 6/8 i2c: imx: add INT flag and IEN bit operatation codes
In reply to Jingchang LuThis add bits operation macro that differ between SoCs. Interrupt flags clear operation in I2SR differ between SoCs:
write zero to clear(w0c) INT flag on i.MX,
but write one to clear(w1c) INT flag on Vybrid.
I2C module enable operation in I2CR also differ between SoCs:
set I2CR_IEN bit enable the module on i.MX,
but clear I2CR_IEN bit enable the module on Vybrid.
--- drivers/i2c/busses/i2c-imx.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-imx.c +22 -5
@@ -95,6 +95,22 @@#define I2CR_IIEN 0x40#define I2CR_IEN 0x80 +/* register bits different operating codes definition: +* 1) I2SR: Interrupt flags clear operation differ between SoCs: +* - write zero to clear(w0c) INT flag on i.MX, +* - but write one to clear(w1c) INT flag on Vybrid. +* 2) I2CR: I2C module enable operation also differ between SoCs: +* - set I2CR_IEN bit enable the module on i.MX, +* - but clear I2CR_IEN bit enable the module on Vybrid. +*/ +#define I2SR_CLR_OPCODE_W0C 0x0 +#define I2SR_CLR_OPCODE_W1C (I2SR_IAL | I2SR_IIF) +#define I2CR_IEN_OPCODE_0 0x0 +#define I2CR_IEN_OPCODE_1 I2CR_IEN + +#define IMX_I2SR_CLR_OPCODE I2SR_CLR_OPCODE_W0C +#define IMX_I2CR_IEN_OPCODE I2CR_IEN_OPCODE_1 +/** Variables ****************************************************************** *******************************************************************************/
@@ -242,8 +258,8 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)clk_prepare_enable (i2c_imx->clk);imx_i2c_write_reg (i2c_imx->ifdr, i2c_imx,IMX_I2C_IFDR );/* Enable I2C controller */ -imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2SR ); -imx_i2c_write_reg (I2CR_IEN , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (IMX_I2SR_CLR_OPCODE , i2c_imx,IMX_I2C_I2SR ); +imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE , i2c_imx,IMX_I2C_I2CR );/* Wait controller to be stable */ udelay (50 );
@@ -287,7 +303,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) }/* Disable I2C controller */ -imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE ^I2CR_IEN , i2c_imx,IMX_I2C_I2CR );clk_disable_unprepare (i2c_imx->clk); }
@@ -339,6 +355,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)/* save status register */ i2c_imx->i2csr = temp; temp &= ~I2SR_IIF ; + temp |= (IMX_I2SR_CLR_OPCODE &I2SR_IIF );imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2SR );wake_up (&i2c_imx->queue);return IRQ_HANDLED ;
@@ -596,8 +613,8 @@ static int __init i2c_imx_probe(struct platform_device *pdev)i2c_imx_set_clk (i2c_imx, bitrate);/* Set up chip registers to defaults */ -imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2CR ); -imx_i2c_write_reg (0 , i2c_imx,IMX_I2C_I2SR ); +imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE ^I2CR_IEN , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (IMX_I2SR_CLR_OPCODE , i2c_imx,IMX_I2C_I2SR );/* Add I2C adapter */ ret =i2c_add_numbered_adapter (&i2c_imx->adapter);
Signature · 1 line
1.8.0
PATCH 7/8 i2c: imx: add struct to hold more configurable quirks
In reply to Jingchang LuThis add struct imx_i2c_hwdata to hold more quirks data which may vary between SoCs, thus the driver can operate on more differences to support more SoCs.
--- drivers/i2c/busses/i2c-imx.c | 77 ++++++++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 25 deletions(-)
drivers/i2c/busses/i2c-imx.c +52 -25
@@ -108,9 +108,6 @@#define I2CR_IEN_OPCODE_0 0x0#define I2CR_IEN_OPCODE_1 I2CR_IEN -#define IMX_I2SR_CLR_OPCODE I2SR_CLR_OPCODE_W0C -#define IMX_I2CR_IEN_OPCODE I2CR_IEN_OPCODE_1 -/** Variables ****************************************************************** *******************************************************************************/
@@ -127,7 +124,7 @@ struct imx_i2c_clk_pair {u16 val; }; -static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = { +static struct imx_i2c_clk_pair imx_i2c_clk_div[] = { {22 ,0x20 }, {24 ,0x21 }, {26 ,0x22 }, {28 ,0x23 }, {30 ,0x00 }, {32 ,0x24 }, {36 ,0x25 }, {40 ,0x26 }, {42 ,0x03 }, {44 ,0x27 }, {48 ,0x28 }, {52 ,0x05 },
@@ -148,6 +145,15 @@ enum imx_i2c_type { IMX21_I2C, }; +struct imx_i2c_hwdata { +enum imx_i2c_type devtype; +unsigned regshift; +struct imx_i2c_clk_pair *clk_div; +unsigned ndivs; +unsigned i2sr_clr_opcode; +unsigned i2cr_ien_opcode; +}; +struct imx_i2c_struct {struct i2c_adapter adapter;struct clk *clk;
@@ -157,16 +163,36 @@ struct imx_i2c_struct {unsigned int disable_delay;int stopped;unsigned int ifdr;/* IMX_I2C_IFDR */ -enum imx_i2c_type devtype; +const struct imx_i2c_hwdata *hwdata; +}; + +static const struct imx_i2c_hwdata imx1_i2c_hwdata = { + .devtype =IMX1_I2C , + .regshift =IMX_I2C_REGSHIFT , + .clk_div = imx_i2c_clk_div, + .ndivs =ARRAY_SIZE (imx_i2c_clk_div), + .i2sr_clr_opcode =I2SR_CLR_OPCODE_W0C , + .i2cr_ien_opcode =I2CR_IEN_OPCODE_1 , + +}; + +static const struct imx_i2c_hwdata imx21_i2c_hwdata = { + .devtype =IMX21_I2C , + .regshift =IMX_I2C_REGSHIFT , + .clk_div = imx_i2c_clk_div, + .ndivs =ARRAY_SIZE (imx_i2c_clk_div), + .i2sr_clr_opcode =I2SR_CLR_OPCODE_W0C , + .i2cr_ien_opcode =I2CR_IEN_OPCODE_1 , + };static struct platform_device_id imx_i2c_devtype[] = { { .name ="imx1-i2c" , - .driver_data =IMX1_I2C , + .driver_data = (kernel_ulong_t)&imx1_i2c_hwdata, }, { .name ="imx21-i2c" , - .driver_data =IMX21_I2C , + .driver_data = (kernel_ulong_t)&imx21_i2c_hwdata, }, {/* sentinel */ }
@@ -174,27 +200,27 @@ static struct platform_device_id imx_i2c_devtype[] = {MODULE_DEVICE_TABLE (platform, imx_i2c_devtype);static const struct of_device_id i2c_imx_dt_ids[] = { - { .compatible ="fsl,imx1-i2c" , .data = &imx_i2c_devtype[IMX1_I2C ], }, - { .compatible ="fsl,imx21-i2c" , .data = &imx_i2c_devtype[IMX21_I2C ], }, + { .compatible ="fsl,imx1-i2c" , .data = &imx1_i2c_hwdata, }, + { .compatible ="fsl,imx21-i2c" , .data = &imx21_i2c_hwdata, }, {/* sentinel */ } };MODULE_DEVICE_TABLE (of, i2c_imx_dt_ids);static inline int is_imx1_i2c (struct imx_i2c_struct *i2c_imx) { -return i2c_imx->devtype ==IMX1_I2C ; +return i2c_imx->hwdata->devtype ==IMX1_I2C ; }static inline void imx_i2c_write_reg (unsigned int val,struct imx_i2c_struct *i2c_imx,unsigned int reg) { -writeb (val, i2c_imx->base + (reg <<IMX_I2C_REGSHIFT )); +writeb (val, i2c_imx->base + (reg << i2c_imx->hwdata->regshift)); }static inline unsigned char imx_i2c_read_reg (struct imx_i2c_struct *i2c_imx,unsigned int reg) { -return readb (i2c_imx->base + (reg <<IMX_I2C_REGSHIFT )); +return readb (i2c_imx->base + (reg << i2c_imx->hwdata->regshift)); }/** Functions for IMX I2C adapter driver ***************************************
@@ -258,8 +284,8 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)clk_prepare_enable (i2c_imx->clk);imx_i2c_write_reg (i2c_imx->ifdr, i2c_imx,IMX_I2C_IFDR );/* Enable I2C controller */ -imx_i2c_write_reg (IMX_I2SR_CLR_OPCODE , i2c_imx,IMX_I2C_I2SR ); -imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE , i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,IMX_I2C_I2SR ); +imx_i2c_write_reg (i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx,IMX_I2C_I2CR );/* Wait controller to be stable */ udelay (50 );
@@ -303,13 +329,15 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) }/* Disable I2C controller */ -imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE ^I2CR_IEN , i2c_imx,IMX_I2C_I2CR ); + temp = i2c_imx->hwdata->i2cr_ien_opcode ^I2CR_IEN , +imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2CR );clk_disable_unprepare (i2c_imx->clk); }static void __init i2c_imx_set_clk (struct imx_i2c_struct *i2c_imx,unsigned int rate) { +struct imx_i2c_clk_pair *i2c_clk_div = i2c_imx->hwdata->clk_div;unsigned int i2c_clk_rate;unsigned int div;int i;
@@ -319,8 +347,8 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, div = (i2c_clk_rate + rate -1 ) / rate;if (div < i2c_clk_div[0 ].div) i =0 ; -else if (div > i2c_clk_div[ARRAY_SIZE (i2c_clk_div) -1 ].div) - i =ARRAY_SIZE (i2c_clk_div) -1 ; +else if (div > i2c_clk_div[i2c_imx->hwdata->ndivs -1 ].div) + i = i2c_imx->hwdata->ndivs -1 ;else for (i =0 ; i2c_clk_div[i].div < div; i++);
@@ -355,7 +383,7 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)/* save status register */ i2c_imx->i2csr = temp; temp &= ~I2SR_IIF ; - temp |= (IMX_I2SR_CLR_OPCODE &I2SR_IIF ); + temp |= (i2c_imx->hwdata->i2sr_clr_opcode &I2SR_IIF );imx_i2c_write_reg (temp, i2c_imx,IMX_I2C_I2SR );wake_up (&i2c_imx->queue);return IRQ_HANDLED ;
@@ -537,7 +565,6 @@ static int __init i2c_imx_probe(struct platform_device *pdev)struct imx_i2c_struct *i2c_imx;struct resource *res;struct imxi2c_platform_data *pdata = pdev->dev.platform_data; -const struct platform_device_id *imx_id;void __iomem *base;int irq, ret;u32 bitrate;
@@ -563,11 +590,10 @@ static int __init i2c_imx_probe(struct platform_device *pdev) }if (of_id) - imx_id = of_id->data; + i2c_imx->hwdata = of_id->data;else - imx_id =platform_get_device_id (pdev); - - i2c_imx->devtype = imx_id->driver_data; + i2c_imx->hwdata = (struct imx_i2c_hwdata *) +platform_get_device_id (pdev)->driver_data;/* Setup i2c_imx driver structure */ strlcpy (i2c_imx->adapter.name, pdev->name,sizeof (i2c_imx->adapter.name));
@@ -613,8 +639,9 @@ static int __init i2c_imx_probe(struct platform_device *pdev)i2c_imx_set_clk (i2c_imx, bitrate);/* Set up chip registers to defaults */ -imx_i2c_write_reg (IMX_I2CR_IEN_OPCODE ^I2CR_IEN , i2c_imx,IMX_I2C_I2CR ); -imx_i2c_write_reg (IMX_I2SR_CLR_OPCODE , i2c_imx,IMX_I2C_I2SR ); +imx_i2c_write_reg (i2c_imx->hwdata->i2cr_ien_opcode ^I2CR_IEN , + i2c_imx,IMX_I2C_I2CR ); +imx_i2c_write_reg (i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,IMX_I2C_I2SR );/* Add I2C adapter */ ret =i2c_add_numbered_adapter (&i2c_imx->adapter);
Signature · 1 line
1.8.0
PATCH 8/8 i2c: imx: Add Vybrid VF610 I2C controller support
In reply to Jingchang LuAdd Freescale Vybrid VF610 I2C controller support to imx I2C driver framework.
--- drivers/i2c/busses/i2c-imx.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
drivers/i2c/busses/i2c-imx.c +32 -0
@@ -79,6 +79,7 @@#define IMX_I2C_I2DR 0x04/* i2c transfer data */ #define IMX_I2C_REGSHIFT 2 +#define VF610_I2C_REGSHIFT 0/* Bits of IMX I2C registers */ #define I2SR_RXAK 0x01
@@ -140,9 +141,29 @@ static struct imx_i2c_clk_pair imx_i2c_clk_div[] = { { 3072, 0x1E }, { 3840, 0x1F } }; +/* Vybrid VF610 clock divider, register value pairs */ +static struct imx_i2c_clk_pair vf610_i2c_clk_div[] = { + {20 ,0x00 }, {22 ,0x01 }, {24 ,0x02 }, {26 ,0x03 }, + {28 ,0x04 }, {30 ,0x05 }, {32 ,0x09 }, {34 ,0x06 }, + {36 ,0x0A }, {40 ,0x07 }, {44 ,0x0C }, {48 ,0x0D }, + {52 ,0x43 }, {56 ,0x0E }, {60 ,0x45 }, {64 ,0x12 }, + {68 ,0x0F }, {72 ,0x13 }, {80 ,0x14 }, {88 ,0x15 }, + {96 ,0x19 }, {104 ,0x16 }, {112 ,0x1A }, {128 ,0x17 }, + {136 ,0x4F }, {144 ,0x1C }, {160 ,0x1D }, {176 ,0x55 }, + {192 ,0x1E }, {208 ,0x56 }, {224 ,0x22 }, {228 ,0x24 }, + {240 ,0x1F }, {256 ,0x23 }, {288 ,0x5C }, {320 ,0x25 }, + {384 ,0x26 }, {448 ,0x2A }, {480 ,0x27 }, {512 ,0x2B }, + {576 ,0x2C }, {640 ,0x2D }, {768 ,0x31 }, {896 ,0x32 }, + {960 ,0x2F }, {1024 ,0x33 }, {1152 ,0x34 }, {1280 ,0x35 }, + {1536 ,0x36 }, {1792 ,0x3A }, {1920 ,0x37 }, {2048 ,0x3B }, + {2304 ,0x3C }, {2560 ,0x3D }, {3072 ,0x3E }, {3584 ,0x7A }, + {3840 ,0x3F }, {4096 ,0x7B }, {5120 ,0x7D }, {6144 ,0x7E }, +}; +enum imx_i2c_type {IMX1_I2C ,IMX21_I2C , +VF610_I2C , };struct imx_i2c_hwdata {
@@ -186,6 +207,16 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata = { }; +static struct imx_i2c_hwdata vf610_i2c_hwdata = { + .devtype =VF610_I2C , + .regshift =VF610_I2C_REGSHIFT , + .clk_div = vf610_i2c_clk_div, + .ndivs =ARRAY_SIZE (vf610_i2c_clk_div), + .i2sr_clr_opcode =I2SR_CLR_OPCODE_W1C , + .i2cr_ien_opcode =I2CR_IEN_OPCODE_0 , + +}; +static struct platform_device_id imx_i2c_devtype[] = { { .name ="imx1-i2c" ,
@@ -202,6 +233,7 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);static const struct of_device_id i2c_imx_dt_ids[] = { { .compatible ="fsl,imx1-i2c" , .data = &imx1_i2c_hwdata, }, { .compatible ="fsl,imx21-i2c" , .data = &imx21_i2c_hwdata, }, + { .compatible ="fsl,vf610-i2c" , .data = &vf610_i2c_hwdata, }, {/* sentinel */ } };MODULE_DEVICE_TABLE (of, i2c_imx_dt_ids);
Signature · 1 line
1.8.0
Sascha Hauer
maintainerPATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair
In reply to Jingchang LuNice improvement of this series.
There are some unnecessary blank lines in the definitions in 7/8 and 8/8, but otherwise:
Sascha
Quoted 78 lines · Jingchang Lu
using struct representing the i2c clk{div, val} pair would make the
i2c_clk_divarray more clear.Signed-off-by Jingchang Lu <[email protected]>--- drivers/i2c/busses/i2c-imx.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)drivers/i2c/busses/i2c-imx.c +13
-7@@ -30,6 +30,8 @@* Copyright (C) 2007 RightHand Technologies, Inc. * Copyright (C) 2008 Darius Augulis <[email protected]> * +* Copyright 2013 Freescale Semiconductor, Inc. +* */ /** Includes ******************************************************************* @@ -95,8 +97,12 @@* * Duplicated divider values removed from list */ +struct imx_i2c_clk_pair { +u16 div; +u16 val; +}; -static u16 __initdata i2c_clk_div[50 ][2 ] = { +static struct imx_i2c_clk_pair __initdata i2c_clk_div[] = { {22 ,0x20 }, {24 ,0x21 }, {26 ,0x22 }, {28 ,0x23 }, {30 ,0x00 }, {32 ,0x24 }, {36 ,0x25 }, {40 ,0x26 }, {42 ,0x03 }, {44 ,0x27 }, {48 ,0x28 }, {52 ,0x05 },@@ -274,15 +280,15 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,/* Divider value calculation */ i2c_clk_rate =clk_get_rate (i2c_imx->clk); div = (i2c_clk_rate + rate -1 ) / rate; -if (div < i2c_clk_div[0 ][0 ]) +if (div < i2c_clk_div[0 ].div) i =0 ; -else if (div > i2c_clk_div[ARRAY_SIZE (i2c_clk_div) -1 ][0 ]) +else if (div > i2c_clk_div[ARRAY_SIZE (i2c_clk_div) -1 ].div) i =ARRAY_SIZE (i2c_clk_div) -1 ;else -for (i =0 ; i2c_clk_div[i][0 ] < div; i++); +for (i =0 ; i2c_clk_div[i].div < div; i++);/* Store divider value */ - i2c_imx->ifdr = i2c_clk_div[i][1 ]; + i2c_imx->ifdr = i2c_clk_div[i].val;/* * There dummy delay is calculated. @@ -290,7 +296,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,* This delay is used in I2C bus disable function * to fix chip hardware bug. */ - i2c_imx->disable_delay = (500000U * i2c_clk_div[i][0 ] + i2c_imx->disable_delay = (500000U * i2c_clk_div[i].div + (i2c_clk_rate /2 ) -1 ) / (i2c_clk_rate /2 );/* dev_dbg() can't be used, because adapter is not yet registered */ @@ -298,7 +304,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,dev_dbg (&i2c_imx->adapter.dev,"<%s> I2C_CLK=%d, REQ DIV=%d\n" , __func__, i2c_clk_rate, div);dev_dbg (&i2c_imx->adapter.dev,"<%s> IFDR[IC]=0x%x, REAL DIV=%d\n" , - __func__, i2c_clk_div[i][1 ], i2c_clk_div[i][0 ]); + __func__, i2c_clk_div[i].val, i2c_clk_div[i].div);#endif }Signature · 1 line
1.8.0
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
PATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair
Quoted 9 lines
-----Original Message-----
From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
Sent: Thursday, August 08, 2013 1:54 AM
To: Lu Jingchang-B35083
Cc: wsa at the-dreams.de; shawn.guo at linaro.org; Estevam Fabio-R49496; linux- i2c at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 1/8] i2c: imx: use struct representing i2c clk{div, val} pair
Quoted 6 lines
Nice improvement of this series.
There are some unnecessary blank lines in the definitions in 7/8 and 8/8, but otherwise:
Reviewed-by Sascha Hauer <[email protected]>
[Lu Jingchang-B35083]
Thank you very much for your review.
I leave one blank lines between definitions for readability, could you please help to comment the unnecessary blank lines on that patches so that I can remove them and send out the next version, thanks.
Best Regards,
Jingchang
Wolfram Sang
AppliedPATCH 1/8 i2c: imx: use struct representing i2c clk{div, val} pair
In reply to Jingchang LuQuoted 4 lines · Jingchang Lu
using struct representing the i2c clk{div, val} pair would make the
i2c_clk_divarray more clear.Signed-off-by Jingchang Lu <[email protected]>
Applied series to for-next, thanks! Please use --cover-letter next time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130815/5ff52d7b/attachment.sig>