[RFC PATCH] mmc: zynq_sdhci: Dependable card detect
Soma, Ashok Reddy
ashok.reddy.soma at amd.com
Tue Jun 6 10:20:50 CEST 2023
Hi Stefan,
> -----Original Message-----
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-
> oss at weidmueller.com>
> Sent: Monday, May 22, 2023 1:26 PM
> To: Soma, Ashok Reddy <ashok.reddy.soma at amd.com>; u-
> boot at lists.denx.de
> Cc: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>;
> Jaehoon Chung <jh80.chung at samsung.com>; Simek, Michal
> <michal.simek at amd.com>; Peng Fan <peng.fan at nxp.com>; Potthuri, Sai
> Krishna <sai.krishna.potthuri at amd.com>
> Subject: Re: [RFC PATCH] mmc: zynq_sdhci: Dependable card detect
>
> Hi Ashok,
>
> Am 18.05.2023 um 08:20 schrieb Soma, Ashok Reddy:
>
> > We haven't seen any instability issue so far on ZynqMP boards and no one
> reported any such issue till now.
>
> It isn't an instability issue. The high level at the card detect input is not
> marked as stable until a short low level is detected.
>
> > We are already taking care of card detection stability issue with below
> waiting logic.
> > https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/mmc/zynq_sd
> > hci.c#L1161
>
> This code makes problems for us. The SDHCI_CARD_STATE_STABLE signal isn't
> set without a card and we see a "Sdhci card detect state not stable" message.
>
> > Could you please answer below questions.
> > 1. is this observed in SPL only or full u-boot ?
>
> Both. We have enable environment support in SPL and see the "Sdhci card
> detect state not stable" message after "Loading Environment from MMC..."
> in SPL and "MMC: " in full U-Boot.
>
> > 2. is this observed in dynamic config case only ? because your patch is in
> dynamic path.
>
> No. I have override the return value of the
> zynqmp_pm_is_function_supported function and see the same error. Without
> dynamic case I don't see a possibility to set the SD_CONFIG_EMMC_SEL
> value.
>
> > 3. "On our hardware we get a "Sdhci card detect state not stable" error in
> the SPL if no mmc card is present"
> > Are you seeing stability issue when card is not present ?
>
> No, the SDHCI_CARD_STATE_STABLE signal is never set.
Surprised that card card state stable signal is never set. But we cannot go with the workaround in this way.
>
> > can you please elaborate more about the issue.
>
> I have change the code to monitor the SDHCI_PRESENT_STATE signal.
> Without a card at boot the SDHCI_CARD_STATE_STABLE signal is unset.
> After card insert the SDHCI_CARD_PRESENT signal and the
> SDHCI_CARD_STATE_STABLE signal is set after some time. After card remove
> the SDHCI_CARD_STATE_STABLE signal is set and the SDHCI_CARD_PRESENT
> signal is unset after some time. It doesn't matter if a physical card is insert or
> the SD_CONFIG_EMMC_SEL config is used to emulate a present card.
> Without a short card present signal on the card detect logic the
> SDHCI_CARD_STATE_STABLE signal isn't set.
>
> > 4. It is not convincing to me that you are trying to select eMMC for some
> time and then switching back to SD.
>
> I don't find a documentation what the consequence of this config is but this
> config is set based on the value of "non-removable" and setting this config
> with enabled clock leads to a functional SDHCI_CARD_STATE_STABLE signal
> even after unset of the config.
>
> Maybe the SD_CDn_CTRL register have the same result but this isn't available
> via PMU.
Please try any other method and lets see, but changing to eMMC mode and back to SDHC is NOT acceptable according to me.
Thanks,
Ashok
>
> Regards
> Stefan
>
> >> -----Original Message-----
> >> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Stefan
> >> Herbrechtsmeier
> >> Sent: Tuesday, May 16, 2023 7:51 PM
> >> To: u-boot at lists.denx.de
> >> Cc: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>;
> >> Jaehoon Chung <jh80.chung at samsung.com>; Simek, Michal
> >> <michal.simek at amd.com>; Peng Fan <peng.fan at nxp.com>
> >> Subject: [RFC PATCH] mmc: zynq_sdhci: Dependable card detect
> >>
> >> CAUTION: This message has originated from an External Source. Please
> >> use proper judgment and caution when opening attachments, clicking
> >> links, or responding to this email.
> >>
> >>
> >> From: Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier at weidmueller.com>
> >>
> >> The card detect logic needs a short card present signal to work
> dependable.
> >> Without a present card the SDHCI_CARD_STATE_STABLE signal is not set
> >> dependable after a reset. Use the internal fixed card present signal
> >> to initiate the card detect logic.
> >>
> >> Signed-off-by: Stefan Herbrechtsmeier
> >> <stefan.herbrechtsmeier at weidmueller.com>
> >>
> >> ---
> >> On our hardware we get a "Sdhci card detect state not stable" error
> >> in the SPL if no mmc card is present. It is unclear if this patch is
> >> the correct solution, but a short card inserts or a fixed card
> >> present signal leads to a SDHCI_CARD_STATE_STABLE signal with and
> without card.
> >>
> >> drivers/mmc/zynq_sdhci.c | 20 ++++++++++++++++++++
> >> 1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> >> index
> >> e44868aaec..a88feeb367 100644
> >> --- a/drivers/mmc/zynq_sdhci.c
> >> +++ b/drivers/mmc/zynq_sdhci.c
> >> @@ -1075,6 +1075,26 @@ static int
> >> sdhci_zynqmp_set_dynamic_config(struct arasan_sdhci_priv *priv,
> >> return ret;
> >> }
> >>
> >> + /* The card detect logic needs a short card present signal to work
> >> + * dependable. Without a present card the
> SDHCI_CARD_STATE_STABLE
> >> + * signal is not set dependable after a reset. Use the internal
> >> + * fixed card present signal to initiate the card detect logic.
> >> + */
> >> + if (!dev_read_bool(dev, "non-removable")) {
> >> + ret = zynqmp_pm_set_sd_config(priv->node_id,
> >> SD_CONFIG_EMMC_SEL,
> >> + 1);
> >> + if (ret) {
> >> + dev_err(dev, "SD_CONFIG_EMMC_SEL failed\n");
> >> + return ret;
> >> + }
> >> + ret = zynqmp_pm_set_sd_config(priv->node_id,
> >> SD_CONFIG_EMMC_SEL,
> >> + 0);
> >> + if (ret) {
> >> + dev_err(dev, "SD_CONFIG_EMMC_SEL failed\n");
> >> + return ret;
> >> + }
> >> + }
> >> +
> >> return 0;
> >> }
> >> #endif
> >> --
> >> 2.30.2
More information about the U-Boot
mailing list