Return-path: Received: from mail-pd0-f172.google.com ([209.85.192.172]:37212 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752172AbaAQM4p convert rfc822-to-8bit (ORCPT ); Fri, 17 Jan 2014 07:56:45 -0500 Received: by mail-pd0-f172.google.com with SMTP id z10so3968824pdj.31 for ; Fri, 17 Jan 2014 04:56:45 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1389952418.8062.57.camel@porter.coelho.fi> References: <1389939896.8062.53.camel@porter.coelho.fi> <1389949304.8062.55.camel@porter.coelho.fi> <1389952418.8062.57.camel@porter.coelho.fi> Date: Fri, 17 Jan 2014 20:56:45 +0800 Message-ID: (sfid-20140117_135652_326672_ADA7F0A6) Subject: Re: [PATCH 1/2] b43: fix the wrong assignment of status.freq in b43_rx() From: ZHAO Gang To: Luca Coelho Cc: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= , Jonas Gorski , "linux-wireless@vger.kernel.org" , b43-dev , Stefano Brivio Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Fri, Jan 17, 2014 at 5:53 PM, Luca Coelho wrote: > On Fri, 2014-01-17 at 10:37 +0100, Rafał Miłecki wrote: >> 2014/1/17 ZHAO Gang : >> > On Fri, Jan 17, 2014 at 5:01 PM, Luca Coelho wrote: >> >> On Fri, 2014-01-17 at 09:56 +0100, Jonas Gorski wrote: >> >>> On Fri, Jan 17, 2014 at 8:11 AM, Rafał Miłecki wrote: >> >>> > 2014/1/17 Luca Coelho : >> >>> >> On Fri, 2014-01-17 at 13:27 +0800, ZHAO Gang wrote: >> >>> >>> In following patch, replace b43 specific helper function with kernel >> >>> >>> api to reduce code duplication. >> >>> >>> >> >>> >>> Signed-off-by: ZHAO Gang >> >>> >>> --- >> >>> >>> drivers/net/wireless/b43/xmit.c | 4 ++-- >> >>> >>> 1 file changed, 2 insertions(+), 2 deletions(-) >> >>> >>> >> >>> >>> diff --git a/drivers/net/wireless/b43/xmit.c b/drivers/net/wireless/b43/xmit.c >> >>> >>> index 4ae63f4..50e5ddb 100644 >> >>> >>> --- a/drivers/net/wireless/b43/xmit.c >> >>> >>> +++ b/drivers/net/wireless/b43/xmit.c >> >>> >>> @@ -821,10 +821,10 @@ void b43_rx(struct b43_wldev *dev, struct sk_buff *skb, const void *_rxhdr) >> >>> >>> * channel number in b43. */ >> >>> >>> if (chanstat & B43_RX_CHAN_5GHZ) { >> >>> >>> status.band = IEEE80211_BAND_5GHZ; >> >>> >>> - status.freq = b43_freq_to_channel_5ghz(chanid); >> >>> >>> + status.freq = b43_channel_to_freq_5ghz(chanid); >> >>> >>> } else { >> >>> >>> status.band = IEEE80211_BAND_2GHZ; >> >>> >>> - status.freq = b43_freq_to_channel_2ghz(chanid); >> >>> >>> + status.freq = b43_channel_to_freq_2ghz(chanid); >> >>> >>> } >> >>> >>> break; >> >>> >>> default: >> >>> >> >> >>> >> Why do you need this patch if you're going to remove these calls in the >> >>> >> next patch anyway? >> >>> > >> >>> > I was thinking about this for a moment too. You could just make a one >> >>> > patch and note in commit message that "translation" was reversed. >> >>> >> >>> That would mean mixing fixes and improvements, which is something you >> >>> are not supposed to do, so IMHO having these split into two is >> >>> correct. Think about stable maintainers wanting the fix but not the >> >>> other change because it might introduce unknown side effects. >> >> >> >> Makes sense. In such case, the first patch should be clearly marked as >> >> a bug fix, so at least the commit message should be changed (ie. >> >> mentioning the next patch in the series is useless). >> >> >> > >> > I am OK to send this fix either in one patch or two, actually I have >> > sent a version 2 which is a one patch version :-) >> > >> > I'm not sure if this patch is needed for stable, yes, as you said, if >> > it's for stable, the commit message should be changed. >> >> Thanks for your help guys. >> >> I think it may be the best idea to send >> 1/2 as fix (probably 3.14) + stable CC >> 2/2 as improvement (for next) >> Does it make sense? > > Sounds good to me. The actual fix is so simple and obvious that I don't > see any reason for not sending it as a fix + stable. > Hi, after reading the code, it seems that status.freq is not actually used in rx path, so this fix has no user sensible changes. So I think it is not needed to send this patch to stable. Anyway, I should mention that the version 2 patch should be ignored. > -- > Luca. >