Add support for setting the I2C bus speed in fsl_i2c.c

PATCH New Timur Tabi 6 participants 16 messages Mar 14, 2008 – Apr 14, 2008 I2C

Share & tools

Timur Tabi

author

2008-03-14 22:45 UTC #0 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.

Signed-off-by Timur Tabi <[email protected]>

---

This patch is for U-Boot 1.3.3. It affects 83xx, 85xx, and 86xx.

 drivers/i2c/fsl_i2c.c |  114 +++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 105 insertions(+), 9 deletions(-)

drivers/i2c/fsl_i2c.c +105 -9

index 22485ea..dde0571 100644
@@ -32,6 +32,8 @@
 #define I2C_READ_BIT  1
 #define I2C_WRITE_BIT 0
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* Initialize the bus pointer to whatever one the SPD EEPROM is on.
  * Default is bus 0.  This is necessary because the DDR initialization
  * runs from ROM, and we can't switch buses because we can't modify

@@ -43,24 +45,111 @@ static unsigned int i2c_bus_num __attribute__ ((section ("data"))) = CFG_SPD_BUS
 static unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
 #endif
 
-static volatile struct fsl_i2c *i2c_dev[2] = {
+static unsigned int i2c_bus_speed[2] = {CFG_I2C_SPEED, CFG_I2C_SPEED};
+
+static const struct fsl_i2c *i2c_dev[2] = {
 	(struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET),
 #ifdef CFG_I2C2_OFFSET
 	(struct fsl_i2c *) (CFG_IMMR + CFG_I2C2_OFFSET)
 #endif
 };
 
+/* I2C speed map for a DFSR value of 1 */
+
+/*
+ * Map I2C frequency dividers to FDR and DFSR values
+ *
+ * This structure is used to define the elements of a table that maps I2C
+ * frequency divider (I2C clock rate divided by I2C bus speed) to a value to be
+ * programmed into the Frequency Divider Ratio (FDR) and Digital Filter
+ * Sampling Rate (DFSR) registers.
+ *
+ * The actual table should be defined in the board file, and it must be called
+ * fsl_i2c_speed_map[].
+ *
+ * The last entry of the table must have a value of {-1, X}, where X is same
+ * FDR/DFSR values as the second-to-last entry.  This guarantees that any
+ * search through the array will always find a match.
+ *
+ * The values of the divider must be in increasing numerical order, i.e.
+ * fsl_i2c_speed_map[x+1].divider > fsl_i2c_speed_map[x].divider.
+ *
+ * For this table, the values are based on a value of 1 for the DFSR
+ * register.  See the application note AN2919 "Determining the I2C Frequency
+ * Divider Ratio for SCL"
+ */
+static const struct {
+	unsigned short divider;
+	u8 dfsr;
+	u8 fdr;
+} fsl_i2c_speed_map[] = {
+	{160, 1, 32}, {192, 1, 33}, {224, 1, 34}, {256, 1, 35},
+	{288, 1, 0}, {320, 1, 1}, {352, 6, 1}, {384, 1, 2}, {416, 6, 2},
+	{448, 1, 38}, {480, 1, 3}, {512, 1, 39}, {544, 11, 3}, {576, 1, 4},
+	{608, 22, 3}, {640, 1, 5}, {672, 32, 3}, {704, 11, 5}, {736, 43, 3},
+	{768, 1, 6}, {800, 54, 3}, {832, 11, 6}, {896, 1, 42}, {960, 1, 7},
+	{1024, 1, 43}, {1088, 22, 7}, {1152, 1, 8}, {1216, 43, 7}, {1280, 1, 9},
+	{1408, 22, 9}, {1536, 1, 10}, {1664, 22, 10}, {1792, 1, 46},
+	{1920, 1, 11}, {2048, 1, 47}, {2176, 43, 11}, {2304, 1, 12},
+	{2560, 1, 13}, {2816, 43, 13}, {3072, 1, 14}, {3328, 43, 14},
+	{3584, 1, 50}, {3840, 1, 15}, {4096, 1, 51}, {4608, 1, 16},
+	{5120, 1, 17}, {6144, 1, 18}, {7168, 1, 54}, {7680, 1, 19},
+	{8192, 1, 55}, {9216, 1, 20}, {10240, 1, 21}, {12288, 1, 22},
+	{14336, 1, 58}, {15360, 1, 23}, {16384, 1, 59}, {18432, 1, 24},
+	{20480, 1, 25}, {24576, 1, 26}, {28672, 1, 62}, {30720, 1, 27},
+	{32768, 1, 63}, {36864, 1, 28}, {40960, 1, 29}, {49152, 1, 30},
+	{61440, 1, 31}, {-1, 1, 31}
+};
+
+/**
+ * Set the I2C bus speed for a given I2C device
+ *
+ * @param dev: the I2C device
+ * @i2c_clk: I2C bus clock frequency
+ * @speed: the desired speed of the bus
+ *
+ * The I2C device must be stopped before calling this function.
+ *
+ * The return value is the actual bus speed that is set.
+ */
+static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
+	unsigned int i2c_clk, unsigned int speed)
+{
+	unsigned short divider = min(i2c_clk / speed, (unsigned short) -1);
+	unsigned int i;
+	u8 fdr, dfsr;
+
+	/*
+	 * We want to choose an FDR/DFSR that generates an I2C bus speed that
+	 * is equal to or lower than the requested speed.  That means that we
+	 * want the first divider that is equal to or greater than the
+	 * calculated divider.
+	 */
+
+	for (i = 0; i < ARRAY_SIZE(fsl_i2c_speed_map); i++)
+		if (fsl_i2c_speed_map[i].divider >= divider) {
+			dfsr = fsl_i2c_speed_map[i].dfsr;
+			fdr = fsl_i2c_speed_map[i].fdr;
+			speed = i2c_clk / fsl_i2c_speed_map[i].divider;
+			break;
+		}
+
+	writeb(fdr, &dev->fdr);			/* set bus speed */
+	writeb(dfsr, &dev->dfsrr);		/* set default filter */
+
+	return speed;
+}
+
 void
 i2c_init(int speed, int slaveadd)
 {
-	volatile struct fsl_i2c *dev;
+	struct fsl_i2c *dev;
 
 	dev = (struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET);
 
 	writeb(0, &dev->cr);			/* stop I2C controller */
 	udelay(5);				/* let it shutdown in peace */
-	writeb(0x3F, &dev->fdr);		/* set bus speed */
-	writeb(0x3F, &dev->dfsrr);		/* set default filter */
+	i2c_bus_speed[0] = set_i2c_bus_speed(dev, gd->i2c1_clk, speed);
 	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
 	writeb(0x0, &dev->sr);			/* clear status register */
 	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */

@@ -70,12 +159,11 @@ i2c_init(int speed, int slaveadd)
 
 	writeb(0, &dev->cr);			/* stop I2C controller */
 	udelay(5);				/* let it shutdown in peace */
-	writeb(0x3F, &dev->fdr);		/* set bus speed */
-	writeb(0x3F, &dev->dfsrr);		/* set default filter */
+	i2c_bus_speed[1] = set_i2c_bus_speed(dev, gd->i2c2_clk, speed);
 	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
 	writeb(0x0, &dev->sr);			/* clear status register */
 	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
-#endif	/* CFG_I2C2_OFFSET */
+#endif
 }
 
 static __inline__ int

