On 3 January 2017 at 14:19, Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx> wrote: > On 3-1-2017 12:31, Rafał Miłecki wrote: >>>> + if (!channel) { >>>> + brcmf_err("Firmware reported unexpected channel %d\n", >>>> + ch.control_ch_num); >>>> + continue; >>>> + } >>> As stated above something is really off when this happens so should we >>> continue and try to make sense of what firmware provides or simply fail. >> Well, I could image something like this happening and not being critical. >> The simplest case: Broadcom team releases a new firmware which >> supports extra 5 GHz channels (e.g. due to the IEEE standard change). >> Why should we refuse to run & support all "old" channel just because of that? > > Fair enough. I was assuming we keep __wl_{2,5}ghz_channels up to date > with IEEE standard. > >> What do you mean by "make sense of what firmware provides"? Would kind >> of solution would you suggest? > > When the above assumption can be assured (by us) the only other scenario > would be a change in the firmware API where we wrongly interpret the > information retrieved. In this case all subsequent channels will likely > result in bogus or accidental matches hence it seems better to bail out > early. Good point, this actually gave me an idea for a small nice improvement. I remember I saw something like WL_VER in wl ioctl protocol, it was already bumped at some point by Broadcom, when chanspec format has changed. We should probably read this number from firmware and maybe refuse to run if version is newer than the one we know. -- Rafał