| Message ID | 1352766871-892-4-git-send-email-marex@denx.de |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Heiko Schocher |
| Headers | show |
Dear Marek, In message <1352766871-892-4-git-send-email-marex@denx.de> you wrote: > This patch pulls out the I2C speed setup from the i2c_init() call > and implements the bus configuration lookup table with register > values that needs to be programmed into the I2C IP to run at > particular speed. > > This patch is a first step towards implementing run-time I2C bus > speed configuration for the MXS I2C IP. Thanks. > +static struct mxs_i2c_speed_table { > + uint32_t speed; > + uint32_t timing0; > + uint32_t timing1; > +} mxs_i2c_tbl[] = { > + { > + 100000, > + (0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) | > + (0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET), > + (0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) | > + (0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET) > + }, > + { > + 400000, > + (0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) | > + (0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET), > + (0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) | > + (0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET), > + } > +}; Do we really need such a compile-time initialized table which will have to include all possible I2C speeds anybody is ever going to use on any board? And if board XXX wants to use a funny I2C clock, we have to add yet another entry to this common file? Such a solution does not scale. Can we not rather calculate these register values for any arbitrary I2C clock given? Best regards, Wolfgang Denk
Dear Wolfgang Denk, > Dear Marek, > > In message <1352766871-892-4-git-send-email-marex@denx.de> you wrote: > > This patch pulls out the I2C speed setup from the i2c_init() call > > and implements the bus configuration lookup table with register > > values that needs to be programmed into the I2C IP to run at > > particular speed. > > > > This patch is a first step towards implementing run-time I2C bus > > speed configuration for the MXS I2C IP. > > Thanks. > > > +static struct mxs_i2c_speed_table { > > + uint32_t speed; > > + uint32_t timing0; > > + uint32_t timing1; > > +} mxs_i2c_tbl[] = { > > + { > > + 100000, > > + (0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) | > > + (0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET), > > + (0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) | > > + (0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET) > > + }, > > + { > > + 400000, > > + (0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) | > > + (0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET), > > + (0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) | > > + (0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET), > > + } > > +}; > > Do we really need such a compile-time initialized table which will > have to include all possible I2C speeds anybody is ever going to use > on any board? Yes > And if board XXX wants to use a funny I2C clock, we have to add yet > another entry to this common file? Such a solution does not scale. The problem is, the algorithm to compute these values is not described in the MX28 manual. There're only values for 100 and 400kHz speed in the manual. > Can we not rather calculate these register values for any arbitrary > I2C clock given? That's what I'd love to do ... no luck so far. That's why there is the crappy table. > Best regards, > > Wolfgang Denk Best regards, Marek Vasut
diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c index 2a193c2..98f6e8c 100644 --- a/drivers/i2c/mxs_i2c.c +++ b/drivers/i2c/mxs_i2c.c @@ -210,34 +210,51 @@ int i2c_probe(uchar chip) return ret; } +static struct mxs_i2c_speed_table { + uint32_t speed; + uint32_t timing0; + uint32_t timing1; +} mxs_i2c_tbl[] = { + { + 100000, + (0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) | + (0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET), + (0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) | + (0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET) + }, + { + 400000, + (0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) | + (0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET), + (0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) | + (0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET), + } +}; + +static struct mxs_i2c_speed_table *mxs_i2c_speed_to_cfg(uint32_t speed) +{ + int i; + for (i = 0; i < ARRAY_SIZE(mxs_i2c_tbl); i++) + if (mxs_i2c_tbl[i].speed == speed) + return &mxs_i2c_tbl[i]; + return NULL; +} + void i2c_init(int speed, int slaveadd) { struct mxs_i2c_regs *i2c_regs = (struct mxs_i2c_regs *)MXS_I2C0_BASE; + struct mxs_i2c_speed_table *spd = mxs_i2c_speed_to_cfg(speed); - mxs_i2c_reset(); - - switch (speed) { - case 100000: - writel((0x0078 << I2C_TIMING0_HIGH_COUNT_OFFSET) | - (0x0030 << I2C_TIMING0_RCV_COUNT_OFFSET), - &i2c_regs->hw_i2c_timing0); - writel((0x0080 << I2C_TIMING1_LOW_COUNT_OFFSET) | - (0x0030 << I2C_TIMING1_XMIT_COUNT_OFFSET), - &i2c_regs->hw_i2c_timing1); - break; - case 400000: - writel((0x000f << I2C_TIMING0_HIGH_COUNT_OFFSET) | - (0x0007 << I2C_TIMING0_RCV_COUNT_OFFSET), - &i2c_regs->hw_i2c_timing0); - writel((0x001f << I2C_TIMING1_LOW_COUNT_OFFSET) | - (0x000f << I2C_TIMING1_XMIT_COUNT_OFFSET), - &i2c_regs->hw_i2c_timing1); - break; - default: + if (!spd) { printf("MXS I2C: Invalid speed selected (%d Hz)\n", speed); return; } + mxs_i2c_reset(); + + writel(spd->timing0, &i2c_regs->hw_i2c_timing0); + writel(spd->timing1, &i2c_regs->hw_i2c_timing1); + writel((0x0015 << I2C_TIMING2_BUS_FREE_OFFSET) | (0x000d << I2C_TIMING2_LEADIN_COUNT_OFFSET), &i2c_regs->hw_i2c_timing2);
This patch pulls out the I2C speed setup from the i2c_init() call and implements the bus configuration lookup table with register values that needs to be programmed into the I2C IP to run at particular speed. This patch is a first step towards implementing run-time I2C bus speed configuration for the MXS I2C IP. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Stefano Babic <sbabic@denx.de> Cc: Fabio Estevam <fabio.estevam@freescale.com> --- drivers/i2c/mxs_i2c.c | 57 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 20 deletions(-)