@@ -279,7 +367,14 @@ int i2c_set_bus_num(unsigned int bus)
 
 int i2c_set_bus_speed(unsigned int speed)
 {
-	return -1;
+	unsigned int i2c_clk = (i2c_bus_num == 1) ? gd->i2c2_clk : gd->i2c1_clk;
+
+	writeb(0, &i2c_dev[i2c_bus_num]->cr);		/* stop controller */
+	i2c_bus_speed[i2c_bus_num] =
+		set_i2c_bus_speed(i2c_dev[i2c_bus_num], i2c_clk, speed);
+	writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);	/* start controller */
+
+	return 0;
 }
 
 unsigned int i2c_get_bus_num(void)

@@ -289,7 +384,8 @@ unsigned int i2c_get_bus_num(void)
 
 unsigned int i2c_get_bus_speed(void)
 {
-	return 0;
+	return i2c_bus_speed[i2c_bus_num];
 }
+
 #endif /* CONFIG_HARD_I2C */
 #endif /* CONFIG_FSL_I2C */

Signature · 1 line

1.5.4

Jean-Christophe PLAGNIOL-VILLARD

2008-03-15 07:35 UTC #1 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 5 lines · Timur Tabi

Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.

Quoted 7 lines

Signed-off-by Timur Tabi <[email protected]>
---
This patch is for U-Boot 1.3.3. It affects 83xx, 85xx, and 86xx.
#endif /* CONFIG_HARD_I2C */
#endif /* CONFIG_FSL_I2C */

