2016-11-17 05:41:59

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v3 0/2] staging: slicoss: fix different address space warnings

This patchset fix the following sparse warning:
warning: incorrect type in assignment (different address spaces)

Changes in v2:
* Remove IOMEM_GET_FIELDADDR macro
* Add ioread64 and iowrite64 defines

Changes in v3:
* Remove ioread64 and iowrite64 defines
* Split into two patches: one for 32 bits stuff and the other for 64 bits.

Sergio Paracuellos (2):
staging: slicoss: fix different address space warnings: 32 bits
staging: slicoss: fix different address space warnings: 64 bits

drivers/staging/slicoss/slic.h | 18 +++++++++
drivers/staging/slicoss/slicoss.c | 82 ++++++++++++++++++++++-----------------
2 files changed, 65 insertions(+), 35 deletions(-)

--
1.9.1


2016-11-17 05:42:05

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v3 2/2] staging: slicoss: fix different address space warnings: 64 bits

This patch fix the following sparse warnings in slicoss driver:
warning: incorrect type in assignment (different address spaces)

This changes are for 64 bits.

Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/staging/slicoss/slic.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h
index d637ab1..63637ba 100644
--- a/drivers/staging/slicoss/slic.h
+++ b/drivers/staging/slicoss/slic.h
@@ -540,6 +540,7 @@ static inline void slic_flush_write(struct adapter *adapter)
ioread32(adapter->regs + SLIC_REG_HOSTID);
}

+#if BITS_PER_LONG == 32
static inline u64 slic_ioread64(void __iomem *mmio)
{
u64 low, high;
@@ -548,6 +549,14 @@ static inline u64 slic_ioread64(void __iomem *mmio)
high = ioread32(mmio + sizeof(u32));
return low | (high << 32);
}
+#elif BITS_PER_LONG == 64
+static inline u64 slic_ioread64(void __iomem *mmio)
+{
+ return readq(mmio);
+}
+#else
+#error BITS_PER_LONG must be 32 or 64
+#endif

#define UPDATE_STATS(largestat, newstat, oldstat) \
{ \
--
1.9.1

2016-11-17 05:42:23

by Sergio Paracuellos

[permalink] [raw]
Subject: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

This patch fix the following sparse warnings in slicoss driver:
warning: incorrect type in assignment (different address spaces)

This changes are for 32 bit architectures.

Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/staging/slicoss/slic.h | 9 +++++
drivers/staging/slicoss/slicoss.c | 82 ++++++++++++++++++++++-----------------
2 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/slicoss/slic.h b/drivers/staging/slicoss/slic.h
index 420546d..d637ab1 100644
--- a/drivers/staging/slicoss/slic.h
+++ b/drivers/staging/slicoss/slic.h
@@ -540,6 +540,15 @@ static inline void slic_flush_write(struct adapter *adapter)
ioread32(adapter->regs + SLIC_REG_HOSTID);
}

