All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
@ 2008-03-14 22:45 Timur Tabi
  2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Timur Tabi @ 2008-03-14 22:45 UTC (permalink / raw
  To: u-boot

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 <timur@freescale.com>
---

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

diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index 22485ea..dde0571 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -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 */
-- 
1.5.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-14 22:45 [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c Timur Tabi
@ 2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
  2008-03-15 21:27   ` Timur Tabi
  2008-03-26 15:39 ` Timur Tabi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2008-03-15  7:35 UTC (permalink / raw
  To: u-boot

On 17:45 Fri 14 Mar     , Timur Tabi 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 <timur@freescale.com>
> ---
> 
> 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2008-03-15 21:27   ` Timur Tabi
  0 siblings, 0 replies; 16+ messages in thread
From: Timur Tabi @ 2008-03-15 21:27 UTC (permalink / raw
  To: u-boot

Jean-Christophe PLAGNIOL-VILLARD wrote:

>> 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?

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-14 22:45 [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c Timur Tabi
  2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2008-03-26 15:39 ` Timur Tabi
  2008-03-26 16:36   ` Andy Fleming
  2008-03-26 23:12 ` Wolfgang Denk
  2008-03-27  9:11 ` Luigi 'Comio' Mantellini
  3 siblings, 1 reply; 16+ messages in thread
From: Timur Tabi @ 2008-03-26 15:39 UTC (permalink / raw
  To: u-boot

Timur Tabi 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.

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.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-26 15:39 ` Timur Tabi
@ 2008-03-26 16:36   ` Andy Fleming
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Fleming @ 2008-03-26 16:36 UTC (permalink / raw
  To: u-boot

On Wed, Mar 26, 2008 at 10:39 AM, Timur Tabi <timur@freescale.com> wrote:
> Timur Tabi 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.
>
>  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 <afleming@freescale.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-14 22:45 [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c Timur Tabi
  2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
  2008-03-26 15:39 ` Timur Tabi
@ 2008-03-26 23:12 ` Wolfgang Denk
  2008-03-27 14:33   ` Timur Tabi
  2008-03-27  9:11 ` Luigi 'Comio' Mantellini
  3 siblings, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-03-26 23:12 UTC (permalink / raw
  To: u-boot

In message <1205534729-26151-1-git-send-email-timur@freescale.com> 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 <timur@freescale.com>

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-14 22:45 [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c Timur Tabi
                   ` (2 preceding siblings ...)
  2008-03-26 23:12 ` Wolfgang Denk
@ 2008-03-27  9:11 ` Luigi 'Comio' Mantellini
  2008-03-27  9:45   ` Luigi 'Comio' Mantellini
  3 siblings, 1 reply; 16+ messages in thread
From: Luigi 'Comio' Mantellini @ 2008-03-27  9:11 UTC (permalink / raw
  To: u-boot


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

On ven, 2008-03-14 at 17:45 -0500, Timur Tabi 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 <timur@freescale.com>
> ---
> 
> 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(-)
> 
> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
> index 22485ea..dde0571 100644
> --- a/drivers/i2c/fsl_i2c.c
> +++ b/drivers/i2c/fsl_i2c.c
> @@ -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 */

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 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27  9:11 ` Luigi 'Comio' Mantellini
@ 2008-03-27  9:45   ` Luigi 'Comio' Mantellini
  2008-03-27 14:46     ` Timur Tabi
  2008-04-14  1:09     ` Wolfgang Denk
  0 siblings, 2 replies; 16+ messages in thread
From: Luigi 'Comio' Mantellini @ 2008-03-27  9:45 UTC (permalink / raw
  To: u-boot

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

Best regards

On gio, 2008-03-27 at 10:11 +0100, Luigi 'Comio' Mantellini wrote:
> 
> 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
> 
> On ven, 2008-03-14 at 17:45 -0500, Timur Tabi 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.
> > 

-- 
     ______       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 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-26 23:12 ` Wolfgang Denk
@ 2008-03-27 14:33   ` Timur Tabi
  0 siblings, 0 replies; 16+ messages in thread
From: Timur Tabi @ 2008-03-27 14:33 UTC (permalink / raw
  To: u-boot

Wolfgang Denk wrote:

> 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.

> 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.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27  9:45   ` Luigi 'Comio' Mantellini
@ 2008-03-27 14:46     ` Timur Tabi
  2008-03-27 14:56       ` Wolfgang Denk
  2008-03-27 17:30       ` Liew Tsi Chung
  2008-04-14  1:09     ` Wolfgang Denk
  1 sibling, 2 replies; 16+ messages in thread
From: Timur Tabi @ 2008-03-27 14:46 UTC (permalink / raw
  To: u-boot

Luigi 'Comio' Mantellini wrote:
> 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.

> @@ -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.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27 14:46     ` Timur Tabi
@ 2008-03-27 14:56       ` Wolfgang Denk
  2008-03-27 17:30       ` Liew Tsi Chung
  1 sibling, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2008-03-27 14:56 UTC (permalink / raw
  To: u-boot

In message <47EBB33F.9000308@freescale.com> 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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27 14:46     ` Timur Tabi
  2008-03-27 14:56       ` Wolfgang Denk
@ 2008-03-27 17:30       ` Liew Tsi Chung
  2008-03-27 17:50         ` Timur Tabi
  1 sibling, 1 reply; 16+ messages in thread
From: Liew Tsi Chung @ 2008-03-27 17:30 UTC (permalink / raw
  To: u-boot

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

Luigi 'Comio' Mantellini wrote:
> 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.

> @@ -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.

--
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27 17:30       ` Liew Tsi Chung
@ 2008-03-27 17:50         ` Timur Tabi
  2008-03-27 18:42           ` Liew Tsi Chung
  0 siblings, 1 reply; 16+ messages in thread
From: Timur Tabi @ 2008-03-27 17:50 UTC (permalink / raw
  To: u-boot

Liew Tsi Chung wrote:
> 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:

--- a/include/asm-m68k/global_data.h
+++ b/include/asm-m68k/global_data.h
@@ -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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27 17:50         ` Timur Tabi
@ 2008-03-27 18:42           ` Liew Tsi Chung
  2008-04-14  1:09             ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Liew Tsi Chung @ 2008-03-27 18:42 UTC (permalink / raw
  To: u-boot

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

Liew Tsi Chung wrote:
> 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:

--- a/include/asm-m68k/global_data.h
+++ b/include/asm-m68k/global_data.h
@@ -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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27  9:45   ` Luigi 'Comio' Mantellini
  2008-03-27 14:46     ` Timur Tabi
@ 2008-04-14  1:09     ` Wolfgang Denk
  1 sibling, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-14  1:09 UTC (permalink / raw
  To: u-boot

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

-- 
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?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c
  2008-03-27 18:42           ` Liew Tsi Chung
@ 2008-04-14  1:09             ` Wolfgang Denk
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-14  1:09 UTC (permalink / raw
  To: u-boot

In message <4791E710007FEB4BBF83775D787F462F06EAF512@az33exm22.fsl.freescale.net> you wrote:
> Yes. Thanks.
...
> Then how about doing this:
> 
> --- a/include/asm-m68k/global_data.h
> +++ b/include/asm-m68k/global_data.h
> @@ -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

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

Best regards,

Wolfgang Denk

-- 
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"

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-04-14  1:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-14 22:45 [U-Boot-Users] [PATCH] Add support for setting the I2C bus speed in fsl_i2c.c Timur Tabi
2008-03-15  7:35 ` Jean-Christophe PLAGNIOL-VILLARD
2008-03-15 21:27   ` Timur Tabi
2008-03-26 15:39 ` Timur Tabi
2008-03-26 16:36   ` Andy Fleming
2008-03-26 23:12 ` Wolfgang Denk
2008-03-27 14:33   ` Timur Tabi
2008-03-27  9:11 ` Luigi 'Comio' Mantellini
2008-03-27  9:45   ` Luigi 'Comio' Mantellini
2008-03-27 14:46     ` Timur Tabi
2008-03-27 14:56       ` Wolfgang Denk
2008-03-27 17:30       ` Liew Tsi Chung
2008-03-27 17:50         ` Timur Tabi
2008-03-27 18:42           ` Liew Tsi Chung
2008-04-14  1:09             ` Wolfgang Denk
2008-04-14  1:09     ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.