[U-Boot-Users] [COLDFIRE]
The coldfire cpus use a little different i2c device that is not perfectly compatible with the i2c fsl driver present on mpc cpus. consequently the fsl_i2c driver needs some little modification in order to work on these cpus (m547x/m548x and m5445x cpus).
luigi
--- drivers/i2c/fsl_i2c.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 0690340..8fd73c0 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -78,6 +78,73 @@ static const struct fsl_i2c *i2c_dev[2] = { * register. See the application note AN2919 "Determining the I2C Frequency * Divider Ratio for SCL" */ +#if defined(CONFIG_MCF547x_8x) || defined(CONFIG_MCF5445x) +static const struct { + u8 fdr; + unsigned short divider; +} fsl_i2c_speed_map[] = { + {.fdr=0x20, .divider= 20}, {.fdr=0x21, .divider= 22}, {.fdr=0x22, .divider= 24}, + {.fdr=0x23, .divider= 26}, {.fdr=0x00, .divider= 28}, {.fdr=0x24, .divider= 28}, + {.fdr=0x01, .divider= 30}, {.fdr=0x25, .divider= 32}, {.fdr=0x02, .divider= 34}, + {.fdr=0x26, .divider= 36}, {.fdr=0x03, .divider= 40}, {.fdr=0x27, .divider= 40}, + {.fdr=0x04, .divider= 44}, {.fdr=0x05, .divider= 48}, /*{.fdr=0x28, .divider= 48},*/ + {.fdr=0x06, .divider= 56}, /*{.fdr=0x29, .divider= 56},*/ {.fdr=0x2A, .divider= 64}, + {.fdr=0x07, .divider= 68}, {.fdr=0x2B, .divider= 72}, {.fdr=0x08, .divider= 80}, + /*{.fdr=0x2C, .divider= 80},*/ {.fdr=0x09, .divider= 88}, {.fdr=0x2D, .divider= 96}, + {.fdr=0x0A, .divider= 104}, {.fdr=0x2E, .divider= 112}, {.fdr=0x0B, .divider= 128}, + /*{.fdr=0x2F, .divider= 128},*/ {.fdr=0x0C, .divider= 144}, {.fdr=0x0D, .divider= 160}, + /*{.fdr=0x30, .divider= 160},*/ {.fdr=0x0E, .divider= 192}, /*{.fdr=0x31, .divider= 192},*/ + {.fdr=0x32, .divider= 224}, {.fdr=0x0F, .divider= 240}, {.fdr=0x33, .divider= 256}, + {.fdr=0x10, .divider= 288}, {.fdr=0x11, .divider= 320}, /*{.fdr=0x34, .divider= 320},*/ + {.fdr=0x12, .divider= 384}, /*{.fdr=0x35, .divider= 384},*/ {.fdr=0x36, .divider= 448}, + /*{.fdr=0x13, .divider= 480},*/ {.fdr=0x37, .divider= 512}, {.fdr=0x14, .divider= 576}, + {.fdr=0x15, .divider= 640}, /*{.fdr=0x38, .divider= 640},*/ {.fdr=0x16, .divider= 768}, + /*{.fdr=0x39, .divider= 768},*/ {.fdr=0x3A, .divider= 896}, {.fdr=0x17, .divider= 960}, + {.fdr=0x3B, .divider=1024}, {.fdr=0x18, .divider=1152}, {.fdr=0x19, .divider=1280}, + /*{.fdr=0x3C, .divider=1280},*/ {.fdr=0x1A, .divider=1536}, /*{.fdr=0x3D, .divider=1536},*/ + {.fdr=0x3E, .divider=1792}, {.fdr=0x1B, .divider=1920}, {.fdr=0x3F, .divider=2048}, + {.fdr=0x1C, .divider=2304}, {.fdr=0x1D, .divider=2560}, {.fdr=0x1E, .divider=3072}, + {.fdr=0x1F, .divider=3840}, {.fdr=0x00, .divider=-1} +}; + +/** + * 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 = 0; + + /* + * 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) { + fdr = fsl_i2c_speed_map[i].fdr; + speed = i2c_clk / fsl_i2c_speed_map[i].divider; + break; + } + + writeb(fdr, &dev->fdr); /* set bus speed */ + + return speed; +} +#else /* ! (defined(CONFIG_MCF547x_8x) || defined(CONFIG_MCF5445x) */ + static const struct { unsigned short divider; u8 dfsr; @@ -140,6 +207,7 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
return speed; } +#endif /* if (defined(CONFIG_MCF547x_8x) || defined(CONFIG_MCF5445x) */
void i2c_init(int speed, int slaveadd)
In message 1215439715-28742-2-git-send-email-luigi.mantellini@idf-hit.com you wrote:
drivers/i2c/fsl_i2c.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 68 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c index 0690340..8fd73c0 100644 --- a/drivers/i2c/fsl_i2c.c +++ b/drivers/i2c/fsl_i2c.c @@ -78,6 +78,73 @@ static const struct fsl_i2c *i2c_dev[2] = {
- register. See the application note AN2919 "Determining the I2C Frequency
- Divider Ratio for SCL"
*/ +#if defined(CONFIG_MCF547x_8x) || defined(CONFIG_MCF5445x) +static const struct {
- u8 fdr;
- unsigned short divider;
+} fsl_i2c_speed_map[] = {
{.fdr=0x20, .divider= 20}, {.fdr=0x21, .divider= 22}, {.fdr=0x22, .divider= 24},{.fdr=0x23, .divider= 26}, {.fdr=0x00, .divider= 28}, {.fdr=0x24, .divider= 28},{.fdr=0x01, .divider= 30}, {.fdr=0x25, .divider= 32}, {.fdr=0x02, .divider= 34},{.fdr=0x26, .divider= 36}, {.fdr=0x03, .divider= 40}, {.fdr=0x27, .divider= 40},{.fdr=0x04, .divider= 44}, {.fdr=0x05, .divider= 48}, /*{.fdr=0x28, .divider= 48},*/{.fdr=0x06, .divider= 56}, /*{.fdr=0x29, .divider= 56},*/ {.fdr=0x2A, .divider= 64},{.fdr=0x07, .divider= 68}, {.fdr=0x2B, .divider= 72}, {.fdr=0x08, .divider= 80},- /*{.fdr=0x2C, .divider= 80},*/ {.fdr=0x09, .divider= 88}, {.fdr=0x2D, .divider= 96},
{.fdr=0x0A, .divider= 104}, {.fdr=0x2E, .divider= 112}, {.fdr=0x0B, .divider= 128},- /*{.fdr=0x2F, .divider= 128},*/ {.fdr=0x0C, .divider= 144}, {.fdr=0x0D, .divider= 160},
- /*{.fdr=0x30, .divider= 160},*/ {.fdr=0x0E, .divider= 192}, /*{.fdr=0x31, .divider= 192},*/
{.fdr=0x32, .divider= 224}, {.fdr=0x0F, .divider= 240}, {.fdr=0x33, .divider= 256},{.fdr=0x10, .divider= 288}, {.fdr=0x11, .divider= 320}, /*{.fdr=0x34, .divider= 320},*/{.fdr=0x12, .divider= 384}, /*{.fdr=0x35, .divider= 384},*/ {.fdr=0x36, .divider= 448},- /*{.fdr=0x13, .divider= 480},*/ {.fdr=0x37, .divider= 512}, {.fdr=0x14, .divider= 576},
{.fdr=0x15, .divider= 640}, /*{.fdr=0x38, .divider= 640},*/ {.fdr=0x16, .divider= 768},- /*{.fdr=0x39, .divider= 768},*/ {.fdr=0x3A, .divider= 896}, {.fdr=0x17, .divider= 960},
{.fdr=0x3B, .divider=1024}, {.fdr=0x18, .divider=1152}, {.fdr=0x19, .divider=1280},- /*{.fdr=0x3C, .divider=1280},*/ {.fdr=0x1A, .divider=1536}, /*{.fdr=0x3D, .divider=1536},*/
{.fdr=0x3E, .divider=1792}, {.fdr=0x1B, .divider=1920}, {.fdr=0x3F, .divider=2048},{.fdr=0x1C, .divider=2304}, {.fdr=0x1D, .divider=2560}, {.fdr=0x1E, .divider=3072},{.fdr=0x1F, .divider=3840}, {.fdr=0x00, .divider=-1}+};
+/**
- 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 = 0;
- /*
* 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) {fdr = fsl_i2c_speed_map[i].fdr;speed = i2c_clk / fsl_i2c_speed_map[i].divider;break;}- writeb(fdr, &dev->fdr); /* set bus speed */
- return speed;
+} +#else /* ! (defined(CONFIG_MCF547x_8x) || defined(CONFIG_MCF5445x) */
What a nightmare of data and code. So many magic numbers, so much potential for errors. And for what? Just for I2C support in U-Boot?
Are you really sure you don't simply want to use the bit-banging driver?
Best regards,
Wolfgang Denk
On lun, 2008-07-07 at 16:59 +0200, Wolfgang Denk wrote:
In message 1215439715-28742-2-git-send-email-luigi.mantellini@idf-hit.com you wrote:
....
What a nightmare of data and code. So many magic numbers, so much potential for errors. And for what? Just for I2C support in U-Boot?
From your PoV you are right.
Are you really sure you don't simply want to use the bit-banging driver?
I just extended the fsl_i2c.c driver that already uses a precomputed table to setup the fsl i2c. I haven't invented these values but just copied from the 547x and 5445x manuals. Furthermore, the h/w i2c is more accurate that the simple bitbanging.
my 2cents.
luigi
Best regards,
Wolfgang Denk
In message 1215447465.12657.15.camel@localhost you wrote:
I just extended the fsl_i2c.c driver that already uses a precomputed
Yes, I know. I never understood what such a complicated driver for sich a simple protocol was good for, especially as we don't do multimegabytepersecond transfers over such a bus.
table to setup the fsl i2c. I haven't invented these values but just copied from the 547x and 5445x manuals. Furthermore, the h/w i2c is more accurate that the simple bitbanging.
Accurate? There is no requirement to be "accurate" anywhere in the I2C protocol. It is a simple, brain-dead protokol where timing is highly uncritical. And given that we need it to read a few bytes from EEPROM or RTC or similar I really see no benefit in using such a complicated driver.
Best regards,
Wolfgang Denk
Il giorno lun, 07/07/2008 alle 20.02 +0200, Wolfgang Denk ha scritto:
In message 1215447465.12657.15.camel@localhost you wrote:
I just extended the fsl_i2c.c driver that already uses a precomputed
Yes, I know. I never understood what such a complicated driver for sich a simple protocol was good for, especially as we don't do multimegabytepersecond transfers over such a bus.
I agree with you. But I prefer to have a (too) complex working driver instead an buggy driver. In other hands if m547x cpus have to use the fsl_driver... this should be fixed in order to work fine. Otherwise the HW i2c driver should be dropped from m57x/m5445x boards in order to avoid to confuse the u-boot users/developers.
Today my HW-man ask me why the i2c bus ran at 1Mhz... The cause was that I used the driver with confidence... too confidence...
table to setup the fsl i2c. I haven't invented these values but just copied from the 547x and 5445x manuals. Furthermore, the h/w i2c is more accurate that the simple bitbanging.
Accurate? There is no requirement to be "accurate" anywhere in the I2C protocol. It is a simple, brain-dead protokol where timing is highly uncritical. And given that we need it to read a few bytes from EEPROM or RTC or similar I really see no benefit in using such a complicated driver.
I know :) but I like see 100kHz +/-0.01% when I configure 100kHz... I'm kidding, of course. eheheh... In the last time I preferred to disable i2c driver on all my applications... I ask me why all embedded controller on the SoC that I'm using are bugged... are the masks too expensive to work fine?
Ciao ciao
luigi
Best regards,
Wolfgang Denk
Ing. Luigi Mantellini Industrie Dial Face S.p.A. Via Canzo, 4 20068 Peschiera Borromeo (MI) Tel.: +39 02 5167 2813 Fax: +39 02 5167 2459 E-mail: luigi.mantellini@idf-hit.com
On Jul 7, 2008, at 2:02 PM, Wolfgang Denk wrote:
In message 1215447465.12657.15.camel@localhost you wrote:
I just extended the fsl_i2c.c driver that already uses a precomputed
Yes, I know. I never understood what such a complicated driver for sich a simple protocol was good for, especially as we don't do multimegabytepersecond transfers over such a bus.
The driver is no more complicated than the hardware itself. The Freescale I2C controller has a complicated, screwball method for setting the I2C bus speed, and the driver tries to simplify that as much as possible. Setting the I2C bus speed *may* be important on some boards with flaky I2C devices.
Accurate? There is no requirement to be "accurate" anywhere in the I2C protocol.
Some devices cannot handle certain bus speeds, and some devices may work better at higher bus speeds, so it may be important to control the bus speed accurately.
However, Luigi's patch is bad because it modifies a common file for a specific board. I'm going to NAK it.
participants (3)
-
Luigi 'Comio' Mantellini -
Timur Tabi -
Wolfgang Denk