[PATCH] brcmfmac: of: Use board compatible string for board type

[PATCH] brcmfmac: of: Use board compatible string for board type

[Ivan T. Ivanov]

When "brcm,board-type" is not explicitly set in devicetree
fallback to board compatible string for board type.

Some of the existing devices rely on the most compatible device
string to find best firmware files, including Raspberry PI's[1].

Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")

[1] https://bugzilla.opensuse.org/show_bug.cgi?id=1206697#c13

Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index a83699de01ec..fdd0c9abc1a1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -79,7 +79,8 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 	/* Apple ARM64 platforms have their own idea of board type, passed in
 	 * via the device tree. They also have an antenna SKU parameter
 	 */
-	if (!of_property_read_string(np, "brcm,board-type", &prop))
+	err = of_property_read_string(np, "brcm,board-type", &prop);
+	if (!err)
 		settings->board_type = prop;
 
 	if (!of_property_read_string(np, "apple,antenna-sku", &prop))
@@ -87,7 +88,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 
 	/* Set board-type to the first string of the machine compatible prop */
 	root = of_find_node_by_path("/");
-	if (root && !settings->board_type) {
+	if (root && err) {
 		char *board_type;
 		const char *tmp;
 
-- 
2.35.3

Re: [PATCH] brcmfmac: of: Use board compatible string for board type

[Hector Martin]

On 2023/01/06 16:27, Ivan T. Ivanov wrote:
> When "brcm,board-type" is not explicitly set in devicetree
> fallback to board compatible string for board type.
> 
> Some of the existing devices rely on the most compatible device
> string to find best firmware files, including Raspberry PI's[1].
> 
> Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
> 
> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1206697#c13
> 
> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>

The existing code already falls back to the compatible string, *as long
as there is no board_type set already*.

As far as I can tell, the only way the board_type can get another value
first is if it comes from DMI. This behavior was inadvertently changed
by commit 7682de8b3351 (since I was not expecting platforms to have
*both* DT and DMI information).

I'm guessing the Raspberry Pi is one such platform, and
`/sys/devices/virtual/dmi` exists? Hybrid UEFI+ACPI+DT platform I take it?

If so, your commit description should probably be something like:

===
brcmfmac: Prefer DT board type over DMI board type

The introduction of support for Apple board types inadvertently changed
the precedence order, causing hybrid ACPI+DT platforms to look up the
firmware using the DMI information instead of the device tree compatible
to generate the board type. Revert back to the old behavior,
as affected platforms use firmwares named after the DT compatible.

Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
===

An also add a Cc: stable@vger.kernel.org to make sure this gets backported.

With the fixed description,

Reviewed-by: Hector Martin <marcan@marcan.st>

- Hector

Re: [PATCH] brcmfmac: of: Use board compatible string for board type

[Hector Martin]

On 2023/01/06 18:27, Hector Martin wrote:
> On 2023/01/06 16:27, Ivan T. Ivanov wrote:
>> When "brcm,board-type" is not explicitly set in devicetree
>> fallback to board compatible string for board type.
>>
>> Some of the existing devices rely on the most compatible device
>> string to find best firmware files, including Raspberry PI's[1].
>>
>> Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
>>
>> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1206697#c13
>>
>> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> 
> The existing code already falls back to the compatible string, *as long
> as there is no board_type set already*.
> 
> As far as I can tell, the only way the board_type can get another value
> first is if it comes from DMI. This behavior was inadvertently changed
> by commit 7682de8b3351 (since I was not expecting platforms to have
> *both* DT and DMI information).
> 
> I'm guessing the Raspberry Pi is one such platform, and
> `/sys/devices/virtual/dmi` exists? Hybrid UEFI+ACPI+DT platform I take it?
> 
> If so, your commit description should probably be something like:
> 
> ===
> brcmfmac: Prefer DT board type over DMI board type
> 
> The introduction of support for Apple board types inadvertently changed
> the precedence order, causing hybrid ACPI+DT platforms to look up the
> firmware using the DMI information instead of the device tree compatible
> to generate the board type. Revert back to the old behavior,
> as affected platforms use firmwares named after the DT compatible.
> 
> Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
> ===
> 
> An also add a Cc: stable@vger.kernel.org to make sure this gets backported.
> 
> With the fixed description,
> 
> Reviewed-by: Hector Martin <marcan@marcan.st>
> 
> - Hector

Looking into this a bit more from what was mentioned in the linked bug,
the DMI data comes from the SMBIOS table. We don't have that on Apple
platforms even though we also boot via U-Boot+EFI, but I'm guessing you
build U-Boot with CONFIG_GENERATE_SMBIOS_TABLE and provide that stuff in
the DT? So s/ACPI/SMBIOS/ would be more accurate in the commit message.

- Hector

Re: [PATCH] brcmfmac: of: Use board compatible string for board type

[Ivan T. Ivanov]

On 01-06 21:13, Hector Martin wrote:

> 
> On 2023/01/06 18:27, Hector Martin wrote:
> > On 2023/01/06 16:27, Ivan T. Ivanov wrote:
> >> When "brcm,board-type" is not explicitly set in devicetree
> >> fallback to board compatible string for board type.
> >>
> >> Some of the existing devices rely on the most compatible device
> >> string to find best firmware files, including Raspberry PI's[1].
> >>
> >> Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
> >>
> >> [1] https://bugzilla.opensuse.org/show_bug.cgi?id=1206697#c13
> >>
> >> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> > 
> > The existing code already falls back to the compatible string, *as long
> > as there is no board_type set already*.
> > 
> > As far as I can tell, the only way the board_type can get another value
> > first is if it comes from DMI. This behavior was inadvertently changed
> > by commit 7682de8b3351 (since I was not expecting platforms to have
> > *both* DT and DMI information).
> > 
> > I'm guessing the Raspberry Pi is one such platform, and
> > `/sys/devices/virtual/dmi` exists? Hybrid UEFI+ACPI+DT platform I take it?
> > 
> > If so, your commit description should probably be something like:
> > 
> > ===
> > brcmfmac: Prefer DT board type over DMI board type
> > 
> > The introduction of support for Apple board types inadvertently changed
> > the precedence order, causing hybrid ACPI+DT platforms to look up the
> > firmware using the DMI information instead of the device tree compatible
> > to generate the board type. Revert back to the old behavior,
> > as affected platforms use firmwares named after the DT compatible.
> > 
> > Fixes: 7682de8b3351 ("wifi: brcmfmac: of: Fetch Apple properties")
> > ===
> > 
> > An also add a Cc: stable@vger.kernel.org to make sure this gets backported.
> > 
> > With the fixed description,
> > 
> > Reviewed-by: Hector Martin <marcan@marcan.st>
> > 
> > - Hector
> 
> Looking into this a bit more from what was mentioned in the linked bug,
> the DMI data comes from the SMBIOS table. We don't have that on Apple
> platforms even though we also boot via U-Boot+EFI, but I'm guessing you
> build U-Boot with CONFIG_GENERATE_SMBIOS_TABLE and provide that stuff in
> the DT?

Yes, that is the way in openSUSE case.

> So s/ACPI/SMBIOS/ would be more accurate in the commit message.

Sure, I will rewrite commit message and repost.

Thanks!