On Fri, Jan 17, 2014 at 8:56 PM, ZHAO Gang <gamerh2o@xxxxxxxxx> wrote: > On Fri, Jan 17, 2014 at 5:53 PM, Luca Coelho <luca@xxxxxxxxx> wrote: >> On Fri, 2014-01-17 at 10:37 +0100, Rafał Miłecki wrote: >>> 2014/1/17 ZHAO Gang <gamerh2o@xxxxxxxxx>: >>> > On Fri, Jan 17, 2014 at 5:01 PM, Luca Coelho <luca@xxxxxxxxx> 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 <zajec5@xxxxxxxxx> wrote: >>> >>> > 2014/1/17 Luca Coelho <luca@xxxxxxxxx>: >>> >>> >> 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 <gamerh2o@xxxxxxxxx> >>> >>> >>> --- >>> >>> >>> 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. > Oh yes, at least it's used in ieee80211_rx -> __ieee80211_rx_handle_packet -> ieee80211_scan_rx -> ieee80211_get_channel(local->hw.wiphy, rx_status->freq) Now I think it should be sent to stable. I think I should resend the two patches to update the commit message. > Anyway, I should mention that the version 2 patch should be ignored. > >> -- >> Luca. >> -- 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