Skip to contentSkip to conversation

staging: slicoss: fix different address space sparse warnings

PATCH In review Peng Sun 2 participants 16 messages Sep 14, 2016 – Sep 16, 2016 Staging

lore
Share & tools
lore

Peng Sun

author
#0 lore raw

cover staging: slicoss: fix different address space sparse warnings

base commit-id d221eb9f14f9

Peng Sun (10):
staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix
sparse warnings
staging: slicoss: slic.h: add a macro IOMEM_GET_FIELD32 to fix sparse
warnings
staging: slicoss: slic.h: add a macro IOMEM_SET_FIELD32 to fix sparse
warnings
staging: slicoss: slic.h: add a macro IOMEM_GET_FIELD64 to fix sparse
warnings
staging: slicoss: slicoss.c: fix different address space sparse
warning
staging: slicoss: slicoss.c: fix different address space sparse
warning
staging: slicoss: slicoss.c: fix different address space sparse
warning
staging: slicoss: slicoss.c: fix different address space sparse
warning
staging: slicoss: slicoss.c: fix different address space sparse
warning
staging: slicoss: slicoss.c: fix different address space sparse
warning

 drivers/staging/slicoss/slic.h    | 46 ++++++++++++++++++++++
 drivers/staging/slicoss/slicoss.c | 82 +++++++++++++++++++++++----------------
 2 files changed, 95 insertions(+), 33 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/10 staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/slicoss/slic.h +7 -0
index fe1d2ce..7c23190 100644
@@ -539,6 +539,13 @@ static inline void slic_flush_write(struct adapter *adapter)
 	ioread32(adapter->regs + SLIC_REG_HOSTID);
 }
 
+#define IOMEM_GET_FIELDADDR(base, member)				\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	(void __iomem *)_base;						\
+})
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4
maintainer
#2 lore raw

Re: [PATCH 01/10] staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

Quoted 4 lines · Peng Sun
---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)

I can't take patches without any changelog text at all, sorry.

Please fix up this series and resend it.

thanks,

greg k-h

maintainer
#3 lore raw

Re: [PATCH 01/10] staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

In reply to Peng Sun
Quoted 25 lines · Peng Sun
---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/slicoss/slic.h +7 -0
index fe1d2ce..7c23190 100644
@@ -539,6 +539,13 @@ static inline void slic_flush_write(struct adapter *adapter)
 	ioread32(adapter->regs + SLIC_REG_HOSTID);
 }
 
+#define IOMEM_GET_FIELDADDR(base, member)				\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	(void __iomem *)_base;						\
+})
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4

Wait, what does this patch on its own do? Why create this macro?

greg k-h

author
#4 lore raw

Re: [PATCH 01/10] staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

these marcos are used to get the value or address of a member of a __iomem structure. GET_FIELDADDR is like &iomem_obj->field, and
GET_FIELD32 is iomem_obj->u32val, GET_FIELD64 is iomem_obj->u64val.

If patch the .c file directly, the source gets into a mess.

Quoted 31 lines · Greg KH
Quoted 25 lines · Peng Sun
---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/slicoss/slic.h +7 -0
index fe1d2ce..7c23190 100644
@@ -539,6 +539,13 @@ static inline void slic_flush_write(struct adapter *adapter)
 	ioread32(adapter->regs + SLIC_REG_HOSTID);
 }
 
+#define IOMEM_GET_FIELDADDR(base, member)				\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	(void __iomem *)_base;						\
+})
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4

Wait, what does this patch on its own do? Why create this macro?

greg k-h

maintainer
#5 lore raw

Re: [PATCH 01/10] staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

Quoted 5 lines · zerons

these marcos are used to get the value or address of a member of a __iomem structure. GET_FIELDADDR is like &iomem_obj->field, and
GET_FIELD32 is iomem_obj->u32val, GET_FIELD64 is iomem_obj->u64val.

If patch the .c file directly, the source gets into a mess.

