[U-Boot] [PATCH] ARM: OMAP: I2C coding style clean up

Subject: [PATCH] ARM: OMAP: I2C coding style clean up
From: Dirk Behme dirk.behme@gmail.com
Clean up coding style and read/write macro usage as requested by Wolfgang Denk and Jean-Christophe PLAGNIOL-VILLARD.
Signed-off-by: Dirk Behme dirk.behme@gmail.com
---
Note: Patch is against U-Boot master commit 1378174a1351c0285736863a665ab758fe8d5f71 "Merge branch 'master' of /home/wd/git/u-boot/custodians"
drivers/i2c/omap24xx_i2c.c | 250 ++++++++++++++++++++++----------------------- 1 files changed, 126 insertions(+), 124 deletions(-)
Index: u-boot-main/drivers/i2c/omap24xx_i2c.c =================================================================== --- u-boot-main.orig/drivers/i2c/omap24xx_i2c.c +++ u-boot-main/drivers/i2c/omap24xx_i2c.c @@ -25,67 +25,64 @@ #include <asm/arch/i2c.h> #include <asm/io.h>
-#define inw(a) __raw_readw(a) -#define outw(a,v) __raw_writew(a,v) - -static void wait_for_bb (void); -static u16 wait_for_pin (void); +static void wait_for_bb(void); +static u16 wait_for_pin(void); static void flush_fifo(void);
-void i2c_init (int speed, int slaveadd) +void i2c_init(int speed, int slaveadd) { u16 scl;
- outw(0x2, I2C_SYSC); /* for ES2 after soft reset */ + writew(0x2, I2C_SYSC); /* for ES2 after soft reset */ udelay(1000); - outw(0x0, I2C_SYSC); /* will probably self clear but */ + writew(0x0, I2C_SYSC); /* will probably self clear but */
- if (inw (I2C_CON) & I2C_CON_EN) { - outw (0, I2C_CON); - udelay (50000); + if (readw(I2C_CON) & I2C_CON_EN) { + writew(0, I2C_CON); + udelay(50000); }
/* 12MHz I2C module clock */ - outw (0, I2C_PSC); - speed = speed/1000; /* 100 or 400 */ - scl = ((12000/(speed*2)) - 7); /* use 7 when PSC = 0 */ - outw (scl, I2C_SCLL); - outw (scl, I2C_SCLH); + writew(0, I2C_PSC); + speed = speed / 1000; /* 100 or 400 */ + scl = ((12000 / (speed * 2)) - 7); /* use 7 when PSC = 0 */ + writew(scl, I2C_SCLL); + writew(scl, I2C_SCLH); /* own address */ - outw (slaveadd, I2C_OA); - outw (I2C_CON_EN, I2C_CON); + writew(slaveadd, I2C_OA); + writew(I2C_CON_EN, I2C_CON);
/* have to enable intrrupts or OMAP i2c module doesn't work */ - outw (I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE | - I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE); - udelay (1000); + writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE | + I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE); + udelay(1000); flush_fifo(); - outw (0xFFFF, I2C_STAT); - outw (0, I2C_CNT); + writew(0xFFFF, I2C_STAT); + writew(0, I2C_CNT); }
-static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) +static int i2c_read_byte(u8 devaddr, u8 regoffset, u8 *value) { int i2c_error = 0; u16 status;
/* wait until bus not busy */ - wait_for_bb (); + wait_for_bb();
/* one byte only */ - outw (1, I2C_CNT); + writew(1, I2C_CNT); /* set slave address */ - outw (devaddr, I2C_SA); + writew(devaddr, I2C_SA); /* no stop bit needed here */ - outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON); + writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON);
- status = wait_for_pin (); + status = wait_for_pin();
if (status & I2C_STAT_XRDY) { /* Important: have to use byte access */ - *(volatile u8 *) (I2C_DATA) = regoffset; - udelay (20000); - if (inw (I2C_STAT) & I2C_STAT_NACK) { + writeb(regoffset, I2C_DATA); + udelay(20000); + if (readw(I2C_STAT) & I2C_STAT_NACK) { i2c_error = 1; } } else { @@ -94,70 +91,70 @@ static int i2c_read_byte (u8 devaddr, u8
if (!i2c_error) { /* free bus, otherwise we can't use a combined transction */ - outw (0, I2C_CON); - while (inw (I2C_STAT) || (inw (I2C_CON) & I2C_CON_MST)) { - udelay (10000); + writew(0, I2C_CON); + while (readw(I2C_STAT) || (readw(I2C_CON) & I2C_CON_MST)) { + udelay(10000); /* Have to clear pending interrupt to clear I2C_STAT */ - outw (0xFFFF, I2C_STAT); + writew(0xFFFF, I2C_STAT); }
- wait_for_bb (); + wait_for_bb(); /* set slave address */ - outw (devaddr, I2C_SA); + writew(devaddr, I2C_SA); /* read one byte from slave */ - outw (1, I2C_CNT); + writew(1, I2C_CNT); /* need stop bit here */ - outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, - I2C_CON); + writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, + I2C_CON);
- status = wait_for_pin (); + status = wait_for_pin(); if (status & I2C_STAT_RRDY) { - *value = inw (I2C_DATA); - udelay (20000); + *value = readw(I2C_DATA); + udelay(20000); } else { i2c_error = 1; }
if (!i2c_error) { - outw (I2C_CON_EN, I2C_CON); - while (inw (I2C_STAT) - || (inw (I2C_CON) & I2C_CON_MST)) { - udelay (10000); - outw (0xFFFF, I2C_STAT); + writew(I2C_CON_EN, I2C_CON); + while (readw(I2C_STAT) + || (readw(I2C_CON) & I2C_CON_MST)) { + udelay(10000); + writew(0xFFFF, I2C_STAT); } } } flush_fifo(); - outw (0xFFFF, I2C_STAT); - outw (0, I2C_CNT); + writew(0xFFFF, I2C_STAT); + writew(0, I2C_CNT); return i2c_error; }
-static int i2c_write_byte (u8 devaddr, u8 regoffset, u8 value) +static int i2c_write_byte(u8 devaddr, u8 regoffset, u8 value) { int i2c_error = 0; u16 status, stat;
/* wait until bus not busy */ - wait_for_bb (); + wait_for_bb();
/* two bytes */ - outw (2, I2C_CNT); + writew(2, I2C_CNT); /* set slave address */ - outw (devaddr, I2C_SA); + writew(devaddr, I2C_SA); /* stop bit needed here */ - outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX | - I2C_CON_STP, I2C_CON); + writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX | + I2C_CON_STP, I2C_CON);
/* wait until state change */ - status = wait_for_pin (); + status = wait_for_pin();
if (status & I2C_STAT_XRDY) { /* send out two bytes */ - outw ((value << 8) + regoffset, I2C_DATA); + writew((value << 8) + regoffset, I2C_DATA); /* must have enough delay to allow BB bit to go low */ - udelay (50000); - if (inw (I2C_STAT) & I2C_STAT_NACK) { + udelay(50000); + if (readw(I2C_STAT) & I2C_STAT_NACK) { i2c_error = 1; } } else { @@ -167,92 +164,97 @@ static int i2c_write_byte (u8 devaddr, u if (!i2c_error) { int eout = 200;
- outw (I2C_CON_EN, I2C_CON); - while ((stat = inw (I2C_STAT)) || (inw (I2C_CON) & I2C_CON_MST)) { - udelay (1000); + writew(I2C_CON_EN, I2C_CON); + while ((stat = readw(I2C_STAT)) || + (readw(I2C_CON) & I2C_CON_MST)) { + udelay(1000); /* have to read to clear intrrupt */ - outw (0xFFFF, I2C_STAT); - if(--eout == 0) /* better leave with error than hang */ + writew(0xFFFF, I2C_STAT); + /* better leave with error than hang */ + if (--eout == 0) break; } } flush_fifo(); - outw (0xFFFF, I2C_STAT); - outw (0, I2C_CNT); + writew(0xFFFF, I2C_STAT); + writew(0, I2C_CNT); return i2c_error; }
static void flush_fifo(void) -{ u16 stat; +{ + u16 stat;
- /* note: if you try and read data when its not there or ready + /* + * note: if you try and read data when its not there or ready * you get a bus error */ - while(1){ - stat = inw(I2C_STAT); - if(stat == I2C_STAT_RRDY){ - inw(I2C_DATA); - outw(I2C_STAT_RRDY,I2C_STAT); + while (1) { + stat = readw(I2C_STAT); + if (stat == I2C_STAT_RRDY) { + readw(I2C_DATA); + writew(I2C_STAT_RRDY, I2C_STAT); udelay(1000); - }else + } else break; } }
-int i2c_probe (uchar chip) +int i2c_probe(uchar chip) { - int res = 1; /* default = fail */ + int res = 1; /* default = fail */
- if (chip == inw (I2C_OA)) { + if (chip == readw(I2C_OA)) return res; - }
/* wait until bus not busy */ - wait_for_bb (); + wait_for_bb();
/* try to read one byte */ - outw (1, I2C_CNT); + writew(1, I2C_CNT); /* set slave address */ - outw (chip, I2C_SA); + writew(chip, I2C_SA); /* stop bit needed here */ - outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, I2C_CON); + writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, I2C_CON); /* enough delay for the NACK bit set */ - udelay (50000); + udelay(50000);
- if (!(inw (I2C_STAT) & I2C_STAT_NACK)) { - res = 0; /* success case */ + if (!(readw(I2C_STAT) & I2C_STAT_NACK)) { + res = 0; /* success case */ flush_fifo(); - outw(0xFFFF, I2C_STAT); + writew(0xFFFF, I2C_STAT); } else { - outw(0xFFFF, I2C_STAT); /* failue, clear sources*/ - outw (inw (I2C_CON) | I2C_CON_STP, I2C_CON); /* finish up xfer */ + writew(0xFFFF, I2C_STAT); /* failue, clear sources*/ + /* finish up xfer */ + writew(readw(I2C_CON) | I2C_CON_STP, I2C_CON); udelay(20000); - wait_for_bb (); + wait_for_bb(); } flush_fifo(); - outw (0, I2C_CNT); /* don't allow any more data in...we don't want it.*/ - outw(0xFFFF, I2C_STAT); + /* don't allow any more data in...we don't want it. */ + writew(0, I2C_CNT); + writew(0xFFFF, I2C_STAT); return res; }
-int i2c_read (uchar chip, uint addr, int alen, uchar * buffer, int len) +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len) { int i;
if (alen > 1) { - printf ("I2C read: addr len %d not supported\n", alen); + printf("I2C read: addr len %d not supported\n", alen); return 1; }
if (addr + len > 256) { - printf ("I2C read: address out of range\n"); + printf("I2C read: address out of range\n"); return 1; }
for (i = 0; i < len; i++) { - if (i2c_read_byte (chip, addr + i, &buffer[i])) { - printf ("I2C read: I/O error\n"); - i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); + if (i2c_read_byte(chip, addr + i, &buffer[i])) { + printf("I2C read: I/O error\n"); + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); return 1; } } @@ -260,24 +262,24 @@ int i2c_read (uchar chip, uint addr, int return 0; }
-int i2c_write (uchar chip, uint addr, int alen, uchar * buffer, int len) +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len) { int i;
if (alen > 1) { - printf ("I2C read: addr len %d not supported\n", alen); + printf("I2C read: addr len %d not supported\n", alen); return 1; }
if (addr + len > 256) { - printf ("I2C read: address out of range\n"); + printf("I2C read: address out of range\n"); return 1; }
for (i = 0; i < len; i++) { - if (i2c_write_byte (chip, addr + i, buffer[i])) { - printf ("I2C read: I/O error\n"); - i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); + if (i2c_write_byte(chip, addr + i, buffer[i])) { + printf("I2C read: I/O error\n"); + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); return 1; } } @@ -285,41 +287,41 @@ int i2c_write (uchar chip, uint addr, in return 0; }
-static void wait_for_bb (void) +static void wait_for_bb(void) { int timeout = 10; u16 stat;
- outw(0xFFFF, I2C_STAT); /* clear current interruts...*/ - while ((stat = inw (I2C_STAT) & I2C_STAT_BB) && timeout--) { - outw (stat, I2C_STAT); - udelay (50000); + writew(0xFFFF, I2C_STAT); /* clear current interruts...*/ + while ((stat = readw(I2C_STAT) & I2C_STAT_BB) && timeout--) { + writew(stat, I2C_STAT); + udelay(50000); }
if (timeout <= 0) { - printf ("timed out in wait_for_bb: I2C_STAT=%x\n", - inw (I2C_STAT)); + printf("timed out in wait_for_bb: I2C_STAT=%x\n", + readw(I2C_STAT)); } - outw(0xFFFF, I2C_STAT); /* clear delayed stuff*/ + writew(0xFFFF, I2C_STAT); /* clear delayed stuff*/ }
-static u16 wait_for_pin (void) +static u16 wait_for_pin(void) { u16 status; int timeout = 10;
do { - udelay (1000); - status = inw (I2C_STAT); - } while ( !(status & - (I2C_STAT_ROVR | I2C_STAT_XUDF | I2C_STAT_XRDY | - I2C_STAT_RRDY | I2C_STAT_ARDY | I2C_STAT_NACK | - I2C_STAT_AL)) && timeout--); + udelay(1000); + status = readw(I2C_STAT); + } while (!(status & + (I2C_STAT_ROVR | I2C_STAT_XUDF | I2C_STAT_XRDY | + I2C_STAT_RRDY | I2C_STAT_ARDY | I2C_STAT_NACK | + I2C_STAT_AL)) && timeout--);
if (timeout <= 0) { - printf ("timed out in wait_for_pin: I2C_STAT=%x\n", - inw (I2C_STAT)); - outw(0xFFFF, I2C_STAT); -} + printf("timed out in wait_for_pin: I2C_STAT=%x\n", + readw(I2C_STAT)); + writew(0xFFFF, I2C_STAT); + } return status; }

Dear dirk.behme@googlemail.com,
In message 4916ed9e.1d255e0a.7af6.ffffe30d@mx.google.com you wrote:
Subject: [PATCH] ARM: OMAP: I2C coding style clean up
From: Dirk Behme dirk.behme@gmail.com
Clean up coding style and read/write macro usage as requested by Wolfgang Denk and Jean-Christophe PLAGNIOL-VILLARD.
Signed-off-by: Dirk Behme dirk.behme@gmail.com
Note: Patch is against U-Boot master commit 1378174a1351c0285736863a665ab758fe8d5f71 "Merge branch 'master' of /home/wd/git/u-boot/custodians"
drivers/i2c/omap24xx_i2c.c | 250 ++++++++++++++++++++++----------------------- 1 files changed, 126 insertions(+), 124 deletions(-)
NAK.
Index: u-boot-main/drivers/i2c/omap24xx_i2c.c
--- u-boot-main.orig/drivers/i2c/omap24xx_i2c.c +++ u-boot-main/drivers/i2c/omap24xx_i2c.c @@ -25,67 +25,64 @@ #include <asm/arch/i2c.h> #include <asm/io.h>
-#define inw(a) __raw_readw(a) -#define outw(a,v) __raw_writew(a,v)
-static void wait_for_bb (void); -static u16 wait_for_pin (void); +static void wait_for_bb(void); +static u16 wait_for_pin(void); static void flush_fifo(void);
-void i2c_init (int speed, int slaveadd) +void i2c_init(int speed, int slaveadd) { u16 scl;
- outw(0x2, I2C_SYSC); /* for ES2 after soft reset */
- writew(0x2, I2C_SYSC); /* for ES2 after soft reset */
NAK. You are mixing two different, unrelated things here: converting the code from in*/out*() to read*/write*() in one thing (= one patch), but the coding style cleanup is another, unrelated change (= separate patch, if you really find worth changing this - as far as I can see the file is consistent in the use of whitespace, so I don't see an urgency here).
But if you decide for the clean up, then please clean up spelling errors, too, like here:
/* have to read to clear intrrupt */
Thanks.
Best regards,
Wolfgang Denk
participants (2)
-
dirk.behmeï¼ googlemail.com
-
Wolfgang Denk