Hi Abhishek, On Tue, Jan 12, 2021 at 10:03 AM Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx> wrote: > > Hi Luiz, > > On Mon, Jan 11, 2021 at 1:55 PM Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx> wrote: > > > > Hi Abhishek, > > > > On Mon, Jan 11, 2021 at 11:38 AM Abhishek Pandit-Subedi > > <abhishekpandit@xxxxxxxxxxxx> wrote: > > > > > > Hi Marcel, > > > > > > I don't think this solves the original problem we were talking about: > > > the driver should replace the runtime firmware on reload if it doesn't > > > match what's on disk. > > > > > > Some background for the mailing list: > > > - On a ChromeOS laptop, we discovered that the Bluetooth controller > > > wasn't being fully powered down in some reboots. As a result, a new > > > firmware wasn't being applied after an update. > > > - The kernel driver was checking if the bluetooth controller had > > > loaded some firmware already. If it was in bootloader mode, it would > > > download new firmware. If it was not, it would skip downloading new > > > firmware. > > > > > > The useful part of this mgmt command is to force the driver to reset > > > to bootloader (Action = 0 in Set Runtime Firmware). However, without > > > being able to compare the firmware version loaded on the controller, > > > there's no clear signal for when this should be called. Loading the > > > firmware through mgmt may be useful for debugging but you could also > > > just replace the firmware on disk and "reset to bootloader" to achieve > > > the same effect. I would actually expect unloading and reloading the > > > module should do that. > > > > > > Also, moving the firmware loading from the driver to the userspace > > > seems odd to me. Since the comparison is between the controller > > > firmware and disk firmware, there's not much extra that the userspace > > > knows that the kernel does not. > > > > My last suggestion was just to have a MGMT command suggesting the > > kernel to load the firmware from a different location, this could be > > useful for testing purpose so one can set for example an old/beta > > firmware to compare for regressions or test new features that > > otherwise would not be available. That said perhaps we don't actually > > need a new MGMT command for doing this and just by replacing the > > current file would trigger a reload but that may get tricky when if > > the location does get unmonted/remounted etc. > > > > > > > > ---- > > > > > > Coming back to the original problem of when to reload runtime > > > firmware, here are the conditions under which we do and don't want a > > > reload. > > > > > > Do want a reload: > > > - Reboot > > > - Module is unloaded and reloaded > > > > > > Don't want a reload: > > > - Transport disconnection (i.e. usb disconnect; some laptops will > > > power down USB during suspend to save additional power but BT will > > > stay powered up) > > > > Well if the device disappears from the host I'm not really sure how > > you will be able to detect that the firmware was retained, that said > > when the adapter is power up again it should be possible to query it > > what firmware it is currently running and then compare with the one > > from file before attempting to load it, this should also work > > regardless of the underlying transport/bus so it would work regardless > > of the driver in use. > > Generally the device drivers detect whether it's in bootloader mode or > runtime firmware is uploaded. With the runtime firmware, it can also > query what version is running. However, there's currently no way to > read the version on disk because the firmware version isn't stored at > some known location in the firmware blob. That's the crux of the > problem here. Hmm I believe we can extract the build number of the firmware, I mean it is obviously in there and we could possibly start encoding its version/build as part of the filename as well not just the model. https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/drivers/bluetooth/btintel.c#n226 https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/drivers/bluetooth/btusb.c#n2723 So we just have to figure out where the build number is in the firmware file, so store it somewhere else perhaps using some metadata perhaps.
