Re: [PATCHv2 3/4] brcm80211: make mgmt_tx in brcmfmac accept a NULL channel

[Antonio Quartulli <antonio@open-mesh.com>]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

On Wed, Jun 05, 2013 at 07:14:07AM -0700, Arend van Spriel wrote:
> On 06/05/2013 03:57 PM, Antonio Quartulli wrote:
> > On Wed, Jun 05, 2013 at 06:53:32AM -0700, Arend van Spriel wrote:
> >
> > [...]
> >
> >>> +		freq = cfg->channel;
> >>> +		if (chan)
> >>> +			freq = chan->center_freq;
> >>> +		chan_nr = ieee80211_frequency_to_channel(freq);
> >>
> >> Could you get rid of 'freq' variable and use
> >> ieee80211_frequency_to_channel() on cfg->channel or chan->center_freq.
> >>
> >
> > I tried that, but the line indented below the if would be longer than 80 chars
> > and breaking it is quite ugly.
> >
> >> Another thing is that cfg->channel can be zero resulting in chan_nr
> >> being zero. I had a quick look whether the device firmware can handle
> >> this. I suspect not, but I will need to ask to be sure.
> >>
> >
> > ok. But when cfg->channel is zero, where is the current freq stored?
> 
> It is not. The rf on the device is set to a certain channel so it can be 
> retrieved from the device:
> 
> brcmf_fil_cmd_int_get(vif->ifp, BRCMF_C_GET_CHANNEL, &freq);
> 
> The vif pointer can be obtained using container_of() on the wdev. That 
> is done in mgmt_tx() already some lines above. May you should move 
> determining the vif pointer and do it unconditional.
> 

Yeah, I like this approach.

I'll send v3 of this patchset with this change in it.
@johannes: I'll also revert the order of the patches.

Thanks a lot Arend.

Regards,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]