| Message ID | 20220104072658.69756-17-marcan@marcan.st |
|---|---|
| State | Accepted |
| Commit | 0f485805d008aa56644f68179a7e6579fc1515e7 |
| Headers | show |
| Series | brcmfmac: Support Apple T2 and M1 platforms | expand |
On 1/4/2022 8:26 AM, Hector Martin 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 | 47 +++++++++++++++++++ > .../broadcom/brcm80211/brcmfmac/common.c | 1 + > .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ > 4 files changed, 59 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..2b1a4448b291 > --- /dev/null > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > @@ -0,0 +1,47 @@ > +// SPDX-License-Identifier: ISC > +/* > + * Copyright The Asahi Linux Contributors > + */ Common format for copyright statement (in this folder) seems to be: Copyright (c) <YEAR> <COPYRIGHT_HOLDER> Regards, Arend
On 2022/01/10 18:11, Arend van Spriel wrote: > On 1/4/2022 8:26 AM, Hector Martin 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 | 47 +++++++++++++++++++ >> .../broadcom/brcm80211/brcmfmac/common.c | 1 + >> .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ >> 4 files changed, 59 insertions(+) >> create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c > > [...] > >> 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..2b1a4448b291 >> --- /dev/null >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c >> @@ -0,0 +1,47 @@ >> +// 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; >> + const union acpi_object *o; >> + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL}; >> + struct acpi_device *adev = ACPI_COMPANION(dev); >> + >> + if (!adev) >> + return; >> + >> + if (!ACPI_FAILURE(acpi_dev_get_property(adev, "module-instance", >> + ACPI_TYPE_STRING, &o))) { >> + brcmf_dbg(INFO, "ACPI module-instance=%s\n", o->string.pointer); >> + settings->board_type = devm_kasprintf(dev, GFP_KERNEL, >> + "apple,%s", >> + o->string.pointer); >> + } else { >> + brcmf_dbg(INFO, "No ACPI module-instance\n"); > > Do you need to obtain the antenna-sku when there is no module-instance? In principle I don't think any machines would have antenna-sku and no module-instance, though the firmware selection will still work without it (it'll just end up using the DMI machine name instead). > >> + } >> + >> + status = acpi_evaluate_object(adev->handle, "RWCV", NULL, &buf); > > Can you clarify what the above does? What does the "RWCV" mean? No idea what it *means* :-) What it is, though, is the ACPI method name to get the antenna-sku.
On 1/10/2022 12:07 PM, Hector Martin wrote: > On 2022/01/10 18:11, Arend van Spriel wrote: >> On 1/4/2022 8:26 AM, Hector Martin 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 | 47 +++++++++++++++++++ >>> .../broadcom/brcm80211/brcmfmac/common.c | 1 + >>> .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ >>> 4 files changed, 59 insertions(+) >>> create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c >> >> [...] >> >>> 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..2b1a4448b291 >>> --- /dev/null >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c >>> @@ -0,0 +1,47 @@ >>> +// 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; >>> + const union acpi_object *o; >>> + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL}; >>> + struct acpi_device *adev = ACPI_COMPANION(dev); >>> + >>> + if (!adev) >>> + return; >>> + >>> + if (!ACPI_FAILURE(acpi_dev_get_property(adev, "module-instance", >>> + ACPI_TYPE_STRING, &o))) { >>> + brcmf_dbg(INFO, "ACPI module-instance=%s\n", o->string.pointer); >>> + settings->board_type = devm_kasprintf(dev, GFP_KERNEL, >>> + "apple,%s", >>> + o->string.pointer); >>> + } else { >>> + brcmf_dbg(INFO, "No ACPI module-instance\n"); >> >> Do you need to obtain the antenna-sku when there is no module-instance? > > In principle I don't think any machines would have antenna-sku and no > module-instance, though the firmware selection will still work without > it (it'll just end up using the DMI machine name instead). Right. That was my assumption as well. I would bail out here and skip obtaining the antenna-sku. >> >>> + } >>> + >>> + status = acpi_evaluate_object(adev->handle, "RWCV", NULL, &buf); >> >> Can you clarify what the above does? What does the "RWCV" mean? > > No idea what it *means* :-) > > What it is, though, is the ACPI method name to get the antenna-sku. Wow. So much for meaning-full naming ;-)
Hector Martin <marcan@marcan.st> writes: > On 2022/01/04 19:21, Arend van Spriel wrote: >> On 1/4/2022 8:26 AM, Hector Martin 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 | 47 +++++++++++++++++++ >>> .../broadcom/brcm80211/brcmfmac/common.c | 1 + >>> .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ >>> 4 files changed, 59 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..2b1a4448b291 >>> --- /dev/null >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c >>> @@ -0,0 +1,47 @@ >>> +// SPDX-License-Identifier: ISC >>> +/* >>> + * Copyright The Asahi Linux Contributors >>> + */ >> >> Common format for copyright statement (in this folder) seems to be: >> >> Copyright (c) <YEAR> <COPYRIGHT_HOLDER> >> >> Regards, >> Arend > > I get this every time I submit a patch to a new subsystem :-) > > This is based on this best practice: > > https://www.linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/ I didn't know about this recommendation, thanks. > Basically, the year format quickly becomes outdated and is rather > useless, and listing specific names also ends up missing every > subsequent contributor, so more general copyright statements work better > for this kind of use case. In the end we all know git history is the > proper record of copyright status. > > I don't have a super strong opinion here, but we've been trying to > standardize on this format for contributions coming from our subproject, > and it feels more useful than a random contributor's name with a date 5 > years in the past :) If LF is fine with this approach, then it's good enough also for me. So at least from my point of view no need to make any changes.
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..2b1a4448b291 --- /dev/null +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c @@ -0,0 +1,47 @@ +// 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; + const union acpi_object *o; + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL}; + struct acpi_device *adev = ACPI_COMPANION(dev); + + if (!adev) + return; + + if (!ACPI_FAILURE(acpi_dev_get_property(adev, "module-instance", + ACPI_TYPE_STRING, &o))) { + brcmf_dbg(INFO, "ACPI module-instance=%s\n", o->string.pointer); + settings->board_type = devm_kasprintf(dev, GFP_KERNEL, + "apple,%s", + o->string.pointer); + } 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 | 47 +++++++++++++++++++ .../broadcom/brcm80211/brcmfmac/common.c | 1 + .../broadcom/brcm80211/brcmfmac/common.h | 9 ++++ 4 files changed, 59 insertions(+) create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/acpi.c