As per TODO. This commit introduces no functional changes.
Signed-off-by: David Matlack <[email protected]>
---
drivers/staging/slicoss/TODO | 1 -
drivers/staging/slicoss/slic.h | 19 +++---
drivers/staging/slicoss/slicoss.c | 125 ++++++++++++++++++--------------------
3 files changed, 65 insertions(+), 80 deletions(-)
diff --git a/drivers/staging/slicoss/TODO b/drivers/staging/slicoss/TODO
index 20cc9ab..9019729 100644
--- a/drivers/staging/slicoss/TODO
+++ b/drivers/staging/slicoss/TODO
@@ -25,7 +25,6 @@ TODO:
- state variables for things that are
easily available and shouldn't be kept in card structure, cardnum, ...
slotnumber, events, ...
- - get rid of slic_spinlock wrapper
- volatile == bad design => bad code
- locking too fine grained, not designed just throw more locks
at problem
diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h
index 3a5aa88..5b23254 100644
--- a/drivers/staging/slicoss/slic.h
+++ b/drivers/staging/slicoss/slic.h
@@ -56,11 +56,6 @@ static u32 OasisRcvUCodeLen = 512;
static u32 GBRcvUCodeLen = 512;
#define SECTION_SIZE 65536
-struct slic_spinlock {
- spinlock_t lock;
- unsigned long flags;
-};
-
#define SLIC_RSPQ_PAGES_GB 10
#define SLIC_RSPQ_BUFSINPAGE (PAGE_SIZE / SLIC_RSPBUF_SIZE)
@@ -165,7 +160,7 @@ struct slic_cmdqueue {
struct slic_hostcmd *head;
struct slic_hostcmd *tail;
int count;
- struct slic_spinlock lock;
+ spinlock_t lock;
};
#define SLIC_MAX_CARDS 32
@@ -346,7 +341,7 @@ struct physcard {
};
struct base_driver {
- struct slic_spinlock driver_lock;
+ spinlock_t driver_lock;
u32 num_slic_cards;
u32 num_slic_ports;
u32 num_slic_ports_active;
@@ -401,8 +396,8 @@ struct adapter {
uint card_size;
uint chipid;
struct net_device *netdev;
- struct slic_spinlock adapter_lock;
- struct slic_spinlock reset_lock;
+ spinlock_t adapter_lock;
+ spinlock_t reset_lock;
struct pci_dev *pcidev;
uint busnumber;
uint slotnumber;
@@ -441,8 +436,8 @@ struct adapter {
u32 pingtimerset;
struct timer_list loadtimer;
u32 loadtimerset;
- struct slic_spinlock upr_lock;
- struct slic_spinlock bit64reglock;
+ spinlock_t upr_lock;
+ spinlock_t bit64reglock;
struct slic_rspqueue rspqueue;
struct slic_rcvqueue rcvqueue;
struct slic_cmdqueue cmdq_free;
@@ -457,7 +452,7 @@ struct adapter {
/* Free object handles*/
struct slic_handle *pfree_slic_handles;
/* Object handle list lock*/
- struct slic_spinlock handle_lock;
+ spinlock_t handle_lock;
ushort slic_handle_ix;
u32 xmitq_full;
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index c2bda1d..39c140c 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -144,8 +144,9 @@ static inline void slic_reg64_write(struct adapter *adapter, void __iomem *reg,
u32 value, void __iomem *regh, u32 paddrh,
bool flush)
{
- spin_lock_irqsave(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ unsigned long flags;
+
+ spin_lock_irqsave(&adapter->bit64reglock, flags);
if (paddrh != adapter->curaddrupper) {
adapter->curaddrupper = paddrh;
writel(paddrh, regh);
@@ -153,8 +154,7 @@ static inline void slic_reg64_write(struct adapter *adapter, void __iomem *reg,
writel(value, reg);
if (flush)
mb();
- spin_unlock_irqrestore(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ spin_unlock_irqrestore(&adapter->bit64reglock, flags);
}
static void slic_mcast_set_bit(struct adapter *adapter, char *address)
@@ -936,9 +936,10 @@ static int slic_upr_request(struct adapter *adapter,
u32 upr_data_h,
u32 upr_buffer, u32 upr_buffer_h)
{
+ unsigned long flags;
int rc;
- spin_lock_irqsave(&adapter->upr_lock.lock, adapter->upr_lock.flags);
+ spin_lock_irqsave(&adapter->upr_lock, flags);
rc = slic_upr_queue_request(adapter,
upr_request,
upr_data,
@@ -948,8 +949,7 @@ static int slic_upr_request(struct adapter *adapter,
slic_upr_start(adapter);
err_unlock_irq:
- spin_unlock_irqrestore(&adapter->upr_lock.lock,
- adapter->upr_lock.flags);
+ spin_unlock_irqrestore(&adapter->upr_lock, flags);
return rc;
}
@@ -1029,12 +1029,12 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
{
struct sliccard *card = adapter->card;
struct slic_upr *upr;
+ unsigned long flags;
- spin_lock_irqsave(&adapter->upr_lock.lock, adapter->upr_lock.flags);
+ spin_lock_irqsave(&adapter->upr_lock, flags);
upr = adapter->upr_list;
if (!upr) {
- spin_unlock_irqrestore(&adapter->upr_lock.lock,
- adapter->upr_lock.flags);
+ spin_unlock_irqrestore(&adapter->upr_lock, flags);
return;
}
adapter->upr_list = upr->next;
@@ -1127,8 +1127,7 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
}
kfree(upr);
slic_upr_start(adapter);
- spin_unlock_irqrestore(&adapter->upr_lock.lock,
- adapter->upr_lock.flags);
+ spin_unlock_irqrestore(&adapter->upr_lock, flags);
}
static int slic_config_get(struct adapter *adapter, u32 config, u32 config_h)
@@ -1310,6 +1309,7 @@ static void slic_cmdq_addcmdpage(struct adapter *adapter, u32 *page)
u32 phys_addrl;
u32 phys_addrh;
struct slic_handle *pslic_handle;
+ unsigned long flags;
cmdaddr = page;
cmd = (struct slic_hostcmd *)cmdaddr;
@@ -1324,12 +1324,10 @@ static void slic_cmdq_addcmdpage(struct adapter *adapter, u32 *page)
while ((cmdcnt < SLIC_CMDQ_CMDSINPAGE) &&
(adapter->slic_handle_ix < 256)) {
/* Allocate and initialize a SLIC_HANDLE for this command */
- spin_lock_irqsave(&adapter->handle_lock.lock,
- adapter->handle_lock.flags);
+ spin_lock_irqsave(&adapter->handle_lock, flags);
pslic_handle = adapter->pfree_slic_handles;
adapter->pfree_slic_handles = pslic_handle->next;
- spin_unlock_irqrestore(&adapter->handle_lock.lock,
- adapter->handle_lock.flags);
+ spin_unlock_irqrestore(&adapter->handle_lock, flags);
pslic_handle->type = SLIC_HANDLE_CMD;
pslic_handle->address = (void *) cmd;
pslic_handle->offset = (ushort) adapter->slic_handle_ix++;
@@ -1356,11 +1354,11 @@ static void slic_cmdq_addcmdpage(struct adapter *adapter, u32 *page)
tail->next_all = cmdq->head;
cmdq->head = prev;
cmdq = &adapter->cmdq_free;
- spin_lock_irqsave(&cmdq->lock.lock, cmdq->lock.flags);
+ spin_lock_irqsave(&cmdq->lock, flags);
cmdq->count += cmdcnt; /* SLIC_CMDQ_CMDSINPAGE; mooktodo */
tail->next = cmdq->head;
cmdq->head = prev;
- spin_unlock_irqrestore(&cmdq->lock.lock, cmdq->lock.flags);
+ spin_unlock_irqrestore(&cmdq->lock, flags);
}
static int slic_cmdq_init(struct adapter *adapter)
@@ -1371,9 +1369,9 @@ static int slic_cmdq_init(struct adapter *adapter)
memset(&adapter->cmdq_all, 0, sizeof(struct slic_cmdqueue));
memset(&adapter->cmdq_free, 0, sizeof(struct slic_cmdqueue));
memset(&adapter->cmdq_done, 0, sizeof(struct slic_cmdqueue));
- spin_lock_init(&adapter->cmdq_all.lock.lock);
- spin_lock_init(&adapter->cmdq_free.lock.lock);
- spin_lock_init(&adapter->cmdq_done.lock.lock);
+ spin_lock_init(&adapter->cmdq_all.lock);
+ spin_lock_init(&adapter->cmdq_free.lock);
+ spin_lock_init(&adapter->cmdq_done.lock);
memset(&adapter->cmdqmem, 0, sizeof(struct slic_cmdqmem));
adapter->slic_handle_ix = 1;
for (i = 0; i < SLIC_CMDQ_INITPAGES; i++) {
@@ -1394,11 +1392,10 @@ static void slic_cmdq_reset(struct adapter *adapter)
struct slic_hostcmd *hcmd;
struct sk_buff *skb;
u32 outstanding;
+ unsigned long flags;
- spin_lock_irqsave(&adapter->cmdq_free.lock.lock,
- adapter->cmdq_free.lock.flags);
- spin_lock_irqsave(&adapter->cmdq_done.lock.lock,
- adapter->cmdq_done.lock.flags);
+ spin_lock_irqsave(&adapter->cmdq_free.lock, flags);
+ spin_lock_irqsave(&adapter->cmdq_done.lock, flags);
outstanding = adapter->cmdq_all.count - adapter->cmdq_done.count;
outstanding -= adapter->cmdq_free.count;
hcmd = adapter->cmdq_all.head;
@@ -1429,40 +1426,40 @@ static void slic_cmdq_reset(struct adapter *adapter)
"free_count %d != all count %d\n",
adapter->cmdq_free.count, adapter->cmdq_all.count);
}
- spin_unlock_irqrestore(&adapter->cmdq_done.lock.lock,
- adapter->cmdq_done.lock.flags);
- spin_unlock_irqrestore(&adapter->cmdq_free.lock.lock,
- adapter->cmdq_free.lock.flags);
+ spin_unlock_irqrestore(&adapter->cmdq_done.lock, flags);
+ spin_unlock_irqrestore(&adapter->cmdq_free.lock, flags);
}
static void slic_cmdq_getdone(struct adapter *adapter)
{
struct slic_cmdqueue *done_cmdq = &adapter->cmdq_done;
struct slic_cmdqueue *free_cmdq = &adapter->cmdq_free;
+ unsigned long flags;
- spin_lock_irqsave(&done_cmdq->lock.lock, done_cmdq->lock.flags);
+ spin_lock_irqsave(&done_cmdq->lock, flags);
free_cmdq->head = done_cmdq->head;
free_cmdq->count = done_cmdq->count;
done_cmdq->head = NULL;
done_cmdq->tail = NULL;
done_cmdq->count = 0;
- spin_unlock_irqrestore(&done_cmdq->lock.lock, done_cmdq->lock.flags);
+ spin_unlock_irqrestore(&done_cmdq->lock, flags);
}
static struct slic_hostcmd *slic_cmdq_getfree(struct adapter *adapter)
{
struct slic_cmdqueue *cmdq = &adapter->cmdq_free;
struct slic_hostcmd *cmd = NULL;
+ unsigned long flags;
lock_and_retry:
- spin_lock_irqsave(&cmdq->lock.lock, cmdq->lock.flags);
+ spin_lock_irqsave(&cmdq->lock, flags);
retry:
cmd = cmdq->head;
if (cmd) {
cmdq->head = cmd->next;
cmdq->count--;
- spin_unlock_irqrestore(&cmdq->lock.lock, cmdq->lock.flags);
+ spin_unlock_irqrestore(&cmdq->lock, flags);
} else {
slic_cmdq_getdone(adapter);
cmd = cmdq->head;
@@ -1471,8 +1468,7 @@ retry:
} else {
u32 *pageaddr;
- spin_unlock_irqrestore(&cmdq->lock.lock,
- cmdq->lock.flags);
+ spin_unlock_irqrestore(&cmdq->lock, flags);
pageaddr = slic_cmdqmem_addpage(adapter);
if (pageaddr) {
slic_cmdq_addcmdpage(adapter, pageaddr);
@@ -1488,14 +1484,14 @@ static void slic_cmdq_putdone_irq(struct adapter *adapter,
{
struct slic_cmdqueue *cmdq = &adapter->cmdq_done;
- spin_lock(&cmdq->lock.lock);
+ spin_lock(&cmdq->lock);
cmd->busy = 0;
cmd->next = cmdq->head;
cmdq->head = cmd;
cmdq->count++;
if ((adapter->xmitq_full) && (cmdq->count > 10))
netif_wake_queue(adapter->netdev);
- spin_unlock(&cmdq->lock.lock);
+ spin_unlock(&cmdq->lock);
}
static int slic_rcvqueue_fill(struct adapter *adapter)
@@ -2250,21 +2246,20 @@ static void slic_adapter_freeresources(struct adapter *adapter)
adapter->rcv_unicasts = 0;
}
-static int slic_adapter_allocresources(struct adapter *adapter)
+static int slic_adapter_allocresources(struct adapter *adapter,
+ unsigned long *flags)
{
if (!adapter->intrregistered) {
int retval;
- spin_unlock_irqrestore(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_unlock_irqrestore(&slic_global.driver_lock, *flags);
retval = request_irq(adapter->netdev->irq,
&slic_interrupt,
IRQF_SHARED,
adapter->netdev->name, adapter->netdev);
- spin_lock_irqsave(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_lock_irqsave(&slic_global.driver_lock, *flags);
if (retval) {
dev_err(&adapter->netdev->dev,
@@ -2283,7 +2278,7 @@ static int slic_adapter_allocresources(struct adapter *adapter)
* Perform initialization of our slic interface.
*
*/
-static int slic_if_init(struct adapter *adapter)
+static int slic_if_init(struct adapter *adapter, unsigned long *flags)
{
struct sliccard *card = adapter->card;
struct net_device *dev = adapter->netdev;
@@ -2311,7 +2306,7 @@ static int slic_if_init(struct adapter *adapter)
if (dev->flags & IFF_MULTICAST)
adapter->macopts |= MAC_MCAST;
}
- rc = slic_adapter_allocresources(adapter);
+ rc = slic_adapter_allocresources(adapter, flags);
if (rc) {
dev_err(&dev->dev, "slic_adapter_allocresources FAILED %x\n",
rc);
@@ -2336,11 +2331,11 @@ static int slic_if_init(struct adapter *adapter)
mdelay(1);
if (!adapter->isp_initialized) {
+ unsigned long flags;
pshmem = (struct slic_shmem *)(unsigned long)
adapter->phys_shmem;
- spin_lock_irqsave(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ spin_lock_irqsave(&adapter->bit64reglock, flags);
#if BITS_PER_LONG == 64
slic_reg32_write(&slic_regs->slic_addr_upper,
@@ -2352,8 +2347,7 @@ static int slic_if_init(struct adapter *adapter)
slic_reg32_write(&slic_regs->slic_isp, (u32)&pshmem->isr,
FLUSH);
#endif
- spin_unlock_irqrestore(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ spin_unlock_irqrestore(&adapter->bit64reglock, flags);
adapter->isp_initialized = 1;
}
@@ -2396,18 +2390,18 @@ static int slic_entry_open(struct net_device *dev)
{
struct adapter *adapter = netdev_priv(dev);
struct sliccard *card = adapter->card;
+ unsigned long flags;
int status;
netif_stop_queue(adapter->netdev);
- spin_lock_irqsave(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_lock_irqsave(&slic_global.driver_lock, flags);
if (!adapter->activated) {
card->adapters_activated++;
slic_global.num_slic_ports_active++;
adapter->activated = 1;
}
- status = slic_if_init(adapter);
+ status = slic_if_init(adapter, &flags);
if (status != 0) {
if (adapter->activated) {
@@ -2421,8 +2415,7 @@ static int slic_entry_open(struct net_device *dev)
card->master = adapter;
spin_unlock:
- spin_unlock_irqrestore(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_unlock_irqrestore(&slic_global.driver_lock, flags);
return status;
}
@@ -2481,9 +2474,9 @@ static int slic_entry_halt(struct net_device *dev)
struct adapter *adapter = netdev_priv(dev);
struct sliccard *card = adapter->card;
__iomem struct slic_regs *slic_regs = adapter->slic_regs;
+ unsigned long flags;
- spin_lock_irqsave(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_lock_irqsave(&slic_global.driver_lock, flags);
netif_stop_queue(adapter->netdev);
adapter->state = ADAPT_DOWN;
adapter->linkstate = LINK_DOWN;
@@ -2512,8 +2505,7 @@ static int slic_entry_halt(struct net_device *dev)
slic_card_init(card, adapter);
#endif
- spin_unlock_irqrestore(&slic_global.driver_lock.lock,
- slic_global.driver_lock.flags);
+ spin_unlock_irqrestore(&slic_global.driver_lock, flags);
return 0;
}
@@ -2663,6 +2655,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
unsigned char oemfruformat;
struct atk_fru *patkfru;
union oemfru *poemfru;
+ unsigned long flags;
/* Reset everything except PCI configuration space */
slic_soft_reset(adapter);
@@ -2693,14 +2686,12 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
pshmem = (struct slic_shmem *)(unsigned long)
adapter->phys_shmem;
- spin_lock_irqsave(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ spin_lock_irqsave(&adapter->bit64reglock, flags);
slic_reg32_write(&slic_regs->slic_addr_upper,
SLIC_GET_ADDR_HIGH(&pshmem->isr), DONT_FLUSH);
slic_reg32_write(&slic_regs->slic_isp,
SLIC_GET_ADDR_LOW(&pshmem->isr), FLUSH);
- spin_unlock_irqrestore(&adapter->bit64reglock.lock,
- adapter->bit64reglock.flags);
+ spin_unlock_irqrestore(&adapter->bit64reglock, flags);
status = slic_config_get(adapter, phys_configl, phys_configh);
if (status) {
@@ -2854,7 +2845,7 @@ static void slic_init_driver(void)
{
if (slic_first_init) {
slic_first_init = 0;
- spin_lock_init(&slic_global.driver_lock.lock);
+ spin_lock_init(&slic_global.driver_lock);
}
}
@@ -2880,11 +2871,11 @@ static void slic_init_adapter(struct net_device *netdev,
adapter->chipid = chip_idx;
adapter->port = 0; /*adapter->functionnumber;*/
adapter->cardindex = adapter->port;
- spin_lock_init(&adapter->upr_lock.lock);
- spin_lock_init(&adapter->bit64reglock.lock);
- spin_lock_init(&adapter->adapter_lock.lock);
- spin_lock_init(&adapter->reset_lock.lock);
- spin_lock_init(&adapter->handle_lock.lock);
+ spin_lock_init(&adapter->upr_lock);
+ spin_lock_init(&adapter->bit64reglock);
+ spin_lock_init(&adapter->adapter_lock);
+ spin_lock_init(&adapter->reset_lock);
+ spin_lock_init(&adapter->handle_lock);
adapter->card_size = 1;
/*
--
2.4.0
curaddrupper caches the last written upper 32-bits of a dma address
(the device has one register for the upper 32-bits of all dma
address registers). The problem is, not every dma address write
checks and sets curaddrupper. This causes the driver to occasionally
not write the upper 32-bits of a dma address to the device when it
really should.
I've seen this manifest particularly when the driver is trying to
read config data from the device (RCONFIG) in order to checksum the
device's eeprom. Since the device writes its config data to the
wrong DMA address the driver reads 0 as the eeprom size and the
eeprom checksum fails.
This patch fixes the issue by removing curaddrupper and always
writing the upper 32-bits of dma addresses.
Signed-off-by: David Matlack <[email protected]>
---
drivers/staging/slicoss/slic.h | 1 -
drivers/staging/slicoss/slicoss.c | 5 +----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h
index 5b23254..67a8c9e 100644
--- a/drivers/staging/slicoss/slic.h
+++ b/drivers/staging/slicoss/slic.h
@@ -414,7 +414,6 @@ struct adapter {
u32 intrregistered;
uint isp_initialized;
uint gennumber;
- u32 curaddrupper;
struct slic_shmem *pshmem;
dma_addr_t phys_shmem;
u32 isrcopy;
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 39c140c..5f34ebbf 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -147,10 +147,7 @@ static inline void slic_reg64_write(struct adapter *adapter, void __iomem *reg,
unsigned long flags;
spin_lock_irqsave(&adapter->bit64reglock, flags);
- if (paddrh != adapter->curaddrupper) {
- adapter->curaddrupper = paddrh;
- writel(paddrh, regh);
- }
+ writel(paddrh, regh);
writel(value, reg);
if (flush)
mb();
--
2.4.0
On Mon, May 11, 2015 at 08:40:35PM -0700, David Matlack wrote:
> As per TODO. This commit introduces no functional changes.
>
It's easy for me to imagine that big shared "flags" variable in the
original code is racy so it might introduce a bugfix. :)
Nice improvement.
regards,
dan carpenter