RE: [PATCH v2 4/6] mmc: host: sdhci-esdhc-imx.c: disable auto-tuning when necessary

From: Bough Chen <haibo.chen@nxp.com>
To: Kevin Groeneveld <kgroeneveld@lenbrook.com>,
	"adrian.hunter@intel.com" <adrian.hunter@intel.com>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>
Cc: "kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH v2 4/6] mmc: host: sdhci-esdhc-imx.c: disable auto-tuning when necessary
Date: Fri, 9 Dec 2022 08:43:37 +0000	[thread overview]
Message-ID: <DB7PR04MB4010F57E5C0A6DC420FA01C5901C9@DB7PR04MB4010.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <a429ea6c-f2d9-ddc8-15b8-23309b3a5051@lenbrook.com>

> -----Original Message-----
> From: Kevin Groeneveld <kgroeneveld@lenbrook.com>
> Sent: 2022年12月5日 23:00
> To: Bough Chen <haibo.chen@nxp.com>; adrian.hunter@intel.com;
> ulf.hansson@linaro.org; shawnguo@kernel.org; robh+dt@kernel.org;
> s.hauer@pengutronix.de
> Cc: kernel@pengutronix.de; festevam@gmail.com; linux-mmc@vger.kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 4/6] mmc: host: sdhci-esdhc-imx.c: disable auto-tuning
> when necessary
> 
> Thank you Haibo for pointing me here from
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spi
> nics.net%2Flists%2Flinux-mmc%2Fmsg73270.html&amp;data=05%7C01%7Chai
> bo.chen%40nxp.com%7C2c5b5f4d53d04051475308dad6d16673%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C638058492114803922%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=sCV8u6Gv7x%2Bqi6kYSvZ
> uZUZeQQ1TaKPwhKpizt49qps%3D&amp;reserved=0.
> 
> On 2021-08-18 07:16, haibo.chen@nxp.com wrote:
> > Add a method to enable/disable auto-tuning function. auto-tuning
> > function is conflict with sdio interrupt. For sdio device with sdio
> > interrupt, need to disable auto-tuning function.
> 
> I tested this patch on an imx8mm system and it made things completely
> unstable. I was never really able to log into the system properly and just got lots
> of messages similar to the following:
> 
> [   31.946640] rcu: INFO: rcu_preempt self-detected stall on CPU
> [   31.952422] rcu:     0-....: (2106 ticks this GP)
> idle=849/1/0x4000000000000000 softirq=902/904 fqs=743
> [   31.961663]  (t=2100 jiffies g=33 q=1158)
> [   31.965682] Task dump for CPU 0:
> [   31.968915] task:kworker/0:1     state:R  running task     stack:
> 0 pid:   33 ppid:     2 flags:0x0000000a
> [   31.978859] Workqueue:  0x0 (pm)

These patch also exist on our local tree, and we do not meet this issue. Can you show me
The detail change you added?

> 
> While working on this I also came across
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcommuni
> ty.nxp.com%2Ft5%2Fi-MX-Processors-Knowledge-Base%2FuSDHC-auto-tuning-a
> nd-possible-SDIO-failures%2Fta-p%2F1352855&amp;data=05%7C01%7Chaibo.c
> hen%40nxp.com%7C2c5b5f4d53d04051475308dad6d16673%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C638058492114960153%7CUnknown%7C
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=w3VEjXfQKTIXvIIef1INySnQFU
> xW09uafNRdkkv8e7M%3D&amp;reserved=0
> which seems to address the same issue as your proposed patch.
> 
> That article suggests only enabling auto tuning for one data line as a
> workaround. I tried this method and so far it seems to have addressed the -84
> errors I was seeing with SDIO communication to a WiFi module.
> 
> Some thoughts / questions:
> 
> Why does this proposed patch make my system unstable? (I was testing with a
> v5.16 mainline based kernel, but I did not see anything in later versions of
> sdhci-esdhc-imx that seemed like this should be a problem.)
> 
> Why does this patch try to disable auto tune entirely vs just setting it up for one
> data bit as suggested in the NXP knowledge base article?
> 
> As some other have suggested it seems like it would be nicer if the workaround
> could be applied automatically if the device using the SDIO interface enabled
> IRQs. Having to include a non standard entry in the DT for a hardware bug you
> may not know about or understand seems error prone. I guess maybe some
> device could generate an IRQ before they actually enable IRQs? In that case
> maybe a DT entry is required, but maybe the driver could generate a warning if
> IRQs are enabled without the DT entry?

Yes, your method seems better, I will try to do like that. Thanks

Best Regards
Haibo Chen
> 
> 
> Thanks,
> Kevin