Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Geraldo Nascimento]

Hi Heiner and Ulf,

After updating to kernel 6.2.0-rc7 from 5.19.1 my H96 Pro+ TV Box
(Amlogic S912) failed to provide wifi. The module in question is a
QCA9377 SDIO driven by ath10k.

Reverting 066ecde6d826b443f492570e080cba3f2212280d
("mmc: meson-gx: add SDIO interrupt support")
solves the problem and I have wifi again.

Thanks,
Geraldo Nascimento

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Heiner Kallweit]

On 11.02.2023 18:48, Geraldo Nascimento wrote:
> Hi Heiner and Ulf,
> 
> After updating to kernel 6.2.0-rc7 from 5.19.1 my H96 Pro+ TV Box
> (Amlogic S912) failed to provide wifi. The module in question is a
> QCA9377 SDIO driven by ath10k.
> 
> Reverting 066ecde6d826b443f492570e080cba3f2212280d
> ("mmc: meson-gx: add SDIO interrupt support")
> solves the problem and I have wifi again.
> 
> Thanks,
> Geraldo Nascimento

Supposedly this patch revealed an issue with incorrect interrupt
trigger types. A fix is currently in testing:
87ef638d6557 ("TEST: arm64: dts: amlogic: Make mmc host controller interrupts level-sensitive")


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Geraldo Nascimento]

On Sat, Feb 11, 2023 at 11:44:25PM +0100, Heiner Kallweit wrote:
> On 11.02.2023 18:48, Geraldo Nascimento wrote:
> > Hi Heiner and Ulf,
> > 
> > After updating to kernel 6.2.0-rc7 from 5.19.1 my H96 Pro+ TV Box
> > (Amlogic S912) failed to provide wifi. The module in question is a
> > QCA9377 SDIO driven by ath10k.
> > 
> > Reverting 066ecde6d826b443f492570e080cba3f2212280d
> > ("mmc: meson-gx: add SDIO interrupt support")
> > solves the problem and I have wifi again.
> > 
> > Thanks,
> > Geraldo Nascimento
> 
> Supposedly this patch revealed an issue with incorrect interrupt
> trigger types. A fix is currently in testing:
> 87ef638d6557 ("TEST: arm64: dts: amlogic: Make mmc host controller interrupts level-sensitive")
>

Hi Heiner, and thanks for the quick reply.

I reverted my revert and added the TEST patch about interrupts in the DT
but still had no luck.

What solved my problem was adding "cap-sdio-irq" to "sd_emmc_a" DT node.
Now all is well.

Thank you,
Geraldo Nascimento

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Christian Hewitt]

> On 12 Feb 2023, at 4:41 am, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> 
> On Sat, Feb 11, 2023 at 11:44:25PM +0100, Heiner Kallweit wrote:
>> On 11.02.2023 18:48, Geraldo Nascimento wrote:
>>> Hi Heiner and Ulf,
>>> 
>>> After updating to kernel 6.2.0-rc7 from 5.19.1 my H96 Pro+ TV Box
>>> (Amlogic S912) failed to provide wifi. The module in question is a
>>> QCA9377 SDIO driven by ath10k.
>>> 
>>> Reverting 066ecde6d826b443f492570e080cba3f2212280d
>>> ("mmc: meson-gx: add SDIO interrupt support")
>>> solves the problem and I have wifi again.
>>> 
>>> Thanks,
>>> Geraldo Nascimento
>> 
>> Supposedly this patch revealed an issue with incorrect interrupt
>> trigger types. A fix is currently in testing:
>> 87ef638d6557 ("TEST: arm64: dts: amlogic: Make mmc host controller interrupts level-sensitive")
>> 
> 
> Hi Heiner, and thanks for the quick reply.
> 
> I reverted my revert and added the TEST patch about interrupts in the DT
> but still had no luck.
> 
> What solved my problem was adding "cap-sdio-irq" to "sd_emmc_a" DT node.
> Now all is well.

Confirming that setting cap-sdio-irq resolved the continuing issues I’ve seen with an
QCA9337 module after restoring the SDIO irq changes and picking recent fixes to my
branch. Thanks Geraldo!

I’ve not observed issues with Broadcom modules, only QCA9377, but is this something
that should be defined for all sd_emmc_a nodes?

Christian
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Heiner Kallweit]