+static inline u64 slic_ioread64(void __iomem *mmio)
+{
+ u64 low, high;
+
+ low = ioread32(mmio);
+ high = ioread32(mmio + sizeof(u32));
+ return low | (high << 32);
+}
+
#define UPDATE_STATS(largestat, newstat, oldstat) \
{ \
if ((newstat) < (oldstat)) \
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index d2929b9..c0bc1a8 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -923,8 +923,8 @@ static int slic_upr_request(struct adapter *adapter,
static void slic_link_upr_complete(struct adapter *adapter, u32 isr)
{
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
- u32 lst = sm_data->lnkstatus;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;
+ u32 lst = ioread32(&sm_data->lnkstatus);
uint linkup;
unsigned char linkspeed;
unsigned char linkduplex;
@@ -1003,8 +1003,8 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
switch (upr->upr_request) {
case SLIC_UPR_STATS: {
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
- struct slic_stats *stats = &sm_data->stats;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;
+ struct slic_stats __iomem *stats = &sm_data->stats;
struct slic_stats *old = &adapter->inicstats_prev;
struct slicnet_stats *stst = &adapter->slic_stats;

@@ -1014,50 +1014,62 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
break;
}

- UPDATE_STATS_GB(stst->tcp.xmit_tcp_segs, stats->xmit_tcp_segs,
+ UPDATE_STATS_GB(stst->tcp.xmit_tcp_segs,
+ slic_ioread64(&stats->xmit_tcp_segs),
old->xmit_tcp_segs);

- UPDATE_STATS_GB(stst->tcp.xmit_tcp_bytes, stats->xmit_tcp_bytes,
+ UPDATE_STATS_GB(stst->tcp.xmit_tcp_bytes,
+ slic_ioread64(&stats->xmit_tcp_bytes),
old->xmit_tcp_bytes);

- UPDATE_STATS_GB(stst->tcp.rcv_tcp_segs, stats->rcv_tcp_segs,
+ UPDATE_STATS_GB(stst->tcp.rcv_tcp_segs,
+ slic_ioread64(&stats->rcv_tcp_segs),
old->rcv_tcp_segs);

- UPDATE_STATS_GB(stst->tcp.rcv_tcp_bytes, stats->rcv_tcp_bytes,
+ UPDATE_STATS_GB(stst->tcp.rcv_tcp_bytes,
+ slic_ioread64(&stats->rcv_tcp_bytes),
old->rcv_tcp_bytes);

- UPDATE_STATS_GB(stst->iface.xmt_bytes, stats->xmit_bytes,
+ UPDATE_STATS_GB(stst->iface.xmt_bytes,
+ slic_ioread64(&stats->xmit_bytes),
old->xmit_bytes);

- UPDATE_STATS_GB(stst->iface.xmt_ucast, stats->xmit_unicasts,
+ UPDATE_STATS_GB(stst->iface.xmt_ucast,
+ slic_ioread64(&stats->xmit_unicasts),
old->xmit_unicasts);

- UPDATE_STATS_GB(stst->iface.rcv_bytes, stats->rcv_bytes,
+ UPDATE_STATS_GB(stst->iface.rcv_bytes,
+ slic_ioread64(&stats->rcv_bytes),
old->rcv_bytes);

- UPDATE_STATS_GB(stst->iface.rcv_ucast, stats->rcv_unicasts,
+ UPDATE_STATS_GB(stst->iface.rcv_ucast,
+ slic_ioread64(&stats->rcv_unicasts),
old->rcv_unicasts);

- UPDATE_STATS_GB(stst->iface.xmt_errors, stats->xmit_collisions,
+ UPDATE_STATS_GB(stst->iface.xmt_errors,
+ slic_ioread64(&stats->xmit_collisions),
old->xmit_collisions);

UPDATE_STATS_GB(stst->iface.xmt_errors,
- stats->xmit_excess_collisions,
+ slic_ioread64(&stats->xmit_excess_collisions),
old->xmit_excess_collisions);

- UPDATE_STATS_GB(stst->iface.xmt_errors, stats->xmit_other_error,
+ UPDATE_STATS_GB(stst->iface.xmt_errors,
+ slic_ioread64(&stats->xmit_other_error),
old->xmit_other_error);

- UPDATE_STATS_GB(stst->iface.rcv_errors, stats->rcv_other_error,
+ UPDATE_STATS_GB(stst->iface.rcv_errors,
+ slic_ioread64(&stats->rcv_other_error),
old->rcv_other_error);

- UPDATE_STATS_GB(stst->iface.rcv_discards, stats->rcv_drops,
+ UPDATE_STATS_GB(stst->iface.rcv_discards,
+ slic_ioread64(&stats->rcv_drops),
old->rcv_drops);

- if (stats->rcv_drops > old->rcv_drops)
- adapter->rcv_drops += (stats->rcv_drops -
+ if (slic_ioread64(&stats->rcv_drops) > old->rcv_drops)
+ adapter->rcv_drops += (slic_ioread64(&stats->rcv_drops) -
old->rcv_drops);
- memcpy(old, stats, sizeof(*stats));
+ memcpy_fromio(old, stats, sizeof(*stats));
break;
}
case SLIC_UPR_RLSR:
@@ -1679,10 +1691,10 @@ static void slic_init_cleanup(struct adapter *adapter)

if (adapter->shmem.shmem_data) {
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;

- pci_free_consistent(adapter->pcidev, sizeof(*sm_data), sm_data,
- sm->isr_phaddr);
+ pci_free_consistent(adapter->pcidev, sizeof(*sm_data),
+ (void __force *)sm_data, sm->isr_phaddr);
}

if (adapter->pingtimerset) {
@@ -2068,15 +2080,15 @@ static irqreturn_t slic_interrupt(int irq, void *dev_id)
struct net_device *dev = dev_id;
struct adapter *adapter = netdev_priv(dev);
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;
u32 isr;

- if (sm_data->isr) {
+ if (ioread32(&sm_data->isr)) {
slic_write32(adapter, SLIC_REG_ICR, ICR_INT_MASK);
slic_flush_write(adapter);

- isr = sm_data->isr;
- sm_data->isr = 0;
+ isr = ioread32(&sm_data->isr);
+ iowrite32(0, &sm_data->isr);
adapter->num_isrs++;
switch (adapter->card->state) {
case CARD_UP:
@@ -2211,7 +2223,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
struct sliccard *card = adapter->card;
struct net_device *dev = adapter->netdev;
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;
int rc;

/* adapter should be down at this point */
@@ -2295,7 +2307,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
/*
* clear any pending events, then enable interrupts
*/
- sm_data->isr = 0;
+ iowrite32(0, &sm_data->isr);
slic_write32(adapter, SLIC_REG_ISR, 0);
slic_write32(adapter, SLIC_REG_ICR, ICR_INT_ON);

@@ -2584,7 +2596,7 @@ static void slic_config_pci(struct pci_dev *pcidev)
static int slic_card_init(struct sliccard *card, struct adapter *adapter)
{
struct slic_shmemory *sm = &adapter->shmem;
- struct slic_shmem_data *sm_data = sm->shmem_data;
+ struct slic_shmem_data __iomem *sm_data = sm->shmem_data;
struct slic_eeprom *peeprom;
struct oslic_eeprom *pOeeprom;
dma_addr_t phys_config;
@@ -2647,9 +2659,9 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
}

for (;;) {
- if (sm_data->isr) {
- if (sm_data->isr & ISR_UPC) {
- sm_data->isr = 0;
+ if (ioread32(&sm_data->isr)) {
+ if (ioread32(&sm_data->isr) & ISR_UPC) {
+ iowrite32(0, &sm_data->isr);
slic_write64(adapter, SLIC_REG_ISP, 0,
0);
slic_write32(adapter, SLIC_REG_ISR, 0);
@@ -2659,7 +2671,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
break;
}

- sm_data->isr = 0;
+ iowrite32(0, &sm_data->isr);
slic_write32(adapter, SLIC_REG_ISR, 0);
slic_flush_write(adapter);
} else {
@@ -2862,7 +2874,7 @@ static int slic_init_adapter(struct net_device *netdev,
if (!sm_data)
return -ENOMEM;

- sm->shmem_data = sm_data;
+ sm->shmem_data = (struct slic_shmem_data __force __iomem *)sm_data;
sm->isr_phaddr = phaddr;
sm->lnkstatus_phaddr = phaddr + offsetof(struct slic_shmem_data,
lnkstatus);
--
1.9.1

2016-11-17 10:52:46

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

Hm... I seem to duplicate the Sparse warnings at all. That's weird.

Anyway as far as I can tell the warnings are false positives... Can't
you just remove the __iomem tag? Have you tested this change?

regards,
dan carpenter

2016-11-17 11:22:03

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

On Thu, Nov 17, 2016 at 11:51 AM, Dan Carpenter
<[email protected]> wrote:
> Hm... I seem to duplicate the Sparse warnings at all. That's weird.
>

With this patch applied, sparse warnings dissapear for me... That's weird.

> Anyway as far as I can tell the warnings are false positives... Can't
> you just remove the __iomem tag? Have you tested this change?
>
I haven' t tested this change because I can't test it :(. Do you think
that just removing
__iomem tag is enough? Maybe we can get feedback from Lior or someone.

Thanks for your review time, Dan.

Cheers,
Sergio Paracuellos

2016-11-17 11:33:23

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

On Thu, Nov 17, 2016 at 12:22:00PM +0100, Sergio Paracuellos wrote:
> On Thu, Nov 17, 2016 at 11:51 AM, Dan Carpenter
> <[email protected]> wrote:
> > Hm... I seem to duplicate the Sparse warnings at all. That's weird.
> >
>
> With this patch applied, sparse warnings dissapear for me... That's weird.
>

I understand that.

> > Anyway as far as I can tell the warnings are false positives... Can't
> > you just remove the __iomem tag? Have you tested this change?
> >
> I haven' t tested this change because I can't test it :(. Do you think
> that just removing
> __iomem tag is enough?

Give it a shot and see if the warnings go away. I don't think the tag
is correct.

regards,
dan carpenter

2016-11-17 11:46:18

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

On Thu, Nov 17, 2016 at 12:33 PM, Dan Carpenter
<[email protected]> wrote:
> Give it a shot and see if the warnings go away. I don't think the tag
> is correct.

Just removing __iomem tag in shmem_data field of slic_shmemory struct
makes sparse happy. No warnings around.

Should I send a v4 patch with the tag removed?

Cheers,
Sergio Paracuellos

2016-11-18 07:44:55

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] staging: slicoss: fix different address space warnings: 32 bits

On Thu, Nov 17, 2016 at 12:46:12PM +0100, Sergio Paracuellos wrote:
> On Thu, Nov 17, 2016 at 12:33 PM, Dan Carpenter
> <[email protected]> wrote:
> > Give it a shot and see if the warnings go away. I don't think the tag
> > is correct.
>
> Just removing __iomem tag in shmem_data field of slic_shmemory struct
> makes sparse happy. No warnings around.
>
> Should I send a v4 patch with the tag removed?

Yes, please fix up this series and resend.

thanks,

greg k-h