IB/ipath: Lock and always use shadow copies of GPIO register

The new LED blinking interface adds more contention for the
unprotected GPIO pins that were already shared, though not commonly at
the same time.  We add locks to the accesses to these pins so that
Read-Modify-Write is now safe.  Some of these locks are added at
interrupt context, so we shadow the registers which drive and inspect
these pins to avoid the mmio read/writes.  This mitigates the effects
of the locks and hastens us through the interrupt.

Add locking and always use shadows for registers controlling GPIO pins
(ExtCtrl and GPIOout). The use of shadows implies doing less I/O,
which can make I2C operation too fast on some platforms. An explicit
udelay(1) in SCL manipulation fixes that.

Signed-off-by: Michael Albaugh <michael.albaugh@qlogic.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/hw/ipath/ipath_eeprom.c b/drivers/infiniband/hw/ipath/ipath_eeprom.c
index 030185f90ee2b6ffe9ef70de2dc61ed4aa9c1c65..26daac9d8b636be8edf711470793718a81798f0e 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_eeprom.c
+++ b/drivers/infiniband/hw/ipath/ipath_eeprom.c
@@ -95,39 +95,37 @@ static int i2c_gpio_set(struct ipath_devdata *dd,
                        enum i2c_type line,
                        enum i2c_state new_line_state)
 {
-       u64 read_val, write_val, mask, *gpioval;
+       u64 out_mask, dir_mask, *gpioval;
+       unsigned long flags = 0;
 
        gpioval = &dd->ipath_gpio_out;
-       read_val = ipath_read_kreg64(dd, dd->ipath_kregs->kr_extctrl);
-       if (line == i2c_line_scl)
-               mask = dd->ipath_gpio_scl;
-       else
-               mask = dd->ipath_gpio_sda;
 
-       if (new_line_state == i2c_line_high)
+       if (line == i2c_line_scl) {
+               dir_mask = dd->ipath_gpio_scl;
+               out_mask = (1UL << dd->ipath_gpio_scl_num);
+       } else {
+               dir_mask = dd->ipath_gpio_sda;
+               out_mask = (1UL << dd->ipath_gpio_sda_num);
+       }
+
+       spin_lock_irqsave(&dd->ipath_gpio_lock, flags);
+       if (new_line_state == i2c_line_high) {
                /* tri-state the output rather than force high */
-               write_val = read_val & ~mask;
-       else
+               dd->ipath_extctrl &= ~dir_mask;
+       } else {
                /* config line to be an output */
-               write_val = read_val | mask;
-       ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, write_val);
+               dd->ipath_extctrl |= dir_mask;
+       }
+       ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, dd->ipath_extctrl);
 
-       /* set high and verify */
+       /* set output as well (no real verify) */
        if (new_line_state == i2c_line_high)
-               write_val = 0x1UL;
+               *gpioval |= out_mask;
        else
-               write_val = 0x0UL;
+               *gpioval &= ~out_mask;
 
-       if (line == i2c_line_scl) {
-               write_val <<= dd->ipath_gpio_scl_num;
-               *gpioval = *gpioval & ~(1UL << dd->ipath_gpio_scl_num);
-               *gpioval |= write_val;
-       } else {
-               write_val <<= dd->ipath_gpio_sda_num;
-               *gpioval = *gpioval & ~(1UL << dd->ipath_gpio_sda_num);
-               *gpioval |= write_val;
-       }
        ipath_write_kreg(dd, dd->ipath_kregs->kr_gpio_out, *gpioval);
+       spin_unlock_irqrestore(&dd->ipath_gpio_lock, flags);
 
        return 0;
 }
@@ -145,8 +143,9 @@ static int i2c_gpio_get(struct ipath_devdata *dd,
                        enum i2c_type line,
                        enum i2c_state *curr_statep)
 {
-       u64 read_val, write_val, mask;
+       u64 read_val, mask;
        int ret;
+       unsigned long flags = 0;
 
        /* check args */
        if (curr_statep == NULL) {
@@ -154,15 +153,21 @@ static int i2c_gpio_get(struct ipath_devdata *dd,
                goto bail;
        }
 
-       read_val = ipath_read_kreg64(dd, dd->ipath_kregs->kr_extctrl);
        /* config line to be an input */
        if (line == i2c_line_scl)
                mask = dd->ipath_gpio_scl;
        else
                mask = dd->ipath_gpio_sda;
-       write_val = read_val & ~mask;
-       ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, write_val);
+
+       spin_lock_irqsave(&dd->ipath_gpio_lock, flags);
+       dd->ipath_extctrl &= ~mask;
+       ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, dd->ipath_extctrl);
+       /*
+        * Below is very unlikely to reflect true input state if Output
+        * Enable actually changed.
+        */
        read_val = ipath_read_kreg64(dd, dd->ipath_kregs->kr_extstatus);
+       spin_unlock_irqrestore(&dd->ipath_gpio_lock, flags);
 
        if (read_val & mask)
                *curr_statep = i2c_line_high;
