Patchwork [U-Boot,4/7] i2c: mxs: Abstract out the MXS I2C speed setup

login
register
mail settings
Submitter Marek Vasut
Date Nov. 13, 2012, 12:34 a.m.
Message ID <1352766871-892-4-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/198514/
State Awaiting Upstream
Delegated to: Heiko Schocher
Headers show

Comments

Marek Vasut - Nov. 13, 2012, 12:34 a.m.
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(-)
Wolfgang Denk - Nov. 13, 2012, 8:29 a.m.
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
Marek Vasut - Nov. 13, 2012, 1:45 p.m.
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

Patch

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);