But add the macro, and then use it, in the same patch, so we can verify that this macro really is doing what you think it is doing, and so that it is used at the same time (no need to add a macro that no one uses...

thanks,

greg k-h

author
#6 lore raw

Re: [PATCH 01/10] staging: slicoss: slic.h: add a macro IOMEM_GET_FIELDADDR to fix sparse warnings

Quoted 19 lines · Greg KH

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

Quoted 5 lines · zerons

these marcos are used to get the value or address of a member of a __iomem structure. GET_FIELDADDR is like &iomem_obj->field, and
GET_FIELD32 is iomem_obj->u32val, GET_FIELD64 is iomem_obj->u64val.

If patch the .c file directly, the source gets into a mess.

Quoted 8 lines

But add the macro, and then use it, in the same patch, so we can verify that this macro really is doing what you think it is doing, and so that it is used at the same time (no need to add a macro that no one uses...

thanks,

greg k-h

Again, sorry about the top-posting. I am still not familiar with these things, thanks for reminding me, I will follow the rules and try my best not to waste your time.

Ok, I will add the macro and use it to fix the warning in the same patch, and resend the patches.

author
#7 lore raw

PATCH 2/10 staging: slicoss: slic.h: add a macro IOMEM_GET_FIELD32 to fix sparse warnings

In reply to Peng Sun
---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/slicoss/slic.h +7 -0
index 7c23190..ff71070 100644
@@ -546,6 +546,13 @@ static inline void slic_flush_write(struct adapter *adapter)
 	(void __iomem *)_base;						\
 })
 
+#define IOMEM_GET_FIELD32(base, member)					\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	ioread32(_base);						\
+})
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4
author
#8 lore raw

PATCH 3/10 staging: slicoss: slic.h: add a macro IOMEM_SET_FIELD32 to fix sparse warnings

In reply to Peng Sun
---
 drivers/staging/slicoss/slic.h | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/staging/slicoss/slic.h +7 -0
index ff71070..4c22863 100644
@@ -553,6 +553,13 @@ static inline void slic_flush_write(struct adapter *adapter)
 	ioread32(_base);						\
 })
 
+#define IOMEM_SET_FIELD32(value, base, member)				\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	iowrite32(value, _base);					\
+})
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4
author
#9 lore raw

PATCH 4/10 staging: slicoss: slic.h: add a macro IOMEM_GET_FIELD64 to fix sparse warnings

In reply to Peng Sun
---
 drivers/staging/slicoss/slic.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
drivers/staging/slicoss/slic.h +25 -0
index 4c22863..b9595c4 100644
@@ -560,6 +560,31 @@ static inline void slic_flush_write(struct adapter *adapter)
 	iowrite32(value, _base);					\
 })
 
+#ifdef CONFIG_64BIT
+#define IOMEM_GET_FIELD64(base, member)					\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	_base += offsetof(typeof(*base), member);			\
+	readq(_base);							\
+})
+#else
+#define IOMEM_GET_FIELD64(base, member)					\
+({									\
+	char __iomem *_base = (char __iomem *)base;			\
+	u64 val;							\
+	_base += offsetof(typeof(*base), member);			\
+	val = ((u64)ioread8(_base + 7)) << 56;				\
+	val += ((u64)ioread8(_base + 6)) << 48;				\
+	val += ((u64)ioread8(_base + 5)) << 40;				\
+	val += ((u64)ioread8(_base + 4)) << 32;				\
+	val += ((u64)ioread8(_base + 3)) << 24;				\
+	val += ((u64)ioread8(_base + 2)) << 16;				\
+	val += ((u64)ioread8(_base + 1)) << 8;				\
+	val += ioread8(_base);						\
+	le64_to_cpu(val);						\
+})
+#endif
+
 #define UPDATE_STATS(largestat, newstat, oldstat)                        \
 {                                                                        \
 	if ((newstat) < (oldstat))                                       \
Signature · 1 line
2.7.4
author
#10 lore raw

PATCH 5/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/slicoss/slicoss.c +2 -2
index 21280a3..929a0d5 100644
@@ -924,8 +924,8 @@ err_unlock_irq:
 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 = IOMEM_GET_FIELD32(sm_data, lnkstatus);
 	uint linkup;
 	unsigned char linkspeed;
 	unsigned char linkduplex;
Signature · 1 line
2.7.4
author
#11 lore raw

PATCH 6/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 49 +++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 17 deletions(-)
drivers/staging/slicoss/slicoss.c +32 -17
index 929a0d5..03b01c5 100644
@@ -1004,8 +1004,9 @@ 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 =
+					IOMEM_GET_FIELDADDR(sm_data, stats);
 		struct slic_stats *old = &adapter->inicstats_prev;
 		struct slicnet_stats *stst = &adapter->slic_stats;
 
@@ -1015,48 +1016,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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(stats, xmit_bytes),
 				old->xmit_bytes);
 
-		UPDATE_STATS_GB(stst->iface.xmt_ucast, stats->xmit_unicasts,
+		UPDATE_STATS_GB(stst->iface.xmt_ucast,
+				IOMEM_GET_FIELD64(stats, xmit_unicasts),
 				old->xmit_unicasts);
 
-		UPDATE_STATS_GB(stst->iface.rcv_bytes, stats->rcv_bytes,
+		UPDATE_STATS_GB(stst->iface.rcv_bytes,
+				IOMEM_GET_FIELD64(stats, rcv_bytes),
 				old->rcv_bytes);
 
-		UPDATE_STATS_GB(stst->iface.rcv_ucast, stats->rcv_unicasts,
+		UPDATE_STATS_GB(stst->iface.rcv_ucast,
+				IOMEM_GET_FIELD64(stats, rcv_unicasts),
 				old->rcv_unicasts);
 
-		UPDATE_STATS_GB(stst->iface.xmt_errors, stats->xmit_collisions,
+		UPDATE_STATS_GB(stst->iface.xmt_errors,
+				IOMEM_GET_FIELD64(stats, xmit_collisions),
 				old->xmit_collisions);
 
 		UPDATE_STATS_GB(stst->iface.xmt_errors,
-				stats->xmit_excess_collisions,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(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,
+				IOMEM_GET_FIELD64(stats, rcv_drops),
 				old->rcv_drops);
 
-		if (stats->rcv_drops > old->rcv_drops)
-			adapter->rcv_drops += (stats->rcv_drops -
+		if (IOMEM_GET_FIELD64(stats, rcv_drops) > old->rcv_drops)
+			adapter->rcv_drops +=
+					(IOMEM_GET_FIELD64(stats, rcv_drops) -
 					       old->rcv_drops);
 		memcpy_fromio(old, stats, sizeof(*stats));
 		break;
Signature · 1 line
2.7.4
author
#12 lore raw

PATCH 7/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/slicoss/slicoss.c +3 -2
index 03b01c5..2097d64 100644
@@ -1697,9 +1697,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,
+		pci_free_consistent(adapter->pcidev, sizeof(*sm_data),
+				    (void __force *)sm_data,
 				    sm->isr_phaddr);
 	}
 
Signature · 1 line
2.7.4
author
#13 lore raw

PATCH 8/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/slicoss/slicoss.c +4 -4
index 2097d64..a2d9c77 100644
@@ -2087,15 +2087,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 (IOMEM_GET_FIELD32(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 = IOMEM_GET_FIELD32(sm_data, isr);
+		IOMEM_SET_FIELD32(0, sm_data, isr);
 		adapter->num_isrs++;
 		switch (adapter->card->state) {
 		case CARD_UP:
Signature · 1 line
2.7.4
author
#14 lore raw

PATCH 9/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/staging/slicoss/slicoss.c +7 -7
index a2d9c77..6ecf71c 100644
@@ -2230,7 +2230,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 */
@@ -2314,7 +2314,7 @@ static int slic_if_init(struct adapter *adapter, unsigned long *flags)
 	/*
 	 *    clear any pending events, then enable interrupts
 	 */
-	sm_data->isr = 0;
+	IOMEM_SET_FIELD32(0, sm_data, isr);
 	slic_write32(adapter, SLIC_REG_ISR, 0);
 	slic_write32(adapter, SLIC_REG_ICR, ICR_INT_ON);
 
@@ -2603,7 +2603,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;
@@ -2666,9 +2666,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 (IOMEM_GET_FIELD32(sm_data, isr)) {
+				if (IOMEM_GET_FIELD32(sm_data, isr) & ISR_UPC) {
+					IOMEM_SET_FIELD32(0, sm_data, isr);
 					slic_write64(adapter, SLIC_REG_ISP, 0,
 						     0);
 					slic_write32(adapter, SLIC_REG_ISR, 0);
@@ -2678,7 +2678,7 @@ static int slic_card_init(struct sliccard *card, struct adapter *adapter)
 					break;
 				}
 
-				sm_data->isr = 0;
+				IOMEM_SET_FIELD32(0, sm_data, isr);
 				slic_write32(adapter, SLIC_REG_ISR, 0);
 				slic_flush_write(adapter);
 			} else {
Signature · 1 line
2.7.4
author
#15 lore raw

PATCH 10/10 staging: slicoss: slicoss.c: fix different address space sparse warning

In reply to Peng Sun
---
 drivers/staging/slicoss/slicoss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/slicoss/slicoss.c +1 -1
index 6ecf71c..e94dbd4 100644
@@ -2883,7 +2883,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);
Signature · 1 line
2.7.4