* [PATCH 1/2] iwmc3200wifi: Fix sparse warnings
@ 2010-04-16 1:28 Zhu Yi
2010-04-16 1:28 ` [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations Zhu Yi
0 siblings, 1 reply; 6+ messages in thread
From: Zhu Yi @ 2010-04-16 1:28 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Samuel Ortiz, Zhu Yi
From: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
drivers/net/wireless/iwmc3200wifi/rx.c | 3 ++-
drivers/net/wireless/iwmc3200wifi/trace.h | 4 ++--
drivers/net/wireless/iwmc3200wifi/tx.c | 4 ++--
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/rx.c b/drivers/net/wireless/iwmc3200wifi/rx.c
index ad53987..e1184de 100644
--- a/drivers/net/wireless/iwmc3200wifi/rx.c
+++ b/drivers/net/wireless/iwmc3200wifi/rx.c
@@ -431,7 +431,8 @@ static int iwm_ntf_rx_ticket(struct iwm_priv *iwm, u8 *buf,
return PTR_ERR(ticket_node);
IWM_DBG_RX(iwm, DBG, "TICKET %s(%d)\n",
- ticket->action == IWM_RX_TICKET_RELEASE ?
+ __le16_to_cpu(ticket->action) ==
+ IWM_RX_TICKET_RELEASE ?
"RELEASE" : "DROP",
ticket->id);
spin_lock(&iwm->ticket_lock);
diff --git a/drivers/net/wireless/iwmc3200wifi/trace.h b/drivers/net/wireless/iwmc3200wifi/trace.h
index 320e54f..abb4805 100644
--- a/drivers/net/wireless/iwmc3200wifi/trace.h
+++ b/drivers/net/wireless/iwmc3200wifi/trace.h
@@ -76,7 +76,7 @@ TRACE_EVENT(iwm_tx_wifi_cmd,
IWM_ASSIGN;
__entry->opcode = hdr->sw_hdr.cmd.cmd;
__entry->lmac = 0;
- __entry->seq = hdr->sw_hdr.cmd.seq_num;
+ __entry->seq = __le16_to_cpu(hdr->sw_hdr.cmd.seq_num);
__entry->resp = GET_VAL8(hdr->sw_hdr.cmd.flags, UMAC_DEV_CMD_FLAGS_RESP_REQ);
__entry->color = GET_VAL32(hdr->sw_hdr.meta_data, UMAC_FW_CMD_TX_STA_COLOR);
__entry->eot = GET_VAL32(hdr->hw_hdr.cmd, UMAC_HDI_OUT_CMD_EOT);
@@ -123,7 +123,7 @@ TRACE_EVENT(iwm_tx_packets,
__entry->ra_tid = GET_VAL32(hdr->hw_hdr.meta_data, UMAC_HDI_OUT_RATID);
__entry->credit_group = GET_VAL32(hdr->hw_hdr.meta_data, UMAC_HDI_OUT_CREDIT_GRP);
__entry->color = GET_VAL32(hdr->sw_hdr.meta_data, UMAC_FW_CMD_TX_STA_COLOR);
- __entry->seq = hdr->sw_hdr.cmd.seq_num;
+ __entry->seq = __le16_to_cpu(hdr->sw_hdr.cmd.seq_num);
__entry->npkt = 1;
__entry->bytes = len;
diff --git a/drivers/net/wireless/iwmc3200wifi/tx.c b/drivers/net/wireless/iwmc3200wifi/tx.c
index 9537cdb..3216621 100644
--- a/drivers/net/wireless/iwmc3200wifi/tx.c
+++ b/drivers/net/wireless/iwmc3200wifi/tx.c
@@ -302,8 +302,8 @@ void iwm_tx_credit_init_pools(struct iwm_priv *iwm,
#define IWM_UDMA_HDR_LEN sizeof(struct iwm_umac_wifi_out_hdr)
-static int iwm_tx_build_packet(struct iwm_priv *iwm, struct sk_buff *skb,
- int pool_id, u8 *buf)
+static __le16 iwm_tx_build_packet(struct iwm_priv *iwm, struct sk_buff *skb,
+ int pool_id, u8 *buf)
{
struct iwm_umac_wifi_out_hdr *hdr = (struct iwm_umac_wifi_out_hdr *)buf;
struct iwm_udma_wifi_cmd udma_cmd;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations
2010-04-16 1:28 [PATCH 1/2] iwmc3200wifi: Fix sparse warnings Zhu Yi
@ 2010-04-16 1:28 ` Zhu Yi
2010-04-16 20:55 ` Pavel Roskin
0 siblings, 1 reply; 6+ messages in thread
From: Zhu Yi @ 2010-04-16 1:28 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Samuel Ortiz, Zhu Yi
From: Samuel Ortiz <sameo@linux.intel.com>
Add -D__CHECK_ENDIAN__ to driver ccflags so that sparse will
always check endianness by default.
Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
drivers/net/wireless/iwmc3200wifi/Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/Makefile b/drivers/net/wireless/iwmc3200wifi/Makefile
index aeed5cd..cdc7e07 100644
--- a/drivers/net/wireless/iwmc3200wifi/Makefile
+++ b/drivers/net/wireless/iwmc3200wifi/Makefile
@@ -6,3 +6,5 @@ iwmc3200wifi-$(CONFIG_IWM_DEBUG) += debugfs.o
iwmc3200wifi-$(CONFIG_IWM_TRACING) += trace.o
CFLAGS_trace.o := -I$(src)
+
+ccflags-y += -D__CHECK_ENDIAN__
--
1.6.3.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations
2010-04-16 1:28 ` [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations Zhu Yi
@ 2010-04-16 20:55 ` Pavel Roskin
2010-04-16 20:57 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Roskin @ 2010-04-16 20:55 UTC (permalink / raw)
To: Zhu Yi; +Cc: linville, linux-wireless, Samuel Ortiz
On Fri, 2010-04-16 at 09:28 +0800, Zhu Yi wrote:
> From: Samuel Ortiz <sameo@linux.intel.com>
>
> Add -D__CHECK_ENDIAN__ to driver ccflags so that sparse will
> always check endianness by default.
I believe it's wrong. I would be fine with enabling __CHECK_ENDIAN__
for the whole kernel by default (perhaps with an option to disable it).
But there is no reason why some particular drivers need that check more
than the rest of the kernel.
--
Regards,
Pavel Roskin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations
2010-04-16 20:55 ` Pavel Roskin
@ 2010-04-16 20:57 ` Johannes Berg
2010-04-16 21:34 ` Pavel Roskin
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2010-04-16 20:57 UTC (permalink / raw)
To: Pavel Roskin; +Cc: Zhu Yi, linville, linux-wireless, Samuel Ortiz
On Fri, 2010-04-16 at 16:55 -0400, Pavel Roskin wrote:
> On Fri, 2010-04-16 at 09:28 +0800, Zhu Yi wrote:
> > From: Samuel Ortiz <sameo@linux.intel.com>
> >
> > Add -D__CHECK_ENDIAN__ to driver ccflags so that sparse will
> > always check endianness by default.
>
> I believe it's wrong. I would be fine with enabling __CHECK_ENDIAN__
> for the whole kernel by default (perhaps with an option to disable it).
> But there is no reason why some particular drivers need that check more
> than the rest of the kernel.
We've done it on other drivers -- we can't do it for all of the kernel
because that would drown people in warnings, but for those drivers that
_should_ be clean it ought to be fine to add it by default.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations
2010-04-16 20:57 ` Johannes Berg
@ 2010-04-16 21:34 ` Pavel Roskin
2010-04-17 8:20 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Roskin @ 2010-04-16 21:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: Zhu Yi, linville, linux-wireless, Samuel Ortiz
On Fri, 2010-04-16 at 22:57 +0200, Johannes Berg wrote:
> We've done it on other drivers -- we can't do it for all of the kernel
> because that would drown people in warnings, but for those drivers that
> _should_ be clean it ought to be fine to add it by default.
Oh, I see, there are several precedents.
That said, there are not many warnings added by __CHECK_ENDIAN__. In my
x86_64 configuration, sparse produces 2316 lines of output without
__CHECK_ENDIAN__ and 3526 lines with __CHECK_ENDIAN__, a 52% increase.
That's hardly "drowning".
Also, the warnings about endianess are perhaps the most useful of all
sparse warnings.
--
Regards,
Pavel Roskin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations
2010-04-16 21:34 ` Pavel Roskin
@ 2010-04-17 8:20 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2010-04-17 8:20 UTC (permalink / raw)
To: Pavel Roskin; +Cc: Zhu Yi, linville, linux-wireless, Samuel Ortiz
On Fri, 2010-04-16 at 17:34 -0400, Pavel Roskin wrote:
> On Fri, 2010-04-16 at 22:57 +0200, Johannes Berg wrote:
>
> > We've done it on other drivers -- we can't do it for all of the kernel
> > because that would drown people in warnings, but for those drivers that
> > _should_ be clean it ought to be fine to add it by default.
>
> Oh, I see, there are several precedents.
>
> That said, there are not many warnings added by __CHECK_ENDIAN__. In my
> x86_64 configuration, sparse produces 2316 lines of output without
> __CHECK_ENDIAN__ and 3526 lines with __CHECK_ENDIAN__, a 52% increase.
> That's hardly "drowning".
Other people feel differently by that, evidenced by the "lack of
enthusiasm" for adding endian checking by default.
> Also, the warnings about endianess are perhaps the most useful of all
> sparse warnings.
I agree, so I want them whenever I run sparse on code I own.
However, I'm simply not willing to fight for making endian checking
default kernel-wide to achieve that goal. You may disagree, and you're
welcome to pick that fight. Until then, please let us do what gets our
job done.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-17 8:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-16 1:28 [PATCH 1/2] iwmc3200wifi: Fix sparse warnings Zhu Yi
2010-04-16 1:28 ` [PATCH 2/2] iwmc3200wifi: check sparse endianness annotations Zhu Yi
2010-04-16 20:55 ` Pavel Roskin
2010-04-16 20:57 ` Johannes Berg
2010-04-16 21:34 ` Pavel Roskin
2010-04-17 8:20 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).