Could you btw move this to the Makefile

Best Regards,
J.

Timur Tabi

author

2008-03-15 21:27 UTC #2 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 4 lines
This patch is for U-Boot 1.3.3. It affects 83xx, 85xx, and 86xx.
#endif /* CONFIG_HARD_I2C */
#endif /* CONFIG_FSL_I2C */
Could you btw move this to the Makefile

If I did, then I'd be doing two completely different things in one patch. I understand the need to update the code to the new config model, but sliming that change into other patches is probably not acceptable.

What does everyone think?

Signature · 2 lines

Timur Tabi
Linux Kernel Developer @ Freescale

Timur Tabi

author

2008-03-26 15:39 UTC #3 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

In reply to Timur Tabi

Quoted 4 lines · Timur Tabi

Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.

Kim and Jon,

I tested this patch on 8360, 8568, and 8641. Could you review this patch and ack/nack? Andy said he'll pick it up if you guys ack it.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Andy Fleming

2008-03-26 16:36 UTC #4 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 10 lines · Timur Tabi

Quoted 4 lines · Timur Tabi
Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.
Kim and Jon,
I tested this patch on 8360, 8568, and 8641. Could you review this patch and ack/nack? Andy said he'll pick it up if you guys ack it.

Actually, looking at it, it's not really in my domain. It's a driver.
I think Wolfgang is in charge of accepting a patch to drivers/. So
I'll just Ack. I can pull it in if that's more convenient for Wolfgang, of course.

Acked-by Andy Fleming <[email protected]>

Wolfgang Denk

2008-03-26 23:12 UTC #5 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

In reply to Timur Tabi

Quoted 6 lines · In message <[email protected]> you wrote:

Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.
Signed-off-by Timur Tabi <[email protected]>

Applied with Andy's ACK.

I have to admit that I don't like the impact on the memory footprint of the driver.

And to make sure I understand this correctly: is it correct to assume that this driver has never been tested with environment in a EEPROM?

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Nothing ever becomes real until it is experienced.       - John Keats

Timur Tabi

author

2008-03-27 14:33 UTC #6 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 2 lines · Wolfgang Denk

I have to admit that I don't like the impact on the memory footprint of the driver.

I can't think of any other way to support programmable I2C bus speeds. Even if we did something like this in the board header files:

#define CFG_FSL_I2C_DFSR 43
#define CFG_FSL_I2C_FDR 3

We still would not be able to support the "i2c speed" command.

I did my best to make fsl_i2c_speed_map[] as small as possible. It should be smaller than 300 bytes.

Besides, this code is for Freescale's high-end SOCs. Memory footprint is critical.

Quoted 2 lines

And to make sure I understand this correctly: is it correct to assume that this driver has never been tested with environment in a EEPROM?

Do you mean the fsl_i2c.c driver itself, or the driver with my patch? I don't know if the driver itself has been tested, but I did not test my driver with any major data in the EEPROM. If it turns out that the new values for the I2C bus speed registers are too aggressive, then they can be tuned. Prior to this patch, the values couldn't be tuned without hacking common code.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Luigi 'Comio' Mantellini

2008-03-27 09:11 UTC #7 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

In reply to Timur Tabi

This patch seems incompatible with cf547x/548x cpus because the cpu specific global data doesn't define the i2c1_clk / i2c2_clk attributes.

Best regards,

luigi

Quoted 174 lines · Timur Tabi

Signed-off-by Timur Tabi <[email protected]>

---

This patch is for U-Boot 1.3.3. It affects 83xx, 85xx, and 86xx.

 drivers/i2c/fsl_i2c.c |  114 +++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 105 insertions(+), 9 deletions(-)

drivers/i2c/fsl_i2c.c +103 -8

index 22485ea..dde0571 100644
@@ -32,6 +32,8 @@
 #define I2C_READ_BIT  1
 #define I2C_WRITE_BIT 0
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* Initialize the bus pointer to whatever one the SPD EEPROM is on.
  * Default is bus 0.  This is necessary because the DDR initialization
  * runs from ROM, and we can't switch buses because we can't modify

@@ -43,24 +45,111 @@ static unsigned int i2c_bus_num __attribute__ ((section ("data"))) = CFG_SPD_BUS
 static unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
 #endif
 
-static volatile struct fsl_i2c *i2c_dev[2] = {
+static unsigned int i2c_bus_speed[2] = {CFG_I2C_SPEED, CFG_I2C_SPEED};
+
+static const struct fsl_i2c *i2c_dev[2] = {
 	(struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET),
 #ifdef CFG_I2C2_OFFSET
 	(struct fsl_i2c *) (CFG_IMMR + CFG_I2C2_OFFSET)
 #endif
 };
 
+/* I2C speed map for a DFSR value of 1 */
+
+/*
+ * Map I2C frequency dividers to FDR and DFSR values
+ *
+ * This structure is used to define the elements of a table that maps I2C
+ * frequency divider (I2C clock rate divided by I2C bus speed) to a value to be
+ * programmed into the Frequency Divider Ratio (FDR) and Digital Filter
+ * Sampling Rate (DFSR) registers.
+ *
+ * The actual table should be defined in the board file, and it must be called
+ * fsl_i2c_speed_map[].
+ *
+ * The last entry of the table must have a value of {-1, X}, where X is same
+ * FDR/DFSR values as the second-to-last entry.  This guarantees that any
+ * search through the array will always find a match.
+ *
+ * The values of the divider must be in increasing numerical order, i.e.
+ * fsl_i2c_speed_map[x+1].divider > fsl_i2c_speed_map[x].divider.
+ *
+ * For this table, the values are based on a value of 1 for the DFSR
+ * register.  See the application note AN2919 "Determining the I2C Frequency
+ * Divider Ratio for SCL"
+ */
+static const struct {
+	unsigned short divider;
+	u8 dfsr;
+	u8 fdr;
+} fsl_i2c_speed_map[] = {
+	{160, 1, 32}, {192, 1, 33}, {224, 1, 34}, {256, 1, 35},
+	{288, 1, 0}, {320, 1, 1}, {352, 6, 1}, {384, 1, 2}, {416, 6, 2},
+	{448, 1, 38}, {480, 1, 3}, {512, 1, 39}, {544, 11, 3}, {576, 1, 4},
+	{608, 22, 3}, {640, 1, 5}, {672, 32, 3}, {704, 11, 5}, {736, 43, 3},
+	{768, 1, 6}, {800, 54, 3}, {832, 11, 6}, {896, 1, 42}, {960, 1, 7},
+	{1024, 1, 43}, {1088, 22, 7}, {1152, 1, 8}, {1216, 43, 7}, {1280, 1, 9},
+	{1408, 22, 9}, {1536, 1, 10}, {1664, 22, 10}, {1792, 1, 46},
+	{1920, 1, 11}, {2048, 1, 47}, {2176, 43, 11}, {2304, 1, 12},
+	{2560, 1, 13}, {2816, 43, 13}, {3072, 1, 14}, {3328, 43, 14},
+	{3584, 1, 50}, {3840, 1, 15}, {4096, 1, 51}, {4608, 1, 16},
+	{5120, 1, 17}, {6144, 1, 18}, {7168, 1, 54}, {7680, 1, 19},
+	{8192, 1, 55}, {9216, 1, 20}, {10240, 1, 21}, {12288, 1, 22},
+	{14336, 1, 58}, {15360, 1, 23}, {16384, 1, 59}, {18432, 1, 24},
+	{20480, 1, 25}, {24576, 1, 26}, {28672, 1, 62}, {30720, 1, 27},
+	{32768, 1, 63}, {36864, 1, 28}, {40960, 1, 29}, {49152, 1, 30},
+	{61440, 1, 31}, {-1, 1, 31}
+};
+
+/**
+ * Set the I2C bus speed for a given I2C device
+ *
+ * @param dev: the I2C device
+ * @i2c_clk: I2C bus clock frequency
+ * @speed: the desired speed of the bus
+ *
+ * The I2C device must be stopped before calling this function.
+ *
+ * The return value is the actual bus speed that is set.
+ */
+static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
+	unsigned int i2c_clk, unsigned int speed)
+{
+	unsigned short divider = min(i2c_clk / speed, (unsigned short) -1);
+	unsigned int i;
+	u8 fdr, dfsr;
+
+	/*
+	 * We want to choose an FDR/DFSR that generates an I2C bus speed that
+	 * is equal to or lower than the requested speed.  That means that we
+	 * want the first divider that is equal to or greater than the
+	 * calculated divider.
+	 */
+
+	for (i = 0; i < ARRAY_SIZE(fsl_i2c_speed_map); i++)
+		if (fsl_i2c_speed_map[i].divider >= divider) {
+			dfsr = fsl_i2c_speed_map[i].dfsr;
+			fdr = fsl_i2c_speed_map[i].fdr;
+			speed = i2c_clk / fsl_i2c_speed_map[i].divider;
+			break;
+		}
+
+	writeb(fdr, &dev->fdr);			/* set bus speed */
+	writeb(dfsr, &dev->dfsrr);		/* set default filter */
+
+	return speed;
+}
+
 void
 i2c_init(int speed, int slaveadd)
 {
-	volatile struct fsl_i2c *dev;
+	struct fsl_i2c *dev;
 
 	dev = (struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET);
 
 	writeb(0, &dev->cr);			/* stop I2C controller */
 	udelay(5);				/* let it shutdown in peace */
-	writeb(0x3F, &dev->fdr);		/* set bus speed */
-	writeb(0x3F, &dev->dfsrr);		/* set default filter */
+	i2c_bus_speed[0] = set_i2c_bus_speed(dev, gd->i2c1_clk, speed);
 	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
 	writeb(0x0, &dev->sr);			/* clear status register */
 	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */

@@ -70,12 +159,11 @@ i2c_init(int speed, int slaveadd)
 
 	writeb(0, &dev->cr);			/* stop I2C controller */
 	udelay(5);				/* let it shutdown in peace */
-	writeb(0x3F, &dev->fdr);		/* set bus speed */
-	writeb(0x3F, &dev->dfsrr);		/* set default filter */
+	i2c_bus_speed[1] = set_i2c_bus_speed(dev, gd->i2c2_clk, speed);
 	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
 	writeb(0x0, &dev->sr);			/* clear status register */
 	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
-#endif	/* CFG_I2C2_OFFSET */
+#endif
 }
 
 static __inline__ int