@@ -192,6 +197,7 @@ static void i2c_wait_for_writes(struct ipath_devdata *dd)
 
 static void scl_out(struct ipath_devdata *dd, u8 bit)
 {
+       udelay(1);
        i2c_gpio_set(dd, i2c_line_scl, bit ? i2c_line_high : i2c_line_low);
 
        i2c_wait_for_writes(dd);
@@ -314,12 +320,18 @@ static int eeprom_reset(struct ipath_devdata *dd)
        int clock_cycles_left = 9;
        u64 *gpioval = &dd->ipath_gpio_out;
        int ret;
+       unsigned long flags;
 
-       eeprom_init = 1;
+       spin_lock_irqsave(&dd->ipath_gpio_lock, flags);
+       /* Make sure shadows are consistent */
+       dd->ipath_extctrl = ipath_read_kreg64(dd, dd->ipath_kregs->kr_extctrl);
        *gpioval = ipath_read_kreg64(dd, dd->ipath_kregs->kr_gpio_out);
+       spin_unlock_irqrestore(&dd->ipath_gpio_lock, flags);
+
        ipath_cdbg(VERBOSE, "Resetting i2c eeprom; initial gpioout reg "
                   "is %llx\n", (unsigned long long) *gpioval);
 
+       eeprom_init = 1;
        /*
         * This is to get the i2c into a known state, by first going low,
         * then tristate sda (and then tristate scl as first thing

diff --git a/drivers/infiniband/hw/ipath/ipath_iba6110.c b/drivers/infiniband/hw/ipath/ipath_iba6110.c
index 4372c6c50ff6543b037fcf9d831d9315716f130f..8482ea366fb1306e3bee2eaf7c47469bc0d705c3 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_iba6110.c
+++ b/drivers/infiniband/hw/ipath/ipath_iba6110.c
@@ -1059,6 +1059,7 @@ static void ipath_setup_ht_setextled(struct ipath_devdata *dd,
                                     u64 lst, u64 ltst)
 {
        u64 extctl;
+       unsigned long flags = 0;
 
        /* the diags use the LED to indicate diag info, so we leave
         * the external LED alone when the diags are running */
@@ -1075,6 +1076,7 @@ static void ipath_setup_ht_setextled(struct ipath_devdata *dd,
                        : INFINIPATH_IBCS_L_STATE_DOWN;
        }
 
+       spin_lock_irqsave(&dd->ipath_gpio_lock, flags);
        /*
         * start by setting both LED control bits to off, then turn
         * on the appropriate bit(s).
@@ -1103,6 +1105,7 @@ static void ipath_setup_ht_setextled(struct ipath_devdata *dd,
        }
        dd->ipath_extctrl = extctl;
        ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, extctl);
+       spin_unlock_irqrestore(&dd->ipath_gpio_lock, flags);
 }
 
 static void ipath_init_ht_variables(struct ipath_devdata *dd)

diff --git a/drivers/infiniband/hw/ipath/ipath_iba6120.c b/drivers/infiniband/hw/ipath/ipath_iba6120.c
index bcb70d67dbb9753f6d63775673e1d67fcccfa51b..2345bb011acdcc13e39e30fe2b4e322c45b4eaea 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_iba6120.c
+++ b/drivers/infiniband/hw/ipath/ipath_iba6120.c
@@ -791,6 +791,7 @@ static void ipath_setup_pe_setextled(struct ipath_devdata *dd, u64 lst,
                                     u64 ltst)
 {
        u64 extctl;
+       unsigned long flags = 0;
 
        /* the diags use the LED to indicate diag info, so we leave
         * the external LED alone when the diags are running */
@@ -807,6 +808,7 @@ static void ipath_setup_pe_setextled(struct ipath_devdata *dd, u64 lst,
                        : INFINIPATH_IBCS_L_STATE_DOWN;
        }
 
+       spin_lock_irqsave(&dd->ipath_gpio_lock, flags);
        extctl = dd->ipath_extctrl & ~(INFINIPATH_EXTC_LED1PRIPORT_ON |
                                       INFINIPATH_EXTC_LED2PRIPORT_ON);
 
@@ -816,6 +818,7 @@ static void ipath_setup_pe_setextled(struct ipath_devdata *dd, u64 lst,
                extctl |= INFINIPATH_EXTC_LED1PRIPORT_ON;
        dd->ipath_extctrl = extctl;
        ipath_write_kreg(dd, dd->ipath_kregs->kr_extctrl, extctl);
+       spin_unlock_irqrestore(&dd->ipath_gpio_lock, flags);
 }
 
 /**

diff --git a/drivers/infiniband/hw/ipath/ipath_init_chip.c b/drivers/infiniband/hw/ipath/ipath_init_chip.c
index 7045ba6894944dd3f1a593661ef083b74069bb6d..f6ee7a83595a96626ea29fb3e7047fe8b8b345fb 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_init_chip.c
+++ b/drivers/infiniband/hw/ipath/ipath_init_chip.c
@@ -340,6 +340,8 @@ static int init_chip_first(struct ipath_devdata *dd,
 
        spin_lock_init(&dd->ipath_tid_lock);
 
+       spin_lock_init(&dd->ipath_gpio_lock);
+
 done:
        *pdp = pd;
        return ret;

diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h
index 2f39db7df31cd4c5cc520c4907628051909953b9..bd1088a9989152f2731bf5a2ccc55c69d3185e85 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_kernel.h
+++ b/drivers/infiniband/hw/ipath/ipath_kernel.h
@@ -399,6 +399,8 @@ struct ipath_devdata {
        u64 ipath_gpio_out;
        /* shadow the gpio mask register */
        u64 ipath_gpio_mask;
+       /* shadow the gpio output enable, etc... */
+       u64 ipath_extctrl;
        /* kr_revision shadow */
        u64 ipath_revision;
        /*
@@ -473,8 +475,6 @@ struct ipath_devdata {
        u32 ipath_cregbase;
        /* shadow the control register contents */
        u32 ipath_control;
-       /* shadow the gpio output contents */
-       u32 ipath_extctrl;
        /* PCI revision register (HTC rev on FPGA) */
        u32 ipath_pcirev;
 
@@ -576,6 +576,9 @@ struct ipath_devdata {
        u64 ipath_gpio_sda;
        u64 ipath_gpio_scl;
 
+       /* lock for doing RMW of shadows/regs for ExtCtrl and GPIO */
+       spinlock_t ipath_gpio_lock;
+
        /* used to override LED behavior */
        u8 ipath_led_override;  /* Substituted for normal value, if non-zero */
        u16 ipath_led_override_timeoff; /* delta to next timer event */