| Message ID | 1501577287-28904-3-git-send-email-wright.feng@cypress.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 01-08-17 10:48, Wright Feng wrote: > The num_different_channels in wiphy info is not correct when firmware > supports mchan. When mchan is on, num_different_channels is always > overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving > num_different_channels setting forward. Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Wright Feng <wright.feng@cypress.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-)
Wright Feng <wright.feng@cypress.com> writes: > The num_different_channels in wiphy info is not correct when firmware > supports mchan. When mchan is on, num_different_channels is always > overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving > num_different_channels setting forward. > > Signed-off-by: Wright Feng <wright.feng@cypress.com> Does this fix a user visible bug? If yes, it would be nice to document that.
On 8/2/2017 11:32 AM, Kalle Valo wrote: > Wright Feng <wright.feng@cypress.com> writes: > >> The num_different_channels in wiphy info is not correct when firmware >> supports mchan. When mchan is on, num_different_channels is always >> overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving >> num_different_channels setting forward. >> >> Signed-off-by: Wright Feng <wright.feng@cypress.com> > > Does this fix a user visible bug? If yes, it would be nice to document > that. Hi Kalle, Depends on the users expectation ;-) When the device/firmware supports multi-channel (better not use mchan abbreviation) it can have P2P connection and regular connection with AP simultaneous. So the current state is that this is not possible regardless whether mchan is supported. So the device is not used to fullest extent. Regards, Arend
Arend van Spriel <arend.vanspriel@broadcom.com> writes: > On 8/2/2017 11:32 AM, Kalle Valo wrote: >> Wright Feng <wright.feng@cypress.com> writes: >> >>> The num_different_channels in wiphy info is not correct when firmware >>> supports mchan. When mchan is on, num_different_channels is always >>> overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving >>> num_different_channels setting forward. >>> >>> Signed-off-by: Wright Feng <wright.feng@cypress.com> >> >> Does this fix a user visible bug? If yes, it would be nice to document >> that. > > Hi Kalle, > > Depends on the users expectation ;-) When the device/firmware supports > multi-channel (better not use mchan abbreviation) it can have P2P > connection and regular connection with AP simultaneous. So the current > state is that this is not possible regardless whether mchan is > supported. So the device is not used to fullest extent. Thanks, I understand now and I think your description should be also in the commit log so that others will also :) My usual mantra: the commit log should ALWAYS answer the question "_why_ the change is made?" and describe the reason what motivated to implement the patch. This is the most important part. It helps maintainers, backports, distros etc to make decisions if the patch is important for them or not.
On 2017/8/2 下午 09:40, Kalle Valo wrote: > Arend van Spriel <arend.vanspriel@broadcom.com> writes: > >> On 8/2/2017 11:32 AM, Kalle Valo wrote: >>> Wright Feng <wright.feng@cypress.com> writes: >>> >>>> The num_different_channels in wiphy info is not correct when firmware >>>> supports mchan. When mchan is on, num_different_channels is always >>>> overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving >>>> num_different_channels setting forward. >>>> >>>> Signed-off-by: Wright Feng <wright.feng@cypress.com> >>> >>> Does this fix a user visible bug? If yes, it would be nice to document >>> that. >> >> Hi Kalle, >> >> Depends on the users expectation ;-) When the device/firmware supports >> multi-channel (better not use mchan abbreviation) it can have P2P >> connection and regular connection with AP simultaneous. So the current >> state is that this is not possible regardless whether mchan is >> supported. So the device is not used to fullest extent. > > Thanks, I understand now and I think your description should be also in > the commit log so that others will also :) > > My usual mantra: the commit log should ALWAYS answer the question "_why_ > the change is made?" and describe the reason what motivated to implement > the patch. This is the most important part. It helps maintainers, > backports, distros etc to make decisions if the patch is important for > them or not. Hi Kalle and Arend, Thanks for your comment and suggestion, I will put them into commit message and resend v2 later. >
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 54588d2..3dcb139 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -6318,6 +6318,8 @@ static int brcmf_setup_ifmodes(struct wiphy *wiphy, struct brcmf_if *ifp) if (p2p) { if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MCHAN)) combo[c].num_different_channels = 2; + else + combo[c].num_different_channels = 1; wiphy->interface_modes |= BIT(NL80211_IFTYPE_P2P_CLIENT) | BIT(NL80211_IFTYPE_P2P_GO) | BIT(NL80211_IFTYPE_P2P_DEVICE); @@ -6327,10 +6329,10 @@ static int brcmf_setup_ifmodes(struct wiphy *wiphy, struct brcmf_if *ifp) c0_limits[i++].types = BIT(NL80211_IFTYPE_P2P_CLIENT) | BIT(NL80211_IFTYPE_P2P_GO); } else { + combo[c].num_different_channels = 1; c0_limits[i].max = 1; c0_limits[i++].types = BIT(NL80211_IFTYPE_AP); } - combo[c].num_different_channels = 1; combo[c].max_interfaces = i; combo[c].n_limits = i; combo[c].limits = c0_limits;
The num_different_channels in wiphy info is not correct when firmware supports mchan. When mchan is on, num_different_channels is always overridden to 1 in brcmf_setup_ifmodes. Correct the logic by moving num_different_channels setting forward. Signed-off-by: Wright Feng <wright.feng@cypress.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)