@@ -279,7 +367,14 @@ int i2c_set_bus_num(unsigned int bus)
 
 int i2c_set_bus_speed(unsigned int speed)
 {
-	return -1;
+	unsigned int i2c_clk = (i2c_bus_num == 1) ? gd->i2c2_clk : gd->i2c1_clk;
+
+	writeb(0, &i2c_dev[i2c_bus_num]->cr);		/* stop controller */
+	i2c_bus_speed[i2c_bus_num] =
+		set_i2c_bus_speed(i2c_dev[i2c_bus_num], i2c_clk, speed);
+	writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);	/* start controller */
+
+	return 0;
 }
 
 unsigned int i2c_get_bus_num(void)

drivers/i2c/fsl_i2c.c unsigned int i2c_get_bus_num(void) · line 391

Quoted 9 lines

drivers/i2c/fsl_i2c.c +2 -1

@@ -289,7 +384,8 @@ unsigned int i2c_get_bus_num(void)
 
 unsigned int i2c_get_bus_speed(void)
 {
-	return 0;
+	return i2c_bus_speed[i2c_bus_num];
 }
+
 #endif /* CONFIG_HARD_I2C */
 #endif /* CONFIG_FSL_I2C */

Industrie Dial Face S.p.A.
Luigi Mantellini
R&D - Software
Industrie Dial Face S.p.A.
Via Canzo, 4
20068 Peschiera Borromeo (MI), Italy

Tel.:   +39 02 5167 2813
Fax:    +39 02 5167 2459
E-mail: luigi.mantellini at idf-hit.com
GPG fingerprint: 3DD1 7B71 FBDF 6376
1B4A
                 B003 175F E979 907E

