| Message ID | 20201106022726.19831-13-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add support UHS-II for GL9755 | expand |
On 6/11/20 4:27 am, AKASHI Takahiro wrote: > Sdhci_uhs2_reset() does a UHS-II specific reset operation. > > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > drivers/mmc/host/sdhci-uhs2.c | 49 +++++++++++++++++++++++++++++++++++ > drivers/mmc/host/sdhci-uhs2.h | 1 + > drivers/mmc/host/sdhci.c | 3 ++- > drivers/mmc/host/sdhci.h | 1 + > 4 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > index 08905ed081fb..e2b9743fe17d 100644 > --- a/drivers/mmc/host/sdhci-uhs2.c > +++ b/drivers/mmc/host/sdhci-uhs2.c > @@ -10,6 +10,7 @@ > * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> > */ > > +#include <linux/delay.h> > #include <linux/module.h> > > #include "sdhci.h" > @@ -49,6 +50,54 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > } > EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > > +/*****************************************************************************\ > + * * > + * Low level functions * > + * * > +\*****************************************************************************/ > + > +/** > + * sdhci_uhs2_reset - invoke SW reset > + * @host: SDHCI host > + * @mask: Control mask > + * > + * Invoke SW reset, depending on a bit in @mask and wait for completion. > + */ > +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask) > +{ > + unsigned long timeout; > + > + if (!(host->mmc->caps & MMC_CAP_UHS2)) Please make a helper so this can be like: if (!sdhci_uhs2_mode(host)) The capability will be always present for hosts that support UHS2, but not all cards support UHS2 mode. I suggest just adding a bool to struct sdhci_host to keep track of when the host is in UHS2 mode. > + return; > + > + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); > + > + if (mask & SDHCI_UHS2_SW_RESET_FULL) { > + host->clock = 0; > + /* Reset-all turns off SD Bus Power */ > + if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON) I would prefer not to support any legacy quirks that we do not need right now. Just be sure to add a comment somewhere listing which quirks are not supported for UHS2 host controllers. > + sdhci_runtime_pm_bus_off(host); > + } > + > + /* Wait max 100 ms */ > + timeout = 10000; > + > + /* hw clears the bit when it's done */ > + while (sdhci_readw(host, SDHCI_UHS2_SW_RESET) & mask) { This kind of loop can now be done with read_poll_timeout_atomic(sdhci_readw, ..., host, SDHCI_UHS2_SW_RESET) > + if (timeout == 0) { > + pr_err("%s: %s: Reset 0x%x never completed.\n", > + __func__, mmc_hostname(host->mmc), (int)mask); > + pr_err("%s: clean reset bit\n", > + mmc_hostname(host->mmc)); > + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); > + return; > + } > + timeout--; > + udelay(10); > + } > +} > +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); > + > /*****************************************************************************\ > * * > * Driver init/exit * > diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h > index b9529d32b58d..7bb7a0d67109 100644 > --- a/drivers/mmc/host/sdhci-uhs2.h > +++ b/drivers/mmc/host/sdhci-uhs2.h > @@ -210,5 +210,6 @@ > struct sdhci_host; > > void sdhci_uhs2_dump_regs(struct sdhci_host *host); > +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask); > > #endif /* __SDHCI_UHS2_H */ > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index d4a57e8c9bb8..af336bdb4305 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -195,13 +195,14 @@ static void sdhci_runtime_pm_bus_on(struct sdhci_host *host) > pm_runtime_get_noresume(host->mmc->parent); > } > > -static void sdhci_runtime_pm_bus_off(struct sdhci_host *host) > +void sdhci_runtime_pm_bus_off(struct sdhci_host *host) > { > if (!host->bus_on) > return; > host->bus_on = false; > pm_runtime_put_noidle(host->mmc->parent); > } > +EXPORT_SYMBOL_GPL(sdhci_runtime_pm_bus_off); > > void sdhci_reset(struct sdhci_host *host, u8 mask) > { > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index d9d7a76cedc1..b9932423db08 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -831,6 +831,7 @@ static inline void sdhci_read_caps(struct sdhci_host *host) > __sdhci_read_caps(host, NULL, NULL, NULL); > } > > +void sdhci_runtime_pm_bus_off(struct sdhci_host *host); > u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock, > unsigned int *actual_clock); > void sdhci_set_clock(struct sdhci_host *host, unsigned int clock); >
On Thu, Nov 26, 2020 at 10:16:11AM +0200, Adrian Hunter wrote: > On 6/11/20 4:27 am, AKASHI Takahiro wrote: > > Sdhci_uhs2_reset() does a UHS-II specific reset operation. > > > > Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > drivers/mmc/host/sdhci-uhs2.c | 49 +++++++++++++++++++++++++++++++++++ > > drivers/mmc/host/sdhci-uhs2.h | 1 + > > drivers/mmc/host/sdhci.c | 3 ++- > > drivers/mmc/host/sdhci.h | 1 + > > 4 files changed, 53 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > > index 08905ed081fb..e2b9743fe17d 100644 > > --- a/drivers/mmc/host/sdhci-uhs2.c > > +++ b/drivers/mmc/host/sdhci-uhs2.c > > @@ -10,6 +10,7 @@ > > * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> > > */ > > > > +#include <linux/delay.h> > > #include <linux/module.h> > > > > #include "sdhci.h" > > @@ -49,6 +50,54 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > > } > > EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > > > > +/*****************************************************************************\ > > + * * > > + * Low level functions * > > + * * > > +\*****************************************************************************/ > > + > > +/** > > + * sdhci_uhs2_reset - invoke SW reset > > + * @host: SDHCI host > > + * @mask: Control mask > > + * > > + * Invoke SW reset, depending on a bit in @mask and wait for completion. > > + */ > > +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask) > > +{ > > + unsigned long timeout; > > + > > + if (!(host->mmc->caps & MMC_CAP_UHS2)) > > Please make a helper so this can be like: > > if (!sdhci_uhs2_mode(host)) > > The capability will be always present for hosts that support UHS2, but not > all cards support UHS2 mode. I suggest just adding a bool to struct > sdhci_host to keep track of when the host is in UHS2 mode. Given the fact that UHS-2 host may (potentially) support a topology like a ring, this kind of status should be attributed to a card (structure) rather than a host. I'm asking Ben to modify the way how the current mode be managed in both 'core' side and 'host' side. That is why I marked "TODO" in many places to check the mode. So I'd defer the change until his rework be done. > > + return; > > + > > + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); > > + > > + if (mask & SDHCI_UHS2_SW_RESET_FULL) { > > + host->clock = 0; > > + /* Reset-all turns off SD Bus Power */ > > + if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON) > > I would prefer not to support any legacy quirks that we do not need right > now. Just be sure to add a comment somewhere listing which quirks are not > supported for UHS2 host controllers. No strong opinion. I'd defer to you. > > + sdhci_runtime_pm_bus_off(host); > > + } > > + > > + /* Wait max 100 ms */ > > + timeout = 10000; > > + > > + /* hw clears the bit when it's done */ > > + while (sdhci_readw(host, SDHCI_UHS2_SW_RESET) & mask) { > > This kind of loop can now be done with read_poll_timeout_atomic(sdhci_readw, > ..., host, SDHCI_UHS2_SW_RESET) Okay. -Takahiro Akashi > > + if (timeout == 0) { > > + pr_err("%s: %s: Reset 0x%x never completed.\n", > > + __func__, mmc_hostname(host->mmc), (int)mask); > > + pr_err("%s: clean reset bit\n", > > + mmc_hostname(host->mmc)); > > + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); > > + return; > > + } > > + timeout--; > > + udelay(10); > > + } > > +} > > +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); > > + > > /*****************************************************************************\ > > * * > > * Driver init/exit * > > diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h > > index b9529d32b58d..7bb7a0d67109 100644 > > --- a/drivers/mmc/host/sdhci-uhs2.h > > +++ b/drivers/mmc/host/sdhci-uhs2.h > > @@ -210,5 +210,6 @@ > > struct sdhci_host; > > > > void sdhci_uhs2_dump_regs(struct sdhci_host *host); > > +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask); > > > > #endif /* __SDHCI_UHS2_H */ > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index d4a57e8c9bb8..af336bdb4305 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -195,13 +195,14 @@ static void sdhci_runtime_pm_bus_on(struct sdhci_host *host) > > pm_runtime_get_noresume(host->mmc->parent); > > } > > > > -static void sdhci_runtime_pm_bus_off(struct sdhci_host *host) > > +void sdhci_runtime_pm_bus_off(struct sdhci_host *host) > > { > > if (!host->bus_on) > > return; > > host->bus_on = false; > > pm_runtime_put_noidle(host->mmc->parent); > > } > > +EXPORT_SYMBOL_GPL(sdhci_runtime_pm_bus_off); > > > > void sdhci_reset(struct sdhci_host *host, u8 mask) > > { > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > > index d9d7a76cedc1..b9932423db08 100644 > > --- a/drivers/mmc/host/sdhci.h > > +++ b/drivers/mmc/host/sdhci.h > > @@ -831,6 +831,7 @@ static inline void sdhci_read_caps(struct sdhci_host *host) > > __sdhci_read_caps(host, NULL, NULL, NULL); > > } > > > > +void sdhci_runtime_pm_bus_off(struct sdhci_host *host); > > u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock, > > unsigned int *actual_clock); > > void sdhci_set_clock(struct sdhci_host *host, unsigned int clock); > > >
On 30/11/20 8:20 am, AKASHI Takahiro wrote: > On Thu, Nov 26, 2020 at 10:16:11AM +0200, Adrian Hunter wrote: >> On 6/11/20 4:27 am, AKASHI Takahiro wrote: >>> Sdhci_uhs2_reset() does a UHS-II specific reset operation. >>> >>> Signed-off-by: Ben Chuang <ben.chuang@genesyslogic.com.tw> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> drivers/mmc/host/sdhci-uhs2.c | 49 +++++++++++++++++++++++++++++++++++ >>> drivers/mmc/host/sdhci-uhs2.h | 1 + >>> drivers/mmc/host/sdhci.c | 3 ++- >>> drivers/mmc/host/sdhci.h | 1 + >>> 4 files changed, 53 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >>> index 08905ed081fb..e2b9743fe17d 100644 >>> --- a/drivers/mmc/host/sdhci-uhs2.c >>> +++ b/drivers/mmc/host/sdhci-uhs2.c >>> @@ -10,6 +10,7 @@ >>> * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> */ >>> >>> +#include <linux/delay.h> >>> #include <linux/module.h> >>> >>> #include "sdhci.h" >>> @@ -49,6 +50,54 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) >>> } >>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); >>> >>> +/*****************************************************************************\ >>> + * * >>> + * Low level functions * >>> + * * >>> +\*****************************************************************************/ >>> + >>> +/** >>> + * sdhci_uhs2_reset - invoke SW reset >>> + * @host: SDHCI host >>> + * @mask: Control mask >>> + * >>> + * Invoke SW reset, depending on a bit in @mask and wait for completion. >>> + */ >>> +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask) >>> +{ >>> + unsigned long timeout; >>> + >>> + if (!(host->mmc->caps & MMC_CAP_UHS2)) >> >> Please make a helper so this can be like: >> >> if (!sdhci_uhs2_mode(host)) >> >> The capability will be always present for hosts that support UHS2, but not >> all cards support UHS2 mode. I suggest just adding a bool to struct >> sdhci_host to keep track of when the host is in UHS2 mode. > > Given the fact that UHS-2 host may (potentially) support a topology like > a ring, this kind of status should be attributed to a card (structure) > rather than a host. It is very unlikely we would ever need to support that, so don't let it make things more complicated.
diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c index 08905ed081fb..e2b9743fe17d 100644 --- a/drivers/mmc/host/sdhci-uhs2.c +++ b/drivers/mmc/host/sdhci-uhs2.c @@ -10,6 +10,7 @@ * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> */ +#include <linux/delay.h> #include <linux/module.h> #include "sdhci.h" @@ -49,6 +50,54 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) } EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); +/*****************************************************************************\ + * * + * Low level functions * + * * +\*****************************************************************************/ + +/** + * sdhci_uhs2_reset - invoke SW reset + * @host: SDHCI host + * @mask: Control mask + * + * Invoke SW reset, depending on a bit in @mask and wait for completion. + */ +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask) +{ + unsigned long timeout; + + if (!(host->mmc->caps & MMC_CAP_UHS2)) + return; + + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); + + if (mask & SDHCI_UHS2_SW_RESET_FULL) { + host->clock = 0; + /* Reset-all turns off SD Bus Power */ + if (host->quirks2 & SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON) + sdhci_runtime_pm_bus_off(host); + } + + /* Wait max 100 ms */ + timeout = 10000; + + /* hw clears the bit when it's done */ + while (sdhci_readw(host, SDHCI_UHS2_SW_RESET) & mask) { + if (timeout == 0) { + pr_err("%s: %s: Reset 0x%x never completed.\n", + __func__, mmc_hostname(host->mmc), (int)mask); + pr_err("%s: clean reset bit\n", + mmc_hostname(host->mmc)); + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); + return; + } + timeout--; + udelay(10); + } +} +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); + /*****************************************************************************\ * * * Driver init/exit * diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h index b9529d32b58d..7bb7a0d67109 100644 --- a/drivers/mmc/host/sdhci-uhs2.h +++ b/drivers/mmc/host/sdhci-uhs2.h @@ -210,5 +210,6 @@ struct sdhci_host; void sdhci_uhs2_dump_regs(struct sdhci_host *host); +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask); #endif /* __SDHCI_UHS2_H */ diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index d4a57e8c9bb8..af336bdb4305 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -195,13 +195,14 @@ static void sdhci_runtime_pm_bus_on(struct sdhci_host *host) pm_runtime_get_noresume(host->mmc->parent); } -static void sdhci_runtime_pm_bus_off(struct sdhci_host *host) +void sdhci_runtime_pm_bus_off(struct sdhci_host *host) { if (!host->bus_on) return; host->bus_on = false; pm_runtime_put_noidle(host->mmc->parent); } +EXPORT_SYMBOL_GPL(sdhci_runtime_pm_bus_off); void sdhci_reset(struct sdhci_host *host, u8 mask) { diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index d9d7a76cedc1..b9932423db08 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -831,6 +831,7 @@ static inline void sdhci_read_caps(struct sdhci_host *host) __sdhci_read_caps(host, NULL, NULL, NULL); } +void sdhci_runtime_pm_bus_off(struct sdhci_host *host); u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock, unsigned int *actual_clock); void sdhci_set_clock(struct sdhci_host *host, unsigned int clock);