On 2021/12/30 1:38, Mark Kettenis wrote: >> From: Hector Martin <marcan@xxxxxxxxx> >> Date: Tue, 28 Dec 2021 02:23:02 +0900 >> >> On 28/12/2021 01.36, Rob Herring wrote: >>> On Mon, Dec 27, 2021 at 12:35:51AM +0900, Hector Martin wrote: >>>> + brcm,cal-blob: >>>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>>> + description: A per-device calibration blob for the Wi-Fi radio. This >>>> + should be filled in by the bootloader from platform configuration >>>> + data, if necessary, and will be uploaded to the device if present. >>>> + >>>> + apple,module-instance: >>>> + $ref: /schemas/types.yaml#/definitions/string >>>> + description: Module codename used to identify a specific board on >>>> + Apple platforms. This is used to build the firmware filenames, to allow >>>> + different platforms to have different firmware and/or NVRAM config. >>>> + >>>> + apple,antenna-sku: >>>> + $def: /schemas/types.yaml#/definitions/string >>>> + description: Antenna SKU used to identify a specific antenna configuration >>>> + on Apple platforms. This is use to build firmware filenames, to allow >>>> + platforms with different antenna configs to have different firmware and/or >>>> + NVRAM. This would normally be filled in by the bootloader from platform >>>> + configuration data. >>> >>> Is there a known set of strings that can be defined? >> >> For apple,module-instance there is, though it will grow with every new >> machine. If you're happy with me pushing updates to this through >> asahi-soc I can keep it maintained as we add DTs and compatibles there. >> >> I'm curious whether you prefer this approach or something like >> brcm,board-name instead. Right now we do: >> >> apple,module-instance = "honshu" >> >> That gets converted to board_name="apple,honshu" in the code, which is >> what the firmwares are named after (plus extra info later appended, if >> the rest of the Apple data is available). >> >> But we could also do: >> >> brcm,board-name = "apple,honshu" >> >> The latter would be more generically useful for other platforms, since >> it would allow e.g. having DTs for different boards that use the same >> WiFi module/subsystem and thus a compatible NVRAM fw file alias to the >> same file name (right now this is done with symlinks in /lib/firmware, >> one for each equivalent board). For non-Apple platforms (i.e. if >> antenna-sku and/or the OTP aren't available to do the funky Apple >> firmware selection), this just ends up replacing what would normally be >> the OF root node compatible in the firmware filename. >> >> E.g. right now we have: >> >> brcmfmac43430-sdio.AP6212.txt >> brcmfmac43430-sdio.raspberrypi,3-model-b.txt >> brcmfmac43430-sdio.raspberrypi,model-zero-w.txt -> brcmfmac43430-sdio.raspberrypi,3-model-b.txt >> brcmfmac43430-sdio.sinovoip,bpi-m2-plus.txt -> brcmfmac43430-sdio.AP6212.txt >> brcmfmac43430-sdio.sinovoip,bpi-m2-ultra.txt -> brcmfmac43430-sdio.AP6212.txt >> brcmfmac43430-sdio.sinovoip,bpi-m2-zero.txt -> brcmfmac43430-sdio.AP6212.txt >> brcmfmac43430-sdio.sinovoip,bpi-m3.txt -> brcmfmac43430-sdio.AP6212.txt >> >> And this could allow the sinovoip.* DTs to say: >> brcm,board-name = "AP6212"; >> >> And the rPi zero one: >> brcm,board-name = "raspberrypi,3-model-b"; >> >> And avoid the symlinks. >> >> The antenna-sku thing is specific to the Apple firmware selection >> process and doesn't make sense as a more generic property. >> >> antenna-sku right now always seems to be one of "ID", "X0", "X2", "X3", >> though that could presumably change in the future. I can add this to the >> binding if you want, though since this will be filled in by the >> bootloader from platform data we wouldn't be validating it anyway. Not >> sure if it's worth it. > > Actually what Apple does here makes quite a bit of sense. Typically > WiFi chips are integrated with some analog components into a shielded > module. The AP6212 mentioned above is an example of such a module. I > suspect that the module defines some of the characteristics encoded in > the "nvmram" files, but certainly not all because the connected > antenna will also affect how the thing behaves. Of course many SBCs > come without an antenna so the actual antenna depends on whatever the > user connects to the board. So using a module-specific "nvram" file > is probably the best one can do here. So I think if you want to have > a generic module name property, it should be called "brcm,module-name" > instead of "brcm,board-name". However... > >>> There's also the somewhat standard 'firmware-name' property that >>> serves similar purpose, but if there's multiple files, then I guess >>> this approach is fine. >> >> Yeah, and the firmware name is constructed using non-DT information too >> (and we have several attempted filenames times several firmware types), >> so it wouldn't be complete. > > ...if the way the firmware name is constructed remains Apple-specific > because of this non-DT information, keeping the "apple,xxx" properties > has the benefit of signalling that firmware names constructed this way > are desired. Or rather, their absence can signal that the > Apple-specific code in the driver should be skipped. > The current logic is that if all the information is available (OTP, antenna-sku, board-name but that always gets filled in by default from DT/DMI global info) it will use the Apple mechanism, otherwise it will use the standard one. So in this case we could still use brcm,board-name/module-name for Apple devices, and we'd keep apple,antenna-sku as one of the two triggers to do Apple firmware selection. Other devices may use nothing (use default DMI or DT device name/compatible as module-name) or specify an override, and they will still get firmwares with that name in them and a fallback to generic firmware, which is the current behavior. -- Hector Martin (marcan@xxxxxxxxx) Public Key: https://mrcn.st/pub