1650
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080327/e4fdf5d8/attachment.htm
-------------- next part --------------
A non-text attachment was scrubbed...
Name: idf_logo.gif
Type: image/gif
Size: 4122 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080327/e4fdf5d8/attachment.gif

Luigi 'Comio' Mantellini

2008-03-27 09:45 UTC #8 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

This patch should resolve the compilation issues on 547x/548x cpus.

Best regards

Quoted 15 lines · Luigi 'Comio' Mantellini

This patch seems incompatible with cf547x/548x cpus because the cpu specific global data doesn't define the i2c1_clk / i2c2_clk attributes.
Best regards,
luigi
Quoted 5 lines · Timur Tabi
Add support to the Freescale I2C driver (fsl_i2c.c) for setting and querying the I2C bus speed. Current 8[356]xx boards define the CFG_I2C_SPEED macro, but fsl_i2c.c ignores it and uses conservative value when programming the I2C bus speed.

Signature · 20 lines

     ______       Luigi Mantellini
   .'______'.     R&D - Software
  (.'      '.)    Industrie Dial Face S.p.A.
  ( :=----=: )    Via Canzo, 4
  ('.______.')    20068 Peschiera Borromeo (MI), Italy
   '.______.'     Tel.: +39 02 5167 2813
                  Fax:  +39 02 5167 2459
Ind.  Dial Face   Email: luigi.mantellini at idf-hit.com
www.idf-hit.com   GPG fingerprint: 3DD1 7B71 FBDF 6376 1B4A
                                   B003 175F E979 907E 1650



-------------- next part --------------
A non-text attachment was scrubbed...
Name: fsl_i2c.patch
Type: text/x-patch
Size: 1296 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080327/276f3d64/attachment.bin

Timur Tabi

author

2008-03-27 14:46 UTC #9 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 1 line · Luigi 'Comio' Mantellini

This patch should resolve the compilation issues on 547x/548x cpus.

Sorry, I didn't even think to check for anything outside of PowerPC. I had no idea m68k used fsl_i2c.c.

