On 16-12-2016 11:13, Johannes Berg wrote: > On Wed, 2016-12-14 at 10:01 +0100, Arend Van Spriel wrote: > >> Had to look for "> 16" ;-) > > Sorry. > >> Here an instance of the tab vs. space issue you mentioned. Will go >> over the patch and fix that. > > There were a few, not really interesting though - git would probably > flag it anyway, or checkpatch :) > >>> + if (num_chans > 16) >>> + return -EINVAL; >> >> I suspect this is the restriction you were referring to. > > Yes. > >> There is no >> reason for this although the android wifi hal has max 16 channels in >> a bucket so I might have picked that up. > > I thought I saw something with a u16 bitmap that seemed related, but I > don't see that now so I'm probably just confused. > >> So could a driver have a similar limit and should we add such to the >> gscan capabilities? For instance our firmware api has a nasty >> restriction of 64 channels for all buckets together, eg. can do 4 >> buckets of 16 channels each. > > We do have a limit of the maximum scan buckets, which seems to be 16 > right now. We also have a limit on the number of channels per bucket, > which is also 16, but no combined limit afaict (so 16x16 seems fine). > > Maybe we do need some advertisement in that area then? Right now, > wifihal seems to be able to read as capabilities the number of buckets > (wifi_gscan_capabilities), but assumes the number of channels: > > const unsigned MAX_CHANNELS = 16; > const unsigned MAX_BUCKETS = 16; > > I guess we took that and combined it, and you had more negotiation with > Google ;-) I was not so much involved with the initial gscan effort, but I guess for brcm it might be true. > We may then have to actually advertise the limit you have ("64 channels > combined over all buckets"), unless you can get away with just > advertising 4 buckets (and us saying 16 channels per bucket is enough?) > > I'm a bit tempted to make this more forward compatible though and not > hard-limit the number of channels per bucket in the code. Indeed so I will remove it. Regards, Arend