| Message ID | 20211226153624.162281-17-marcan@marcan.st (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: Support Apple T2 and M1 platforms | expand |
On Sun, Dec 26, 2021 at 4:38 PM Hector Martin <marcan@marcan.st> wrote: > On DT platforms, the module-instance and antenna-sku-info properties > are passed in the DT. On ACPI platforms, module-instance is passed via > the analogous Apple device property mechanism, while the antenna SKU > info is instead obtained via an ACPI method that grabs it from > non-volatile storage. > > Add support for this, to allow proper firmware selection on Apple > platforms. > > Signed-off-by: Hector Martin <marcan@marcan.st> If the strings treated here are exactly the same as for the device tree, you should be able to just use "devprops" (firmware node) to handle it abstractly, and then the respective DT and ACPI backend will provide the properties. I don't know if this patch I made recently is enough of an examples: https://lore.kernel.org/linux-hwmon/20211206020423.62402-2-linus.walleij@linaro.org/ If the ACPI and DT differs a lot in format and strings etc it may not be worth it. Yours, Linus Walleij
On 2022/01/02 14:58, Linus Walleij wrote: > On Sun, Dec 26, 2021 at 4:38 PM Hector Martin <marcan@marcan.st> wrote: > >> On DT platforms, the module-instance and antenna-sku-info properties >> are passed in the DT. On ACPI platforms, module-instance is passed via >> the analogous Apple device property mechanism, while the antenna SKU >> info is instead obtained via an ACPI method that grabs it from >> non-volatile storage. >> >> Add support for this, to allow proper firmware selection on Apple >> platforms. >> >> Signed-off-by: Hector Martin <marcan@marcan.st> > > If the strings treated here are exactly the same as for the device tree, > you should be able to just use "devprops" (firmware node) to handle it > abstractly, and then the respective DT and ACPI backend will provide > the properties. > > I don't know if this patch I made recently is enough of an examples: > https://lore.kernel.org/linux-hwmon/20211206020423.62402-2-linus.walleij@linaro.org/ > > If the ACPI and DT differs a lot in format and strings etc it may not > be worth it. It's not quite the same; module-instance is the same from macOS' perspective, but we don't use Apple's device tree directly but rather roll our own DT which uses a different property name in this case. antenna-sku-info uses an ACPI method on x86, so that one is completely different. So in the end nothing is actually shared.
On Mon, Jan 3, 2022 at 7:03 AM Hector Martin <marcan@marcan.st> wrote: > On 2022/01/02 14:58, Linus Walleij wrote: > > On Sun, Dec 26, 2021 at 4:38 PM Hector Martin <marcan@marcan.st> wrote: > > > >> On DT platforms, the module-instance and antenna-sku-info properties > >> are passed in the DT. On ACPI platforms, module-instance is passed via > >> the analogous Apple device property mechanism, while the antenna SKU > >> info is instead obtained via an ACPI method that grabs it from > >> non-volatile storage. > >> > >> Add support for this, to allow proper firmware selection on Apple > >> platforms. > >> > >> Signed-off-by: Hector Martin <marcan@marcan.st> > > > > If the strings treated here are exactly the same as for the device tree, > > you should be able to just use "devprops" (firmware node) to handle it > > abstractly, and then the respective DT and ACPI backend will provide > > the properties. > > > > I don't know if this patch I made recently is enough of an examples: > > https://lore.kernel.org/linux-hwmon/20211206020423.62402-2-linus.walleij@linaro.org/ > > > > If the ACPI and DT differs a lot in format and strings etc it may not > > be worth it. > > It's not quite the same; module-instance is the same from macOS' > perspective, but we don't use Apple's device tree directly but rather > roll our own DT which uses a different property name in this case. > antenna-sku-info uses an ACPI method on x86, so that one is completely > different. So in the end nothing is actually shared. OK then! Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On 2022/01/04 1:20, Andy Shevchenko wrote: > +void brcmf_acpi_probe(struct device *dev, enum brcmf_bus_type bus_type, > + struct brcmf_mp_device *settings) > +{ > + acpi_status status; > + struct acpi_device *adev = ACPI_COMPANION(dev); > > > Please, move the assignment closer to its first user So... two lines down? :-) > > > + const union acpi_object *o; > + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL}; > + > + if (!adev) > + return; > + > + if (!ACPI_FAILURE(acpi_dev_get_property(adev, "module-instance", > + ACPI_TYPE_STRING, > &o))) { > + const char *prefix = "apple,"; > + int len = strlen(prefix) + o->string.length + 1; > + char *board_type = devm_kzalloc(dev, len, GFP_KERNEL); > + > + strscpy(board_type, prefix, len); > + strlcat(board_type, o->string.pointer, > > > NIH devm_kasprintf()? That sounds useful, didn't know that existed. Thanks! > > > + brcmf_dbg(INFO, "ACPI module-instance=%s\n", > o->string.pointer); > + settings->board_type = board_type; > + } else { > + brcmf_dbg(INFO, "No ACPI module-instance\n"); > + } > + > + status = acpi_evaluate_object(adev->handle, "RWCV", NULL, &buf); > + o = buf.pointer; > + if (!ACPI_FAILURE(status) && o && o->type == ACPI_TYPE_BUFFER && > + o->buffer.length >= 2) { > + char *antenna_sku = devm_kzalloc(dev, 3, GFP_KERNEL); > + > + memcpy(antenna_sku, o->buffer.pointer, 2); > > > NIH devm_kmemdup()? Not *quite*. I take the first two bytes of the returned buffer and turn them into a null-terminated 3-byte string. kmemdup wouldn't null-terminate or would copy too much, depending on length.
On 2022/01/04 7:50, Andy Shevchenko wrote: > > + status = acpi_evaluate_object(adev->handle, "RWCV", > NULL, &buf); > > + o = buf.pointer; > > + if (!ACPI_FAILURE(status) && o && o->type == > ACPI_TYPE_BUFFER && > > + o->buffer.length >= 2) { > > + char *antenna_sku = devm_kzalloc(dev, 3, > GFP_KERNEL); > > + > > + memcpy(antenna_sku, o->buffer.pointer, 2); > > > > > > NIH devm_kmemdup()? > > Not *quite*. I take the first two bytes of the returned buffer and turn > them into a null-terminated 3-byte string. kmemdup wouldn't > null-terminate or would copy too much, depending on length. > > > > devm_kstrndup() then? > > That doesn't seem to be a thing.
Andy Shevchenko <andy.shevchenko@gmail.com> writes: > On Sunday, December 26, 2021, Hector Martin <marcan@marcan.st> wrote: > > On DT platforms, the module-instance and antenna-sku-info properties > are passed in the DT. On ACPI platforms, module-instance is passed via > the analogous Apple device property mechanism, while the antenna SKU > info is instead obtained via an ACPI method that grabs it from > non-volatile storage. > > Add support for this, to allow proper firmware selection on Apple > platforms. > > Signed-off-by: Hector Martin <marcan@marcan.st> > --- > .../broadcom/brcm80211/brcmfmac/Makefile | 2 + > .../broadcom/brcm80211/brcmfmac/acpi.c | 51 +++++++++++++++++++ > .../broadcom/brcm80211/brcmfmac/common.c | 1 + > .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ > 4 files changed, 63 insertions(+) > create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile > index 13c13504a6e8..19009eb9db93 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile > @@ -47,3 +47,5 @@ brcmfmac-$(CONFIG_OF) += \ > of.o > brcmfmac-$(CONFIG_DMI) += \ > dmi.o > +brcmfmac-$(CONFIG_ACPI) += \ > + acpi.o > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > new file mode 100644 > index 000000000000..3e56dc7a8db2 > --- /dev/null > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > @@ -0,0 +1,51 @@ > +// SPDX-License-Identifier: ISC > +/* > + * Copyright The Asahi Linux Contributors > + */ > + > +#include <linux/acpi.h> > +#include "debug.h" > +#include "core.h" > +#include "common.h" > + > +void brcmf_acpi_probe(struct device *dev, enum brcmf_bus_type bus_type, > + struct brcmf_mp_device *settings) > +{ > + acpi_status status; > + struct acpi_device *adev = ACPI_COMPANION(dev); > > Please, move the assignment closer to its first user Andy, your email was formatted in HTML. I'm sure you know this already, but our mailing lists drop all HTML emails so other people (and patchwork) don't see your comments.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile index 13c13504a6e8..19009eb9db93 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile @@ -47,3 +47,5 @@ brcmfmac-$(CONFIG_OF) += \ of.o brcmfmac-$(CONFIG_DMI) += \ dmi.o +brcmfmac-$(CONFIG_ACPI) += \ + acpi.o diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c new file mode 100644 index 000000000000..3e56dc7a8db2 --- /dev/null +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: ISC +/* + * Copyright The Asahi Linux Contributors + */ + +#include <linux/acpi.h> +#include "debug.h" +#include "core.h" +#include "common.h" + +void brcmf_acpi_probe(struct device *dev, enum brcmf_bus_type bus_type, + struct brcmf_mp_device *settings) +{ + acpi_status status; + struct acpi_device *adev = ACPI_COMPANION(dev); + const union acpi_object *o; + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL}; + + if (!adev) + return; + + if (!ACPI_FAILURE(acpi_dev_get_property(adev, "module-instance", + ACPI_TYPE_STRING, &o))) { + const char *prefix = "apple,"; + int len = strlen(prefix) + o->string.length + 1; + char *board_type = devm_kzalloc(dev, len, GFP_KERNEL); + + strscpy(board_type, prefix, len); + strlcat(board_type, o->string.pointer, len); + brcmf_dbg(INFO, "ACPI module-instance=%s\n", o->string.pointer); + settings->board_type = board_type; + } else { + brcmf_dbg(INFO, "No ACPI module-instance\n"); + } + + status = acpi_evaluate_object(adev->handle, "RWCV", NULL, &buf); + o = buf.pointer; + if (!ACPI_FAILURE(status) && o && o->type == ACPI_TYPE_BUFFER && + o->buffer.length >= 2) { + char *antenna_sku = devm_kzalloc(dev, 3, GFP_KERNEL); + + memcpy(antenna_sku, o->buffer.pointer, 2); + brcmf_dbg(INFO, "ACPI RWCV data=%*phN antenna-sku=%s\n", + (int)o->buffer.length, o->buffer.pointer, + antenna_sku); + + settings->antenna_sku = antenna_sku; + } else { + brcmf_dbg(INFO, "No ACPI antenna-sku\n"); + } +} diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index b8ed851129b4..c84c48e49fde 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -437,6 +437,7 @@ struct brcmf_mp_device *brcmf_get_module_param(struct device *dev, /* No platform data for this device, try OF and DMI data */ brcmf_dmi_probe(settings, chip, chiprev); brcmf_of_probe(dev, bus_type, settings); + brcmf_acpi_probe(dev, bus_type, settings); } return settings; } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h index d4aa25d646fe..a88c4a9310f3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h @@ -73,6 +73,15 @@ static inline void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev) {} #endif +#ifdef CONFIG_ACPI +void brcmf_acpi_probe(struct device *dev, enum brcmf_bus_type bus_type, + struct brcmf_mp_device *settings); +#else +static inline void brcmf_acpi_probe(struct device *dev, + enum brcmf_bus_type bus_type, + struct brcmf_mp_device *settings) {} +#endif + u8 brcmf_map_prio_to_prec(void *cfg, u8 prio); u8 brcmf_map_prio_to_aci(void *cfg, u8 prio);
On DT platforms, the module-instance and antenna-sku-info properties are passed in the DT. On ACPI platforms, module-instance is passed via the analogous Apple device property mechanism, while the antenna SKU info is instead obtained via an ACPI method that grabs it from non-volatile storage. Add support for this, to allow proper firmware selection on Apple platforms. Signed-off-by: Hector Martin <marcan@marcan.st> --- .../broadcom/brcm80211/brcmfmac/Makefile | 2 + .../broadcom/brcm80211/brcmfmac/acpi.c | 51 +++++++++++++++++++ .../broadcom/brcm80211/brcmfmac/common.c | 1 + .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ 4 files changed, 63 insertions(+) create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c