Review typedef struct global_data { · line 55

Quoted 10 lines

(quoted diff) +3 -0

@@ -47,6 +47,9 @@ typedef	struct	global_data {
 	unsigned long	vco_clk;
 	unsigned long	flb_clk;
 #endif
+#ifdef CONFIG_MCF547x_8x
+	u32 i2c1_clk;
+#endif
 	unsigned long	ram_size;	/* RAM size */
 	unsigned long	reloc_off;	/* Relocation Offset */
 	unsigned long	reset_status;	/* reset status register at boot	*/

I suggest that you get rid of the #ifdef and always define i2c1_clk. The value will be set to 0 on systems that don't have an I2C. I'm planning on submitting patches to asm-ppc/global_data.h to get rid of a bunch of #ifdefs because it's getting too messy.

Either way, I ACK this patch.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Wolfgang Denk

2008-03-27 14:56 UTC #10 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 5 lines · In message <[email protected]> you wrote:

I suggest that you get rid of the #ifdef and always define i2c1_clk. The value will be set to 0 on systems that don't have an I2C. I'm planning on submitting patches to asm-ppc/global_data.h to get rid of a bunch of #ifdefs because it's getting too messy.

Make sure you don't increase the size of the global data by such pataches. This is critical on some systems.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every little picofarad has a nanohenry all its own.      - Don Vonada

Liew Tsi Chung

2008-03-27 17:30 UTC #11 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

In reply to Timur Tabi

547x/548x are not the only ColdFire affected, 5235, 5373, 5275, 5329, 54455, 52277, and 5271 are required to update as well.

-----Original Message-----
From: Tabi Timur
Sent: Thursday, March 27, 2008 9:46 AM
To: Luigi 'Comio' Mantellini
Cc: U-boot-Users; Liew Tsi Chung
Subject: Re: [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 1 line · Luigi 'Comio' Mantellini

This patch should resolve the compilation issues on 547x/548x cpus.

Sorry, I didn't even think to check for anything outside of PowerPC. I had no idea m68k used fsl_i2c.c.

Review typedef struct global_data { · line 55

Quoted 10 lines

(quoted diff) +3 -0

@@ -47,6 +47,9 @@ typedef	struct	global_data {
 	unsigned long	vco_clk;
 	unsigned long	flb_clk;
 #endif
+#ifdef CONFIG_MCF547x_8x
+	u32 i2c1_clk;
+#endif
 	unsigned long	ram_size;	/* RAM size */
 	unsigned long	reloc_off;	/* Relocation Offset */
 	unsigned long	reset_status;	/* reset status register at boot

Either way, I ACK this patch.

--
Timur Tabi
Linux kernel developer at Freescale

Timur Tabi

author

2008-03-27 17:50 UTC #12 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 2 lines · Liew Tsi Chung

547x/548x are not the only ColdFire affected, 5235, 5373, 5275, 5329, 54455, 52277, and 5271 are required to update as well.

Then how about doing this:

include/asm-m68k/global_data.h +3 -0

@@ -47,6 +47,9 @@ typedef	struct	global_data {
 	unsigned long	vco_clk;
 	unsigned long	flb_clk;
 #endif
+#ifdef CONFIG_FSL_I2C
+	u32 i2c1_clk;
+#endif
 	unsigned long	ram_size;	/* RAM size */
 	unsigned long	reloc_off;	/* Relocation Offset */
 	unsigned long	reset_status;	/* reset status register at boot

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Liew Tsi Chung

2008-03-27 18:42 UTC #13 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Yes. Thanks.

Regards,
TsiChung

-----Original Message-----
From: Tabi Timur
Sent: Thursday, March 27, 2008 12:51 PM
To: Liew Tsi Chung
Cc: Luigi 'Comio' Mantellini; U-boot-Users
Subject: Re: [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 2 lines · Liew Tsi Chung

547x/548x are not the only ColdFire affected, 5235, 5373, 5275, 5329, 54455, 52277, and 5271 are required to update as well.

Then how about doing this:

include/asm-m68k/global_data.h +3 -0

@@ -47,6 +47,9 @@ typedef	struct	global_data {
 	unsigned long	vco_clk;
 	unsigned long	flb_clk;
 #endif
+#ifdef CONFIG_FSL_I2C
+	u32 i2c1_clk;
+#endif
 	unsigned long	ram_size;	/* RAM size */
 	unsigned long	reloc_off;	/* Relocation Offset */
 	unsigned long	reset_status;	/* reset status register at boot

--
Timur Tabi
Linux kernel developer at Freescale

Wolfgang Denk

2008-04-14 01:09 UTC #14 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

Quoted 1 line · In message <4791E710007FEB4BBF83775D787F462F06EAF512@az33exm22.fsl.freescale.ne…

Yes. Thanks.

...

Quoted 17 lines

Then how about doing this:

include/asm-m68k/global_data.h +3 -1

@@ -47,6 +47,9 @@ typedef	struct	global_data {
 	unsigned long	vco_clk;
 	unsigned long	flb_clk;
 #endif
+#ifdef CONFIG_FSL_I2C
+	u32 i2c1_clk;
+#endif
 	unsigned long	ram_size;	/* RAM size */
 	unsigned long	reloc_off;	/* Relocation Offset */
 	unsigned long	reset_status;	/* reset status register at boot
 
 
--

Quoted 1 line

Timur Tabi

So will somebody submit a proper patch with usable commit message and proper Signed-off-by: line, please?

Best regards,

Wolfgang Denk

Signature · 8 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The connection between the language in which we think/program and the
problems and solutions we can imagine is very close. For this  reason
restricting  language  features  with  the intent of eliminating pro-
grammer errors is at best dangerous.
               - Bjarne Stroustrup in "The  C++ Programming Language"

Wolfgang Denk

2008-04-14 01:09 UTC #15 lore raw

PATCH 1/1 Add support for setting the I2C bus speed in fsl_i2c.c

In reply to Luigi 'Comio' Mantellini

Quoted 2 lines · In message <1206611122.23794.13.camel@localhost> you wrote:

This patch should resolve the compilation issues on 547x/548x cpus.

Unfortunately it's missing your Signed-off-by: line.

Also, please post patches inline rather than attachments.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How come everyone's going so slow if it's called rush hour?