From mboxrd@z Thu Jan 1 00:00:00 1970 From: Baruch Siach Date: Tue, 23 Jul 2019 16:45:10 +0300 Subject: [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect In-Reply-To: References: <091c702edd64fc04f3f54d6ea9285d371997e6a9.1563815306.git.baruch@tkos.co.il> <20190723090903.2gzbhcfofmzi7kkp@sapphire.tkos.co.il> <80b2fb45-4d8f-2438-7e84-7b4d44142bf8@ti.com> <20190723104645.noasbqckkhqrchhb@sapphire.tkos.co.il> Message-ID: <20190723134510.2zeubmm6uhccqczn@sapphire.tkos.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Faiz, On Tue, Jul 23, 2019 at 04:47:47PM +0530, Faiz Abbas wrote: > On 23/07/19 4:16 PM, Baruch Siach wrote: > > On Tue, Jul 23, 2019 at 03:35:31PM +0530, Faiz Abbas wrote: > >> On 23/07/19 2:39 PM, Baruch Siach wrote: > >>> On Tue, Jul 23, 2019 at 02:27:28PM +0530, Faiz Abbas wrote: > >>>> On 23/07/19 1:30 PM, Peng Fan wrote: > >>>>> + Faiz > >>>>> > >>>>>> Subject: [PATCH] mmd: sdhci: fix non GPIO card detect > >>>>>> > >>>>>> Some SD cards do not assert the SDHCI_CARD_PRESENT bit. Only the > >>>>>> SDHCI_CARD_DETECT_PIN_LEVEL is enabled. Consider that enough for card > >>>>>> detect indication. > >>>>>> > >>>>>> This fixes SD card access from SPL, since DM_GPIO is not available in SPL > >>>>>> code. > >>>>>> > >>>>>> Fixes: da18c62b6e6a ("mmc: sdhci: Implement SDHCI card detect") > >>>>>> Cc: T Karthik Reddy > >>>>>> Cc: Michal Simek > >>>>>> Signed-off-by: Baruch Siach > >>>>>> --- > >>>>>> drivers/mmc/sdhci.c | 2 +- > >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index > >>>>>> 2779bca93f08..17a28181fcca 100644 > >>>>>> --- a/drivers/mmc/sdhci.c > >>>>>> +++ b/drivers/mmc/sdhci.c > >>>>>> @@ -683,7 +683,7 @@ int sdhci_get_cd(struct udevice *dev) > >>>>>> } > >>>>>> #endif > >>>>>> value = !!(sdhci_readl(host, SDHCI_PRESENT_STATE) & > >>>>>> - SDHCI_CARD_PRESENT); > >>>>>> + (SDHCI_CARD_PRESENT | SDHCI_CARD_DETECT_PIN_LEVEL)); > >>>>> > >>>>> Faiz, are you fine with this change? > >>>> > >>>> Not really. The spec is pretty clear that DETECT_PIN_LEVEL is not to be > >>>> trusted. Also how does the CARD_PRESENT assertion depend on the SD card > >>>> you use? Are you normally muxing the SDCD line to the IP (for hardware > >>>> to detect) or are you connecting it as a gpio which software must detect? > >>> > >>> I tested SanDisk 8GB SD card, class 10, UHS1, on Armada 388 based SolidRun > >>> Clearfog Base. The SDHCI_PRESENT_STATE register consistently reads 0x01f60000, > >>> that is, CARD_PRESENT is disabled, DETECT_PIN_LEVEL is enabled. > >>> > >>> The SD card-detect GPIO is present at the hardware level, but it is not > >>> accessible from SPL code because there is currently no SPL_DM_GPIO. The main > >>> U-Boot image detects the SD card correctly (once the other MMC patches I > >>> posted are applied). > >>> > >>> Without this patch boot from SD card is broken. What is the right fix then? > >> > >> There are two choices to implement card detect: > >> > >> 1. Mux the card detect line from the SD card cage directly to the host > >> controller and expect PRESENT state register to indicate whether card is > >> present or not. > >> > >> 2. Mux the card detect line as a GPIO and use software > >> (dm_gpio_get_value() call) to detect whether card is present or not. In > >> that case, PRESENT_STATE[16,17,18] are completely useless because there > >> is no card detect line going into the IP. > >> > >> It seems that you are using #2. What confuses me is how any cards are > >> able to assert CARD_DETECT. > > > > As far as I can see the Armada 388 SD host controller does not provide option > > #1. The Clearfog indeed uses option #2. Until commit da18c62b6e6a4 ("mmc: > > sdhci: Implement SDHCI card detect") it used to work because the mv_sdhci > > driver does not implement the get_cd callback, so card-detect was ignored. Now > > we have a get_cd implementation at the sdhci level that returns 0 in SPL > > because the GPIO is not accessible. > > > > What would you suggest? > > You can just add your own get_cd() callback instead of using sdhci_get_cd(). I can do that, but I would still hit the basic problem. GPIOs are not accessible from SPL when DM is enabled. I guess that would affect a few other platforms. How about making an exception for SPL? Something like this: diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 654931a82f54..03c132631d23 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -672,6 +672,9 @@ int sdhci_get_cd(struct udevice *dev) /* If polling, assume that the card is always present. */ if (mmc->cfg->host_caps & MMC_CAP_NEEDS_POLL) return 1; + /* In SPL we have no DM_GPIO access; assume card is present */ + if (IS_ENABLED(CONFIG_SPL_BUILD)) + return 1; #if CONFIG_IS_ENABLED(DM_GPIO) value = dm_gpio_get_value(&host->cd_gpio); What do you think? -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -