[PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Alex Bee]

The currently existing of_property_present check for interrupts does not
cover all ways interrupts can be defined in a device tree, e.g.
"interrupts-extended".

In order to support all current and future ways that can be done, drop that
check and call of_irq_parse_one to figure out if an interrupt is defined
and irq_create_of_mapping for the actual mapping and let it be handled by
the interrupt subsystem.

Signed-off-by: Alex Bee <knaerzche@gmail.com>
---
Link to v1:
https://lore.kernel.org/all/20240621225558.280462-1-knaerzche@gmail.com/

 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 e406e11481a6..fe4f65756105 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -70,6 +70,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 {
 	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
 	struct device_node *root, *np = dev->of_node;
+	struct of_phandle_args oirq;
 	const char *prop;
 	int irq;
 	int err;
@@ -129,10 +130,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 		sdio->drive_strength = val;
 
 	/* make sure there are interrupts defined in the node */
-	if (!of_property_present(np, "interrupts"))
+	if (of_irq_parse_one(np, 0, &oirq))
 		return;
 
-	irq = irq_of_parse_and_map(np, 0);
+	irq = irq_create_of_mapping(&oirq);
 	if (!irq) {
 		brcmf_err("interrupt could not be mapped\n");
 		return;
-- 
2.45.2

Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

On 6/22/2024 11:54 PM, Alex Bee wrote:
> The currently existing of_property_present check for interrupts does not
> cover all ways interrupts can be defined in a device tree, e.g.
> "interrupts-extended".
> 
> In order to support all current and future ways that can be done, drop that
> check and call of_irq_parse_one to figure out if an interrupt is defined
> and irq_create_of_mapping for the actual mapping and let it be handled by
> the interrupt subsystem.

Thanks for the rework. Looks good.

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> Link to v1:
> https://lore.kernel.org/all/20240621225558.280462-1-knaerzche@gmail.com/
> 
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Kalle Valo]

Alex Bee <knaerzche@gmail.com> wrote:

> The currently existing of_property_present check for interrupts does not
> cover all ways interrupts can be defined in a device tree, e.g.
> "interrupts-extended".
> 
> In order to support all current and future ways that can be done, drop that
> check and call of_irq_parse_one to figure out if an interrupt is defined
> and irq_create_of_mapping for the actual mapping and let it be handled by
> the interrupt subsystem.
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>

Patch applied to wireless-next.git, thanks.

f3f942d6e588 wifi: brcmfmac: of: Support interrupts-extended

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240622215416.659208-1-knaerzche@gmail.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Luca Weiss]

Hi Alex,

On zaterdag 22 juni 2024 23:54:16 Midden-Europese standaardtijd Alex Bee wrote:
> The currently existing of_property_present check for interrupts does not
> cover all ways interrupts can be defined in a device tree, e.g.
> "interrupts-extended".
> 
> In order to support all current and future ways that can be done, drop that
> check and call of_irq_parse_one to figure out if an interrupt is defined
> and irq_create_of_mapping for the actual mapping and let it be handled by
> the interrupt subsystem.

This commit seems to break WiFi on qcom/apq8026-asus-sparrow on 6.11.11
release, and I'm guessing also other apq8026 devices that I have.

dmesg looks like the following on this board:

[   33.699503] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
[   33.811445] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
[   36.565674] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
[   36.565725] ieee80211 phy0: brcmf_bus_started: failed: -110
[   36.565777] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
[   36.575930] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed

After reverting this commit it works well again:

[  141.299739] cfg80211: Loading compiled-in X.509 certificates for regulatory database
[  141.355040] Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[  141.364695] Loaded X.509 cert 'wens: 61c038651aabdcf94bd0ac7ff06c7248db18c600'
[  141.469042] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
[  141.612365] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
[  141.730597] brcmfmac: brcmf_c_process_clm_blob: no clm_blob available (err=-2), device may have limited channels available
[  141.730661] brcmfmac: brcmf_c_process_txcap_blob: no txcap_blob available (err=-2)
[  141.731778] brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM43430/1 wl0: Jun 23 2016 21:14:35 version 7.10.324.google.security.test (TOB) (r640416) FWID 01-495ddd98

Would you have a different solution apart from just reverting this commit?

Regards
Luca

> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> Link to v1:
> https://lore.kernel.org/all/20240621225558.280462-1-knaerzche@gmail.com/
> 
>  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 e406e11481a6..fe4f65756105 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -70,6 +70,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>  {
>  	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
>  	struct device_node *root, *np = dev->of_node;
> +	struct of_phandle_args oirq;
>  	const char *prop;
>  	int irq;
>  	int err;
> @@ -129,10 +130,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>  		sdio->drive_strength = val;
>  
>  	/* make sure there are interrupts defined in the node */
> -	if (!of_property_present(np, "interrupts"))
> +	if (of_irq_parse_one(np, 0, &oirq))
>  		return;
>  
> -	irq = irq_of_parse_and_map(np, 0);
> +	irq = irq_create_of_mapping(&oirq);
>  	if (!irq) {
>  		brcmf_err("interrupt could not be mapped\n");
>  		return;
> 

Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Alex Bee]

Hi Luca,

> Hi Alex,
>
> On zaterdag 22 juni 2024 23:54:16 Midden-Europese standaardtijd Alex Bee wrote:
>> The currently existing of_property_present check for interrupts does not
>> cover all ways interrupts can be defined in a device tree, e.g.
>> "interrupts-extended".
>>
>> In order to support all current and future ways that can be done, drop that
>> check and call of_irq_parse_one to figure out if an interrupt is defined
>> and irq_create_of_mapping for the actual mapping and let it be handled by
>> the interrupt subsystem.
> This commit seems to break WiFi on qcom/apq8026-asus-sparrow on 6.11.11
> release, and I'm guessing also other apq8026 devices that I have.
>
> dmesg looks like the following on this board:
>
> [   33.699503] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
> [   33.811445] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
> [   36.565674] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> [   36.565725] ieee80211 phy0: brcmf_bus_started: failed: -110
> [   36.565777] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
> [   36.575930] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed
>
> After reverting this commit it works well again:
>
> [  141.299739] cfg80211: Loading compiled-in X.509 certificates for regulatory database
> [  141.355040] Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> [  141.364695] Loaded X.509 cert 'wens: 61c038651aabdcf94bd0ac7ff06c7248db18c600'
> [  141.469042] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
> [  141.612365] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
> [  141.730597] brcmfmac: brcmf_c_process_clm_blob: no clm_blob available (err=-2), device may have limited channels available
> [  141.730661] brcmfmac: brcmf_c_process_txcap_blob: no txcap_blob available (err=-2)
> [  141.731778] brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM43430/1 wl0: Jun 23 2016 21:14:35 version 7.10.324.google.security.test (TOB) (r640416) FWID 01-495ddd98
>
> Would you have a different solution apart from just reverting this commit?


it's very likely that the interrupts are wrongly defined in the device tree

and nobody noticed it yet because the driver didn't pick them up before
this commit.

In particular, looking at qcom-apq8026-asus-sparrow.dts: I would guess that
the IRQ_TYPE_EDGE_FALLING trigger doesn't make much sense if this pinctrl
is set to bias-disable.

You could try changing this to IRQ_TYPE_EDGE_RISING. If changing the IRQ
trigger flag doesn't help, you should check if your board's wifi interrupt
pin is really connected to gpio46.

(Please don't forget to submit a patch if you can fix the problem)

Regards,
Alex

> Regards
> Luca
>
>> Signed-off-by: Alex Bee <knaerzche@gmail.com>
>> ---
>> Link to v1:
>> https://lore.kernel.org/all/20240621225558.280462-1-knaerzche@gmail.com/
>>
>>   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 e406e11481a6..fe4f65756105 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
>> @@ -70,6 +70,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>>   {
>>   	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
>>   	struct device_node *root, *np = dev->of_node;
>> +	struct of_phandle_args oirq;
>>   	const char *prop;
>>   	int irq;
>>   	int err;
>> @@ -129,10 +130,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>>   		sdio->drive_strength = val;
>>   
>>   	/* make sure there are interrupts defined in the node */
>> -	if (!of_property_present(np, "interrupts"))
>> +	if (of_irq_parse_one(np, 0, &oirq))
>>   		return;
>>   
>> -	irq = irq_of_parse_and_map(np, 0);
>> +	irq = irq_create_of_mapping(&oirq);
>>   	if (!irq) {
>>   		brcmf_err("interrupt could not be mapped\n");
>>   		return;
>>
>
>
>

Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

[Luca Weiss]

Hi Alex,

On zondag 5 januari 2025 21:36:25 Midden-Europese standaardtijd Alex Bee wrote:
> Hi Luca,
> 
> > Hi Alex,
> >
> > On zaterdag 22 juni 2024 23:54:16 Midden-Europese standaardtijd Alex Bee wrote:
> >> The currently existing of_property_present check for interrupts does not
> >> cover all ways interrupts can be defined in a device tree, e.g.
> >> "interrupts-extended".
> >>
> >> In order to support all current and future ways that can be done, drop that
> >> check and call of_irq_parse_one to figure out if an interrupt is defined
> >> and irq_create_of_mapping for the actual mapping and let it be handled by
> >> the interrupt subsystem.
> > This commit seems to break WiFi on qcom/apq8026-asus-sparrow on 6.11.11
> > release, and I'm guessing also other apq8026 devices that I have.
> >
> > dmesg looks like the following on this board:
> >
> > [   33.699503] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
> > [   33.811445] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
> > [   36.565674] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> > [   36.565725] ieee80211 phy0: brcmf_bus_started: failed: -110
> > [   36.565777] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
> > [   36.575930] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed
> >
> > After reverting this commit it works well again:
> >
> > [  141.299739] cfg80211: Loading compiled-in X.509 certificates for regulatory database
> > [  141.355040] Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> > [  141.364695] Loaded X.509 cert 'wens: 61c038651aabdcf94bd0ac7ff06c7248db18c600'
> > [  141.469042] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43430-sdio for chip BCM43430/1
> > [  141.612365] brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43430-sdio.clm_blob failed with error -2
> > [  141.730597] brcmfmac: brcmf_c_process_clm_blob: no clm_blob available (err=-2), device may have limited channels available
> > [  141.730661] brcmfmac: brcmf_c_process_txcap_blob: no txcap_blob available (err=-2)
> > [  141.731778] brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM43430/1 wl0: Jun 23 2016 21:14:35 version 7.10.324.google.security.test (TOB) (r640416) FWID 01-495ddd98
> >
> > Would you have a different solution apart from just reverting this commit?
> 
> 
> it's very likely that the interrupts are wrongly defined in the device tree
> 
> and nobody noticed it yet because the driver didn't pick them up before
> this commit.

Ah, would make sense. At least from what I can find easily upstream only my
3 apq8026 boards and rk3368-lba3368 is using interrupts-extended. For example
qcom-msm8974pro-samsung-klte-common.dtsi is using interrupt-parent +
interrupts.

> 
> In particular, looking at qcom-apq8026-asus-sparrow.dts: I would guess that
> the IRQ_TYPE_EDGE_FALLING trigger doesn't make much sense if this pinctrl
> is set to bias-disable.
> 
> You could try changing this to IRQ_TYPE_EDGE_RISING. If changing the IRQ
> trigger flag doesn't help, you should check if your board's wifi interrupt
> pin is really connected to gpio46.

I was re-reading the downstream code and if I follow it correctly,
EDGE_RISING should be correct.

https://android.googlesource.com/kernel/msm/+/refs/heads/android-msm-sparrow-3.10-marshmallow-mr1-wear-release/arch/arm/mach-msm/board_wifi_bcm.c#451
(interesting lines are also with Ctrl-F for WLAN_HOSTWAKE)

> 
> (Please don't forget to submit a patch if you can fix the problem)

For sure, I will test this since it looks like I copy pasted the wrong
definition to all of my apq8026 smartwatches.

> 
> Regards,
> Alex
> 
> > Regards
> > Luca
> >
> >> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> >> ---
> >> Link to v1:
> >> https://lore.kernel.org/all/20240621225558.280462-1-knaerzche@gmail.com/
> >>
> >>   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 e406e11481a6..fe4f65756105 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> >> @@ -70,6 +70,7 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
> >>   {
> >>   	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
> >>   	struct device_node *root, *np = dev->of_node;
> >> +	struct of_phandle_args oirq;
> >>   	const char *prop;
> >>   	int irq;
> >>   	int err;
> >> @@ -129,10 +130,10 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
> >>   		sdio->drive_strength = val;
> >>   
> >>   	/* make sure there are interrupts defined in the node */
> >> -	if (!of_property_present(np, "interrupts"))
> >> +	if (of_irq_parse_one(np, 0, &oirq))
> >>   		return;
> >>   
> >> -	irq = irq_of_parse_and_map(np, 0);
> >> +	irq = irq_create_of_mapping(&oirq);
> >>   	if (!irq) {
> >>   		brcmf_err("interrupt could not be mapped\n");
> >>   		return;
> >>
> >
> >
> >
>