Lior David <liord@xxxxxxxxxxxxxx> writes: > On 10/28/2017 6:01 PM, Kalle Valo wrote: >> Maya Erez <qca_merez@xxxxxxxxxxxxxxxx> writes: >> >>> From: Lior David <qca_liord@xxxxxxxxxxxxxxxx> >>> >>> FW capabilities are currently retrieved only during module >>> initialization, but userspace can replace the firmware while >>> interface is down, so refresh the FW capabilities when >>> interface is up (after FW is loaded) to ensure driver >>> functionality matches the loaded FW. >> >> I think usually the firmware is loaded only once during probe() and I >> think it's quite special that you retrieve it during interface up. Being >> able to change the firmware version runtime like that can lead to >> problems eventually, for example cfg80211 might not allow changing >> already registered configuration etc. >> > Yes you are right, it is not perfect. > We shutdown our chip in interface down so the FW is lost, we have to load it > every interface up. We could request_firmware only once at insmod and store it > so the same FW will be used every time but this is a big waste of memory (FW > size is ~400KB and may be larger with future chips). We only touch one or two > very simple fields in struct wiphy that are not validated in wiphy_register. The > behavior is not 100% proof but good enough, and we recommend to our users to > always rmmod/insmod when replacing FW (replacing FW at runtime is usually not > done in production systems, this is only for debugging). > However, if we do not refresh the capabilities we will have larger problems - > the driver can try to access features not supported by FW and cause FW/host crashes. Yes, I agree that this patch does improve things and should be applied. But I think in the long run you should consider changing this and do the same as other drivers do (load the firmware only on probe()). -- Kalle Valo
