Message-ID: <4618988.LvFx2qVVIh@lucaweiss.eu>
Date: Mon, 06 Jan 2025 17:51:40 +0100
From: Luca Weiss <luca@...aweiss.eu>
To: Arend van Spriel <arend.vanspriel@...adcom.com>,
 Kalle Valo <kvalo@...nel.org>, linux-wireless@...r.kernel.org,
 brcm80211@...ts.linux.dev, brcm80211-dev-list.pdl@...adcom.com,
 linux-kernel@...r.kernel.org, Alex Bee <knaerzche@...il.com>
Cc: ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v2] wifi: brcmfmac: of: Support interrupts-extended

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@...il.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;
> >>
> >
> >
> >
> 

Powered by blists - more mailing lists