On 12.02.2023 03:38, Christian Hewitt wrote:
> 
>> On 12 Feb 2023, at 4:41 am, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
>>
>> On Sat, Feb 11, 2023 at 11:44:25PM +0100, Heiner Kallweit wrote:
>>> On 11.02.2023 18:48, Geraldo Nascimento wrote:
>>>> Hi Heiner and Ulf,
>>>>
>>>> After updating to kernel 6.2.0-rc7 from 5.19.1 my H96 Pro+ TV Box
>>>> (Amlogic S912) failed to provide wifi. The module in question is a
>>>> QCA9377 SDIO driven by ath10k.
>>>>
>>>> Reverting 066ecde6d826b443f492570e080cba3f2212280d
>>>> ("mmc: meson-gx: add SDIO interrupt support")
>>>> solves the problem and I have wifi again.
>>>>
>>>> Thanks,
>>>> Geraldo Nascimento
>>>
>>> Supposedly this patch revealed an issue with incorrect interrupt
>>> trigger types. A fix is currently in testing:
>>> 87ef638d6557 ("TEST: arm64: dts: amlogic: Make mmc host controller interrupts level-sensitive")
>>>
>>
>> Hi Heiner, and thanks for the quick reply.
>>
>> I reverted my revert and added the TEST patch about interrupts in the DT
>> but still had no luck.
>>
>> What solved my problem was adding "cap-sdio-irq" to "sd_emmc_a" DT node.
>> Now all is well.
> 
> Confirming that setting cap-sdio-irq resolved the continuing issues I’ve seen with an
> QCA9337 module after restoring the SDIO irq changes and picking recent fixes to my
> branch. Thanks Geraldo!
> 
> I’ve not observed issues with Broadcom modules, only QCA9377, but is this something
> that should be defined for all sd_emmc_a nodes?
> 
> Christian

Could you please test whether the following fixes the issue for you
if cap-sdio-irq isn't set?


diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 6e5ea0213..34f2a8df8 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -435,7 +435,8 @@ static int meson_mmc_clk_init(struct meson_host *host)
 	clk_reg |= FIELD_PREP(CLK_CORE_PHASE_MASK, CLK_PHASE_180);
 	clk_reg |= FIELD_PREP(CLK_TX_PHASE_MASK, CLK_PHASE_0);
 	clk_reg |= FIELD_PREP(CLK_RX_PHASE_MASK, CLK_PHASE_0);
-	clk_reg |= CLK_IRQ_SDIO_SLEEP(host);
+	if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
+		clk_reg |= CLK_IRQ_SDIO_SLEEP(host);
 	writel(clk_reg, host->regs + SD_EMMC_CLOCK);
 
 	/* get the mux parents */
@@ -948,11 +949,13 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 {
 	struct meson_host *host = dev_id;
 	struct mmc_command *cmd;
-	u32 status, raw_status;
+	u32 status, raw_status, irq_mask = IRQ_EN_MASK;
 	irqreturn_t ret = IRQ_NONE;
 
+	if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
+		irq_mask |= IRQ_SDIO;
 	raw_status = readl(host->regs + SD_EMMC_STATUS);
-	status = raw_status & (IRQ_EN_MASK | IRQ_SDIO);
+	status = raw_status & irq_mask;
 
 	if (!status) {
 		dev_dbg(host->dev,
@@ -1204,6 +1207,11 @@ static int meson_mmc_probe(struct platform_device *pdev)
 		goto free_host;
 	}
 
+	mmc->caps |= MMC_CAP_CMD23;
+
+	if (mmc->caps & MMC_CAP_SDIO_IRQ)
+		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
+
 	host->data = (struct meson_mmc_data *)
 		of_device_get_match_data(&pdev->dev);
 	if (!host->data) {
@@ -1277,11 +1285,6 @@ static int meson_mmc_probe(struct platform_device *pdev)
 
 	spin_lock_init(&host->lock);
 
-	mmc->caps |= MMC_CAP_CMD23;
-
-	if (mmc->caps & MMC_CAP_SDIO_IRQ)
-		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
-
 	if (host->dram_access_quirk) {
 		/* Limit segments to 1 due to low available sram memory */
 		mmc->max_segs = 1;
-- 
2.39.1



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Geraldo Nascimento]

On Sun, Feb 12, 2023 at 12:55:25PM +0100, Heiner Kallweit wrote:
> 
> Could you please test whether the following fixes the issue for you
> if cap-sdio-irq isn't set?
>

Hi Heiner,

With this patch there are no problems even if cap-sdio-irq isn't set.

But shouldn't we set this capability to take advantage of your new
codepath? I noticed that on the old Amlogic BSP (downstream kernel) such
capability was already being set on the DT, so it's not some sort of
hack.

Thank you,
Geraldo Nascimento

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: Regression caused by 066ecde6d826b443 ("mmc: meson-gx: add SDIO interrupt support")

[Heiner Kallweit]

On 12.02.2023 21:32, Geraldo Nascimento wrote:
> On Sun, Feb 12, 2023 at 12:55:25PM +0100, Heiner Kallweit wrote:
>>
>> Could you please test whether the following fixes the issue for you
>> if cap-sdio-irq isn't set?
>>
> 
> Hi Heiner,
> 
> With this patch there are no problems even if cap-sdio-irq isn't set.
> 
Thanks for testing and the feedback!

> But shouldn't we set this capability to take advantage of your new
> codepath? I noticed that on the old Amlogic BSP (downstream kernel) such
> capability was already being set on the DT, so it's not some sort of
> hack.
> 
Right, next step is to add cap-sdio-irq to the chip family dtsi's.
My intention was to let users opt-in for SDIO IRQ mode and test,
before it's rolled out to all users.

> Thank you,
> Geraldo Nascimento

Heiner


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic