* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect
@ 2019-07-22 17:08 Baruch Siach
2019-07-23 8:00 ` Peng Fan
0 siblings, 1 reply; 10+ messages in thread
From: Baruch Siach @ 2019-07-22 17:08 UTC (permalink / raw)
To: u-boot
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 <t.karthik.reddy@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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));
if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH)
return !value;
else
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-22 17:08 [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect Baruch Siach @ 2019-07-23 8:00 ` Peng Fan 2019-07-23 8:57 ` Faiz Abbas 0 siblings, 1 reply; 10+ messages in thread From: Peng Fan @ 2019-07-23 8:00 UTC (permalink / raw) To: u-boot + 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 <t.karthik.reddy@xilinx.com> > Cc: Michal Simek <michal.simek@xilinx.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > 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? Thanks, Peng. > if (mmc->cfg->host_caps & MMC_CAP_CD_ACTIVE_HIGH) > return !value; > else > -- > 2.20.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 8:00 ` Peng Fan @ 2019-07-23 8:57 ` Faiz Abbas 2019-07-23 9:09 ` Baruch Siach 0 siblings, 1 reply; 10+ messages in thread From: Faiz Abbas @ 2019-07-23 8:57 UTC (permalink / raw) To: u-boot Hi, 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 <t.karthik.reddy@xilinx.com> >> Cc: Michal Simek <michal.simek@xilinx.com> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >> --- >> 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? Thanks, Faiz ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 8:57 ` Faiz Abbas @ 2019-07-23 9:09 ` Baruch Siach 2019-07-23 10:05 ` Faiz Abbas 0 siblings, 1 reply; 10+ messages in thread From: Baruch Siach @ 2019-07-23 9:09 UTC (permalink / raw) To: u-boot Hi Faiz, 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 <t.karthik.reddy@xilinx.com> > >> Cc: Michal Simek <michal.simek@xilinx.com> > >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > >> --- > >> 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? baruch -- 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 - ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 9:09 ` Baruch Siach @ 2019-07-23 10:05 ` Faiz Abbas 2019-07-23 10:46 ` Baruch Siach 0 siblings, 1 reply; 10+ messages in thread From: Faiz Abbas @ 2019-07-23 10:05 UTC (permalink / raw) To: u-boot Hi Baruch, On 23/07/19 2:39 PM, Baruch Siach wrote: > Hi Faiz, > > 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 <t.karthik.reddy@xilinx.com> >>>> Cc: Michal Simek <michal.simek@xilinx.com> >>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >>>> --- >>>> 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. Thanks, Faiz ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 10:05 ` Faiz Abbas @ 2019-07-23 10:46 ` Baruch Siach 2019-07-23 11:17 ` Faiz Abbas 0 siblings, 1 reply; 10+ messages in thread From: Baruch Siach @ 2019-07-23 10:46 UTC (permalink / raw) To: u-boot Hi Faiz, 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 <t.karthik.reddy@xilinx.com> > >>>> Cc: Michal Simek <michal.simek@xilinx.com> > >>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > >>>> --- > >>>> 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? baruch -- 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 - ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 10:46 ` Baruch Siach @ 2019-07-23 11:17 ` Faiz Abbas 2019-07-23 13:45 ` Baruch Siach 0 siblings, 1 reply; 10+ messages in thread From: Faiz Abbas @ 2019-07-23 11:17 UTC (permalink / raw) To: u-boot Hi Baruch, On 23/07/19 4:16 PM, Baruch Siach wrote: > Hi Faiz, > > 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 <t.karthik.reddy@xilinx.com> >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> >>>>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >>>>>> --- >>>>>> 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(). Thanks, Faiz ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 11:17 ` Faiz Abbas @ 2019-07-23 13:45 ` Baruch Siach 2019-07-24 1:17 ` Peng Fan 0 siblings, 1 reply; 10+ messages in thread From: Baruch Siach @ 2019-07-23 13:45 UTC (permalink / raw) To: u-boot 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 <t.karthik.reddy@xilinx.com> > >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> > >>>>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > >>>>>> --- > >>>>>> 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 - ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-23 13:45 ` Baruch Siach @ 2019-07-24 1:17 ` Peng Fan 2019-07-24 4:11 ` Baruch Siach 0 siblings, 1 reply; 10+ messages in thread From: Peng Fan @ 2019-07-24 1:17 UTC (permalink / raw) To: u-boot > Subject: Re: [PATCH] mmd: sdhci: fix non GPIO card detect > > 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 <t.karthik.reddy@xilinx.com> > > >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> > > >>>>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > >>>>>> --- > > >>>>>> 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 */ I think this assumption is wrong. It is not always true the DM_GPIO not enabled in SPL. Regards, Peng. > + 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? > > -- > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fbaruch. > siach.name%2Fblog%2F&data=02%7C01%7Cpeng.fan%40nxp.com%7C5 > 3b8baab6aff47e5c1ba08d70f73ff94%7C686ea1d3bc2b4c6fa92cd99c5c30163 > 5%7C0%7C0%7C636994863201960255&sdata=yB2JSsQelVqgidEChJrFijg > q1cUinIYdyR9uK3zwxM0%3D&reserved=0 ~. .~ > Tk Open Systems > =}------------------------------------------------ooO--U--Ooo------------{= > - baruch at tkos.co.il - tel: +972.2.679.5364, > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.tk > os.co.il&data=02%7C01%7Cpeng.fan%40nxp.com%7C53b8baab6aff47e > 5c1ba08d70f73ff94%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C636994863201960255&sdata=tKC8PWwGmWzKpDih3Sz5WU1U3i07dF > fkoIVaTFPC1ro%3D&reserved=0 - ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect 2019-07-24 1:17 ` Peng Fan @ 2019-07-24 4:11 ` Baruch Siach 0 siblings, 0 replies; 10+ messages in thread From: Baruch Siach @ 2019-07-24 4:11 UTC (permalink / raw) To: u-boot Hi Peng, On Wed, Jul 24, 2019 at 01:17:49AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH] mmd: sdhci: fix non GPIO card detect > > 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 <t.karthik.reddy@xilinx.com> > > > >>>>>> Cc: Michal Simek <michal.simek@xilinx.com> > > > >>>>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > > >>>>>> --- > > > >>>>>> 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 */ > > I think this assumption is wrong. It is not always true the DM_GPIO not > enabled in SPL. How can you enable DM_GPIO in SPL? The dm_gpio_get_value() code is under CONFIG_IS_ENABLED(DM_GPIO). The CONFIG_IS_ENABLED definition comment (include/linux/kconfig.h) says this: /* * CONFIG_IS_ENABLED(FOO) evaluates to * 1 if CONFIG_SPL_BUILD is undefined and CONFIG_FOO is set to 'y' or 'm', * 1 if CONFIG_SPL_BUILD is defined and CONFIG_SPL_FOO is set to 'y' or 'm', * 0 otherwise. */ There is no CONFIG_SPL_DM_GPIO in current U-Boot code as of commit ff8c23e784f5. So CONFIG_IS_ENABLED(DM_GPIO) will always evaluate as 0 in SPL. baruch > > + 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 - ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-24 4:11 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-22 17:08 [U-Boot] [PATCH] mmd: sdhci: fix non GPIO card detect Baruch Siach 2019-07-23 8:00 ` Peng Fan 2019-07-23 8:57 ` Faiz Abbas 2019-07-23 9:09 ` Baruch Siach 2019-07-23 10:05 ` Faiz Abbas 2019-07-23 10:46 ` Baruch Siach 2019-07-23 11:17 ` Faiz Abbas 2019-07-23 13:45 ` Baruch Siach 2019-07-24 1:17 ` Peng Fan 2019-07-24 4:11 ` Baruch Siach
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.