Re: [PATCH] i2c-ibm_iic driver bonus patch

2008-02-18 Thread Sean MacLennan
Here is an optional bonus patch that cleans up most of the checkpatch 
warnings in the common code. I left in the volatiles, since I don't 
understand why they where needed. The memory always seems to be access 
with in_8 and out_8, which are declared volatile. But they could be 
there to fix a very specific bug.

Cheers,
   Sean

Signed-off-by: Sean MacLennan [EMAIL PROTECTED]
---
--- for-2.6.25/drivers/i2c/busses/submitted-i2c-ibm_iic.c   2008-02-18 
20:44:06.0 -0500
+++ for-2.6.25/drivers/i2c/busses/i2c-ibm_iic.c 2008-02-18 20:53:53.0 
-0500
@@ -76,17 +76,17 @@
 #endif
 
 #if DBG_LEVEL  0
-#  define DBG(f,x...)  printk(KERN_DEBUG ibm-iic f, ##x)
+#  define DBG(f, x...) printk(KERN_DEBUG ibm-iic f, ##x)
 #else
-#  define DBG(f,x...)  ((void)0)
+#  define DBG(f, x...) ((void)0)
 #endif
 #if DBG_LEVEL  1
-#  define DBG2(f,x...) DBG(f, ##x)
+#  define DBG2(f, x...)DBG(f, ##x)
 #else
-#  define DBG2(f,x...) ((void)0)
+#  define DBG2(f, x...)((void)0)
 #endif
 #if DBG_LEVEL  2
-static void dump_iic_regs(const char* header, struct ibm_iic_private* dev)
+static void dump_iic_regs(const char *header, struct ibm_iic_private *dev)
 {
volatile struct iic_regs __iomem *iic = dev-vaddr;
printk(KERN_DEBUG ibm-iic%d: %s\n, dev-idx, header);
@@ -98,9 +98,9 @@
in_8(iic-extsts), in_8(iic-clkdiv), in_8(iic-xfrcnt),
in_8(iic-xtcntlss), in_8(iic-directcntl));
 }
-#  define DUMP_REGS(h,dev) dump_iic_regs((h),(dev))
+#  define DUMP_REGS(h, dev)dump_iic_regs((h), (dev))
 #else
-#  define DUMP_REGS(h,dev) ((void)0)
+#  define DUMP_REGS(h, dev)((void)0)
 #endif
 
 /* Bus timings (in ns) for bit-banging */
@@ -111,25 +111,26 @@
unsigned int high;
unsigned int buf;
 } timings [] = {
-/* Standard mode (100 KHz) */
-{
-   .hd_sta = 4000,
-   .su_sto = 4000,
-   .low= 4700,
-   .high   = 4000,
-   .buf= 4700,
-},
-/* Fast mode (400 KHz) */
-{
-   .hd_sta = 600,
-   .su_sto = 600,
-   .low= 1300,
-   .high   = 600,
-   .buf= 1300,
-}};
+   /* Standard mode (100 KHz) */
+   {
+   .hd_sta = 4000,
+   .su_sto = 4000,
+   .low= 4700,
+   .high   = 4000,
+   .buf= 4700,
+   },
+   /* Fast mode (400 KHz) */
+   {
+   .hd_sta = 600,
+   .su_sto = 600,
+   .low= 1300,
+   .high   = 600,
+   .buf= 1300,
+   }
+};
 
 /* Enable/disable interrupt generation */
-static inline void iic_interrupt_mode(struct ibm_iic_private* dev, int enable)
+static inline void iic_interrupt_mode(struct ibm_iic_private *dev, int enable)
 {
out_8(dev-vaddr-intmsk, enable ? INTRMSK_EIMTC : 0);
 }
@@ -137,7 +138,7 @@
 /*
  * Initialize IIC interface.
  */
-static void iic_dev_init(struct ibm_iic_private* dev)
+static void iic_dev_init(struct ibm_iic_private *dev)
 {
volatile struct iic_regs __iomem *iic = dev-vaddr;
 
@@ -182,7 +183,7 @@
 /*
  * Reset IIC interface
  */
-static void iic_dev_reset(struct ibm_iic_private* dev)
+static void iic_dev_reset(struct ibm_iic_private *dev)
 {
volatile struct iic_regs __iomem *iic = dev-vaddr;
int i;
@@ -191,19 +192,19 @@
DBG(%d: soft reset\n, dev-idx);
DUMP_REGS(reset, dev);
 
-   /* Place chip in the reset state */
+   /* Place chip in the reset state */
out_8(iic-xtcntlss, XTCNTLSS_SRST);
 
/* Check if bus is free */
dc = in_8(iic-directcntl);
-   if (!DIRCTNL_FREE(dc)){
+   if (!DIRCTNL_FREE(dc)) {
DBG(%d: trying to regain bus control\n, dev-idx);
 
/* Try to set bus free state */
out_8(iic-directcntl, DIRCNTL_SDAC | DIRCNTL_SCC);
 
/* Wait until we regain bus control */
-   for (i = 0; i  100; ++i){
+   for (i = 0; i  100; ++i) {
dc = in_8(iic-directcntl);
if (DIRCTNL_FREE(dc))
break;
@@ -235,7 +236,7 @@
 static int iic_dc_wait(volatile struct iic_regs __iomem *iic, u8 mask)
 {
unsigned long x = jiffies + HZ / 28 + 2;
-   while ((in_8(iic-directcntl)  mask) != mask){
+   while ((in_8(iic-directcntl)  mask) != mask) {
if (unlikely(time_after(jiffies, x)))
return -1;
cond_resched();
@@ -243,15 +244,15 @@
return 0;
 }
 
-static int iic_smbus_quick(struct ibm_iic_private* dev, const struct i2c_msg* 
p)
+static int iic_smbus_quick(struct ibm_iic_private *dev, const struct i2c_msg 
*p)
 {
volatile struct iic_regs __iomem *iic = dev-vaddr;
-   const struct i2c_timings* t = timings[dev-fast_mode ? 1 : 0];
+   const struct i2c_timings *t = timings[dev-fast_mode ? 1 : 0];
u8 mask, v, sda;
int i, res;
 
/* Only 7-bit 

Re: [PATCH] i2c-ibm_iic driver bonus patch

2008-02-18 Thread Arnd Bergmann
On Tuesday 19 February 2008, Sean MacLennan wrote:
  I left in the volatiles, since I don't 
 understand why they where needed. The memory always seems to be access 
 with in_8 and out_8, which are declared volatile. But they could be 
 there to fix a very specific bug.

It's very unlikely that they were really needed, and you certainly
shouldn't mark data as volatile in new code. It's very common to
mark I/O data structures as volatile when they should be __iomem,
because that's what people learn at university, but that is never
the right solution, even if it can hide other bugs in your code.

Of course, unlike the other changes in your patch, it does impact
code generation, so if you want to change it, that should be a
separate patch.

Arnd 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev