Search Linux Wireless

Re: [PATCH v2 24/26] staging: brcm80211: declared global vars in softmac phy as const

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/27/2011 08:35 PM, Larry Finger wrote:
On 09/27/2011 12:45 PM, Franky Lin wrote:
From: Roland Vossen<rvossen@xxxxxxxxxxxx>

Global variables are undesirable unless they are read only.

Reported-by: Johannes Berg<johannes@xxxxxxxxxxxxxxxx>
Reviewed-by: Pieter-Paul Giesberts<pieterpg@xxxxxxxxxxxx>
Reviewed-by: Arend van Spriel<arend@xxxxxxxxxxxx>
Signed-off-by: Franky Lin<frankyl@xxxxxxxxxxxx>
---
   drivers/staging/brcm80211/brcmsmac/phy/phy_cmn.c   |   16 +-----
   drivers/staging/brcm80211/brcmsmac/phy/phy_int.h   |   23 --------
   drivers/staging/brcm80211/brcmsmac/phy/phy_lcn.c   |    6 +-
   drivers/staging/brcm80211/brcmsmac/phy/phy_n.c     |   60 ++++++++++----------
   .../staging/brcm80211/brcmsmac/phy/phytbl_lcn.c    |    2 +-
   5 files changed, 36 insertions(+), 71 deletions(-)

diff --git a/drivers/staging/brcm80211/brcmsmac/phy/phy_cmn.c b/drivers/staging/brcm80211/brcmsmac/phy/phy_cmn.c
index f9702c0..5b81480 100644
--- a/drivers/staging/brcm80211/brcmsmac/phy/phy_cmn.c
+++ b/drivers/staging/brcm80211/brcmsmac/phy/phy_cmn.c
@@ -52,7 +52,7 @@ struct chan_info_basic {
   	u16 freq;
   };

-static struct chan_info_basic chan_info_all[] = {
+static const struct chan_info_basic chan_info_all[] = {
   	{1, 2412},
   	{2, 2417},
   	{3, 2422},
What are you doing?  You make this change here in patch 24, then undo it in #25,
and redo it in #26! Are you guys actually thinking about what you are doing?

We reordered the patches a little to move the ones we got feedback about at the end for rework. My bet is that you see rebase at work although I agree this is pretty tricky. I am glad the net result is that the const qualifier is added. There was some thinking involved in what we did today ;-)

This needs to be fixed. In addition, the next time you submit this patch bomb,
change from V2 to V3. That is the standard way to handle resubmissions that are
addressing comments, not by increasing some extra counter in the 00/XX cover
patch. The VX notation helps the maintainer keep track of the patches as he
knows to drop all the V2 ones when V3 arrives. Maintainers are precious and must
be treated very carefully.

And here we were thinking we would have learned all tricks about submitting patches in our staging year ;-)

So to be sure I have it right: Only the cover letter should say: [PATCH v2 00/20] with the same headline as the orignal. Or should each patch say [PATCH v2 xx/20] even when there are 26 patches?

Larry

It would not have hurt if the cover letter of this series would indicate it replaces the previous series by refering to its message-id. Would that be an alternative approach or just a good addition.

Gr. AvS

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux