On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_ab...@ti.com> wrote: > > The 4 bit MMC controllers have an internal debounce for the SDCD line > with a debounce delay of 1 second. Therefore, after clocks to the IP are > enabled, software has to wait for this time before it can power on the > controller. > > Add an init() callback which polls on sdcd for a maximum of 2 seconds > before switching on power to the controller or (in the case of no card) > returning a ENOMEDIUM. This pushes the 1 second wait time to when the > card is actually needed rather than at every probe() making sure that > users who don't insert an SD card in the slot don't have to wait such a > long time. > > Signed-off-by: Faiz Abbas <faiz_ab...@ti.com> > Signed-off-by: Lokesh Vutla <lokeshvu...@ti.com> > --- > drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++--- > 1 file changed, 32 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c > index ff0a81eaab..ccae3fea31 100644 > --- a/drivers/mmc/am654_sdhci.c > +++ b/drivers/mmc/am654_sdhci.c > @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = { > .flags = IOMUX_PRESENT, > }; > > -int am654_sdhci_init(struct am654_sdhci_plat *plat) > +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat) > { > u32 ctl_cfg_2 = 0; > u32 mask, val; > @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice > *dev, > return 0; > } > > +#define MAX_SDCD_DEBOUNCE_TIME 2000 > +int am654_sdhci_init(struct udevice *dev) > +{ > + struct am654_sdhci_plat *plat = dev_get_platdata(dev); > + struct mmc *mmc = mmc_get_mmc_dev(dev); > + struct sdhci_host *host = mmc->priv; > + unsigned long start; > + int val; > + > + /* > + * The controller takes about 1 second to debounce the card detect > line > + * and doesn't let us power on until that time is up. Instead of > waiting > + * for 1 second at every stage, poll on the CARD_PRESENT bit upto a > + * maximum of 2 seconds to be safe.. > + */ > + start = get_timer(0); > + do { > + if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME) > + return -ENOMEDIUM; > + > + val = mmc_getcd(host->mmc); > + } while (!val); > + > + am654_sdhci_init_phy(plat); > + > + return sdhci_init(mmc); > +} > + > static int am654_sdhci_probe(struct udevice *dev) > { > + struct dm_mmc_ops *ops = mmc_get_ops(dev); > struct am654_driver_data *drv_data = > (struct am654_driver_data *)dev_get_driver_data(dev); > struct am654_sdhci_plat *plat = dev_get_platdata(dev); > @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev) > > regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1); > > - am654_sdhci_init(plat); > + ops->init = am654_sdhci_init;
Is this a valid approach? I mean, many drivers create their 'ops' const like this: static const struct ram_ops altera_gen5_sdram_ops (...) That would mean you write to a const region. I know the U-Boot sources make this easy for you by providing the ops non-const via mmc_get_ops, but I still think this is not good. Regards, Simon > > - return sdhci_probe(dev); > + return 0; > } > > static int am654_sdhci_ofdata_to_platdata(struct udevice *dev) > -- > 2.19.2 >