| Message ID | 20211226153624.162281-1-marcan@marcan.st |
|---|---|
| Headers | show |
| Series | brcmfmac: Support Apple T2 and M1 platforms | expand |
On Mon, Dec 27, 2021 at 12:35:50AM +0900, Hector Martin wrote: > # On firmware > > As you might expect, the firmware for these machines is not available > under a redistributable license; however, every owner of one of these > machines *is* implicitly licensed to posess the firmware, and the OS > packages containing it are available under well-known URLs on Apple's > CDN with no authentication. Apple's EFI firmware contains a full-fledged network stack for downloading macOS images from osrecovery.apple.com. I suspect that it also contains wifi firmware. You may want to check if it's passed to the OS as an EFI property. Using that would sidestep license issues. There's EDID data, Thunderbolt DROM data and whatnot in those properties, so I wouldn't be surprised if it contained wifi stuff as well. Enable CONFIG_APPLE_PROPERTIES and pass "dump_apple_properties" on the command line to see all EFI properties in dmesg. Alternatively, check "ioreg -l" on macOS. Generally, what's available in the I/O registry should also be available on Linux either as an ACPI or EFI property. Thanks, Lukas
On Sun, Dec 26, 2021 at 4:36 PM Hector Martin <marcan@marcan.st> wrote: > This binding is currently used for SDIO devices, but these chips are > also used as PCIe devices on DT platforms and may be represented in the > DT. Re-use the existing binding and add chip compatibles used by Apple > T2 and M1 platforms (the T2 ones are not known to be used in DT > platforms, but we might as well document them). > > Then, add properties required for firmware selection and calibration on > M1 machines. > > Signed-off-by: Hector Martin <marcan@marcan.st> Makes sense to me! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > + 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. This is especially nice. This way on other systems U-Boot can read the calibration file (usually stored in a special partition) and modify the device tree to include this, then we don't need the driver to learn about any specific file locations for calibrations or worry about inserting it from userspace. Yours, Linus Walleij
Hi, On 12/26/21 20:17, Lukas Wunner wrote: > On Mon, Dec 27, 2021 at 12:35:50AM +0900, Hector Martin wrote: >> # On firmware >> >> As you might expect, the firmware for these machines is not available >> under a redistributable license; however, every owner of one of these >> machines *is* implicitly licensed to posess the firmware, and the OS >> packages containing it are available under well-known URLs on Apple's >> CDN with no authentication. > > Apple's EFI firmware contains a full-fledged network stack for > downloading macOS images from osrecovery.apple.com. I suspect > that it also contains wifi firmware. > > You may want to check if it's passed to the OS as an EFI property. > Using that would sidestep license issues. There's EDID data, > Thunderbolt DROM data and whatnot in those properties, so I > wouldn't be surprised if it contained wifi stuff as well. > > Enable CONFIG_APPLE_PROPERTIES and pass "dump_apple_properties" > on the command line to see all EFI properties in dmesg. > Alternatively, check "ioreg -l" on macOS. Generally, what's > available in the I/O registry should also be available on Linux > either as an ACPI or EFI property. Interesting, note that even if the files are not available as a property we also have CONFIG_EFI_EMBEDDED_FIRMWARE, see: drivers/firmware/efi/embedded-firmware.c Documentation/driver-api/firmware/fallback-mechanisms.rst I wrote this to pry/dig out some touchscreen firmwares (where we have been unable to get permission to redistribute) out of EFI boot_services_code mem regions on tablets where the touchsceen is supported under the EFI environment. This may need some tweaks, but if there is an embedded copy of the firmware files in the EFI mem regions somewhere it should be possible to adjust this code to grab it and present it to the firmware-loader mechanism as a fallback option. Refards, Hans
On 2021/12/27 6:42, Hans de Goede wrote: > Hi, > > On 12/26/21 20:17, Lukas Wunner wrote: >> On Mon, Dec 27, 2021 at 12:35:50AM +0900, Hector Martin wrote: >>> # On firmware >>> >>> As you might expect, the firmware for these machines is not available >>> under a redistributable license; however, every owner of one of these >>> machines *is* implicitly licensed to posess the firmware, and the OS >>> packages containing it are available under well-known URLs on Apple's >>> CDN with no authentication. >> >> Apple's EFI firmware contains a full-fledged network stack for >> downloading macOS images from osrecovery.apple.com. I suspect >> that it also contains wifi firmware. >> >> You may want to check if it's passed to the OS as an EFI property. >> Using that would sidestep license issues. There's EDID data, >> Thunderbolt DROM data and whatnot in those properties, so I >> wouldn't be surprised if it contained wifi stuff as well. >> >> Enable CONFIG_APPLE_PROPERTIES and pass "dump_apple_properties" >> on the command line to see all EFI properties in dmesg. >> Alternatively, check "ioreg -l" on macOS. Generally, what's >> available in the I/O registry should also be available on Linux >> either as an ACPI or EFI property. > > Interesting, note that even if the files are not available as > a property we also have CONFIG_EFI_EMBEDDED_FIRMWARE, see: > > drivers/firmware/efi/embedded-firmware.c > Documentation/driver-api/firmware/fallback-mechanisms.rst > > I wrote this to pry/dig out some touchscreen firmwares (where > we have been unable to get permission to redistribute) out of > EFI boot_services_code mem regions on tablets where > the touchsceen is supported under the EFI environment. > > This may need some tweaks, but if there is an embedded copy > of the firmware files in the EFI mem regions somewhere it > should be possible to adjust this code to grab it and present > it to the firmware-loader mechanism as a fallback option. Note that this wouldn't work on M1 Macs anyway, since those don't have EFI (we provide EFI via U-Boot as a chained bootloader on those), and their bootloader doesn't support any networking (it doesn't even do USB or any kind of UI). Quick recap for those not familiar with the M1 boot process: the bootloader is iBoot, which is extremely simple (at least compared to EFI). All it can do is boot kernels from APFS volumes on internal NVMe. The boot selection menu and recovery options are implemented as macOS apps running from a recovery image (~1GB), and "USB boot" is implemented by copying the macOS equivalent of /boot to NVMe. There is a global recovery image as well as per-OS recovery image. The WiFi firmware is present in this image as well as on normal macOS root volumes. Our Linux install script is actually mostly a macOS install script that sets up all the boot components that macOS would normally have, including the recovery image, minus the main root filesystem. This is all required to work properly within Apple's security and multi-boot framework. So, since we're installing the recovery image, we're already in an easy position to pull the firmware out and stick it in the EFI partition for Linux to easily use. The alternative would be for Linux userspace to read it from APFS directly, but that seems unlikely to be practical until linux-apfs is upstreamed. For T2 Macs I'm sure the firmware will be in EFI somewhere, but even if we can get it from there (I wouldn't be surprised if it's e.g. still compressed in the normal boot path that doesn't start network services), I'm not sure it's worth implementing yet another mechanism for those machines. Once we have the vendor-firmware mechanism implemented for M1, it's easy to just run the same script on T2s and get the proper firmware from macOS (which might even be different from the EFI firmware...). macOS definitely doesn't read the firmware from EFI on those machines, so a hack to do it by scanning the code would probably not be something we can rely on to continue working across firmware updates (and they do update WiFi firmware; it's a rather well known source of security issues... so then we'd have to play the update-the-sha256 cat and mouse game). I'm pretty sure there's no property containing the big firmware blob passed explicitly to the OS; it has its own copy.
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. > 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.
On Sun, Dec 26, 2021 at 4:36 PM Hector Martin <marcan@marcan.st> wrote: > Merry Christmas! This year Santa brings us a 34-patch series to add > proper support for the Broadcom FullMAC chips used on Apple T2 and M1 > platforms: I tried to review as best I could, when I think I know what I'm doing I state Reviewed-by and when I think it just LooksGoodToMe(TM) I replied Acked-by. If I missed some patch you can assume Acked-by from me on these as well. Thanks for doing this, some really old bugs and code improvements long overdue is in the series, much appreciated. Yours, Linus Walleij
On 2021/12/30 1:38, Mark Kettenis wrote: >> From: Hector Martin <marcan@marcan.st> >> 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.
On 2022/01/02 15:25, Linus Walleij wrote: > On Sun, Dec 26, 2021 at 4:36 PM Hector Martin <marcan@marcan.st> wrote: > >> Merry Christmas! This year Santa brings us a 34-patch series to add >> proper support for the Broadcom FullMAC chips used on Apple T2 and M1 >> platforms: > > I tried to review as best I could, when I think I know what I'm doing I state > Reviewed-by and when I think it just LooksGoodToMe(TM) I replied > Acked-by. If I missed some patch you can assume Acked-by from me > on these as well. > > Thanks for doing this, some really old bugs and code improvements long > overdue is in the series, much appreciated. > > Yours, > Linus Walleij > Thanks for the comprehensive review! I'm glad this all makes some sense and I'm not crazy about the approach :) I'll wait a bit for any other feedback that might come in and then submit a v2 with the fixes/changes mentioned so far. Cheers,
On 1/3/2022 7:27 AM, Hector Martin wrote: > On 2022/01/02 15:25, Linus Walleij wrote: >> On Sun, Dec 26, 2021 at 4:36 PM Hector Martin <marcan@marcan.st> wrote: >> >>> Merry Christmas! This year Santa brings us a 34-patch series to add >>> proper support for the Broadcom FullMAC chips used on Apple T2 and M1 >>> platforms: >> >> I tried to review as best I could, when I think I know what I'm doing I state >> Reviewed-by and when I think it just LooksGoodToMe(TM) I replied >> Acked-by. If I missed some patch you can assume Acked-by from me >> on these as well. >> >> Thanks for doing this, some really old bugs and code improvements long >> overdue is in the series, much appreciated. >> >> Yours, >> Linus Walleij >> > > Thanks for the comprehensive review! I'm glad this all makes some sense > and I'm not crazy about the approach :) > > I'll wait a bit for any other feedback that might come in and then > submit a v2 with the fixes/changes mentioned so far. Happy new year to you all. I wanted to start the new year relaxing by reviewing this series, but with the comments already given I prefer to do that with v2 so don't wait for me :-) Regards, Arend