| Message ID | 20240522110909.10060-13-victorshihgli@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support UHS-II for GL9755 and GL9767 | expand |
On Fri, May 24, 2024 at 3:23 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 22/05/24 14:08, Victor Shih wrote: > > From: Victor Shih <victor.shih@genesyslogic.com.tw> > > > > 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> > > Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> > > --- > > > > Updates in V15: > > - Refer the SD Host Controller Standard Specification Section 3.10 > > to add reset command data mechanism. > > > > Updates in V14: > > - Since mmc_card_uhs2() is the same as sdhci_uhs2_mode(), so drop > > sdhci_uhs2_mode() and use mmc_card_uhs2() instead of sdhci_uhs2_mode(). > > > > Updates in V13: > > - Use ios timing to stead MMC_UHS2_SUPPORT for indicate the UHS2 mode. > > > > Updates in V8: > > - Adjust the position of matching brackets. > > > > Updates in V6: > > - Remove unnecessary functions and simplify code. > > > > --- > > > > drivers/mmc/host/sdhci-uhs2.c | 57 +++++++++++++++++++++++++++++++++++ > > drivers/mmc/host/sdhci-uhs2.h | 1 + > > drivers/mmc/host/sdhci.c | 3 +- > > drivers/mmc/host/sdhci.h | 1 + > > 4 files changed, 61 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > > index 9cb0f1b2a37d..7652158ea151 100644 > > --- a/drivers/mmc/host/sdhci-uhs2.c > > +++ b/drivers/mmc/host/sdhci-uhs2.c > > @@ -10,7 +10,9 @@ > > * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> > > */ > > > > +#include <linux/delay.h> > > #include <linux/module.h> > > +#include <linux/iopoll.h> > > > > #include "sdhci.h" > > #include "sdhci-uhs2.h" > > @@ -21,6 +23,8 @@ > > #define SDHCI_UHS2_DUMP(f, x...) \ > > pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x) > > > > +#define UHS2_RESET_TIMEOUT_100MS 100000 > > + > > void sdhci_uhs2_dump_regs(struct sdhci_host *host) > > { > > if (!(mmc_card_uhs2(host->mmc))) > > @@ -49,6 +53,57 @@ 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) > > +{ > > + u32 val; > > + > > + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); > > + > > + if (mask & SDHCI_UHS2_SW_RESET_FULL) > > + host->clock = 0; > > + > > + /* hw clears the bit when it's done */ > > + if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10, > > + UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) { > > + pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__, > > + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc)); > > + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); > > + return; > > + } > > +} > > +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); > > + > > +static void sdhci_uhs2_reset_cmd_data(struct mmc_host *mmc) > > +{ > > + struct sdhci_host *host = mmc_priv(mmc); > > + > > + sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA); > > + > > + if (host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A || > > + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A_HD || > > + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B || > > + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B_HD) { > > Please use mmc_card_uhs2() > Hi, Adrian I will update this in the v17 version. Thanks, Victor Shih > > + sdhci_uhs2_reset(host, SDHCI_UHS2_SW_RESET_SD); > > + > > + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); > > + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); > > + sdhci_uhs2_clear_set_irqs(host, SDHCI_INT_ALL_MASK, SDHCI_UHS2_INT_ERROR_MASK); > > + } > > +} > > + > > /*****************************************************************************\ > > * * > > * Driver init/exit * > > @@ -57,6 +112,8 @@ EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > > > > static int sdhci_uhs2_host_ops_init(struct sdhci_host *host) > > { > > + host->mmc_host_ops.uhs2_reset_cmd_data = sdhci_uhs2_reset_cmd_data; > > As noted for patch 8, any host controller resets needed > should be done before completing the request, so a call > back function should not be needed. > Hi, Adrian I want to confirm further, so can I call sdhci_uhs2_reset_cmd_data() directly in the mmc_wait_for_req_done()? Am I understanding something wrong? Thanks, Victor Shih > > + > > return 0; > > } > > > > diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h > > index 2bfe18d29bca..caaf9fba4975 100644 > > --- a/drivers/mmc/host/sdhci-uhs2.h > > +++ b/drivers/mmc/host/sdhci-uhs2.h > > @@ -177,5 +177,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 8fc3e001db74..f212da6dc2aa 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -236,7 +236,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask) > > } > > EXPORT_SYMBOL_GPL(sdhci_reset); > > > > -static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) > > +bool sdhci_do_reset(struct sdhci_host *host, u8 mask) > > { > > if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) { > > struct mmc_host *mmc = host->mmc; > > @@ -249,6 +249,7 @@ static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) > > > > return true; > > } > > +EXPORT_SYMBOL_GPL(sdhci_do_reset); > > > > static void sdhci_reset_for_all(struct sdhci_host *host) > > { > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > > index 13703f1a3710..83d994c8d89e 100644 > > --- a/drivers/mmc/host/sdhci.h > > +++ b/drivers/mmc/host/sdhci.h > > @@ -845,6 +845,7 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq); > > int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq); > > void sdhci_set_bus_width(struct sdhci_host *host, int width); > > void sdhci_reset(struct sdhci_host *host, u8 mask); > > +bool sdhci_do_reset(struct sdhci_host *host, u8 mask); > > void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing); > > int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode); > > int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode); >
On 31/05/24 13:32, Victor Shih wrote: > On Fri, May 24, 2024 at 3:23 PM Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 22/05/24 14:08, Victor Shih wrote: >>> From: Victor Shih <victor.shih@genesyslogic.com.tw> >>> >>> 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> >>> Signed-off-by: Victor Shih <victor.shih@genesyslogic.com.tw> >>> --- >>> >>> Updates in V15: >>> - Refer the SD Host Controller Standard Specification Section 3.10 >>> to add reset command data mechanism. >>> >>> Updates in V14: >>> - Since mmc_card_uhs2() is the same as sdhci_uhs2_mode(), so drop >>> sdhci_uhs2_mode() and use mmc_card_uhs2() instead of sdhci_uhs2_mode(). >>> >>> Updates in V13: >>> - Use ios timing to stead MMC_UHS2_SUPPORT for indicate the UHS2 mode. >>> >>> Updates in V8: >>> - Adjust the position of matching brackets. >>> >>> Updates in V6: >>> - Remove unnecessary functions and simplify code. >>> >>> --- >>> >>> drivers/mmc/host/sdhci-uhs2.c | 57 +++++++++++++++++++++++++++++++++++ >>> drivers/mmc/host/sdhci-uhs2.h | 1 + >>> drivers/mmc/host/sdhci.c | 3 +- >>> drivers/mmc/host/sdhci.h | 1 + >>> 4 files changed, 61 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >>> index 9cb0f1b2a37d..7652158ea151 100644 >>> --- a/drivers/mmc/host/sdhci-uhs2.c >>> +++ b/drivers/mmc/host/sdhci-uhs2.c >>> @@ -10,7 +10,9 @@ >>> * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> */ >>> >>> +#include <linux/delay.h> >>> #include <linux/module.h> >>> +#include <linux/iopoll.h> >>> >>> #include "sdhci.h" >>> #include "sdhci-uhs2.h" >>> @@ -21,6 +23,8 @@ >>> #define SDHCI_UHS2_DUMP(f, x...) \ >>> pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x) >>> >>> +#define UHS2_RESET_TIMEOUT_100MS 100000 >>> + >>> void sdhci_uhs2_dump_regs(struct sdhci_host *host) >>> { >>> if (!(mmc_card_uhs2(host->mmc))) >>> @@ -49,6 +53,57 @@ 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) >>> +{ >>> + u32 val; >>> + >>> + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); >>> + >>> + if (mask & SDHCI_UHS2_SW_RESET_FULL) >>> + host->clock = 0; >>> + >>> + /* hw clears the bit when it's done */ >>> + if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10, >>> + UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) { >>> + pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__, >>> + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc)); >>> + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); >>> + return; >>> + } >>> +} >>> +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); >>> + >>> +static void sdhci_uhs2_reset_cmd_data(struct mmc_host *mmc) >>> +{ >>> + struct sdhci_host *host = mmc_priv(mmc); >>> + >>> + sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA); >>> + >>> + if (host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A || >>> + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A_HD || >>> + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B || >>> + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B_HD) { >> >> Please use mmc_card_uhs2() >> > > Hi, Adrian > > I will update this in the v17 version. > > Thanks, Victor Shih > >>> + sdhci_uhs2_reset(host, SDHCI_UHS2_SW_RESET_SD); >>> + >>> + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); >>> + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); >>> + sdhci_uhs2_clear_set_irqs(host, SDHCI_INT_ALL_MASK, SDHCI_UHS2_INT_ERROR_MASK); >>> + } >>> +} >>> + >>> /*****************************************************************************\ >>> * * >>> * Driver init/exit * >>> @@ -57,6 +112,8 @@ EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); >>> >>> static int sdhci_uhs2_host_ops_init(struct sdhci_host *host) >>> { >>> + host->mmc_host_ops.uhs2_reset_cmd_data = sdhci_uhs2_reset_cmd_data; >> >> As noted for patch 8, any host controller resets needed >> should be done before completing the request, so a call >> back function should not be needed. >> > > Hi, Adrian > > I want to confirm further, so can I call > sdhci_uhs2_reset_cmd_data() directly > in the mmc_wait_for_req_done()? No > Am I understanding something wrong? See the response to your questions over patch 8. > > Thanks, Victor Shih > >>> + >>> return 0; >>> } >>> >>> diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h >>> index 2bfe18d29bca..caaf9fba4975 100644 >>> --- a/drivers/mmc/host/sdhci-uhs2.h >>> +++ b/drivers/mmc/host/sdhci-uhs2.h >>> @@ -177,5 +177,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 8fc3e001db74..f212da6dc2aa 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -236,7 +236,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask) >>> } >>> EXPORT_SYMBOL_GPL(sdhci_reset); >>> >>> -static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) >>> +bool sdhci_do_reset(struct sdhci_host *host, u8 mask) >>> { >>> if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) { >>> struct mmc_host *mmc = host->mmc; >>> @@ -249,6 +249,7 @@ static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) >>> >>> return true; >>> } >>> +EXPORT_SYMBOL_GPL(sdhci_do_reset); >>> >>> static void sdhci_reset_for_all(struct sdhci_host *host) >>> { >>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >>> index 13703f1a3710..83d994c8d89e 100644 >>> --- a/drivers/mmc/host/sdhci.h >>> +++ b/drivers/mmc/host/sdhci.h >>> @@ -845,6 +845,7 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq); >>> int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq); >>> void sdhci_set_bus_width(struct sdhci_host *host, int width); >>> void sdhci_reset(struct sdhci_host *host, u8 mask); >>> +bool sdhci_do_reset(struct sdhci_host *host, u8 mask); >>> void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing); >>> int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode); >>> int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode); >>
diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c index 9cb0f1b2a37d..7652158ea151 100644 --- a/drivers/mmc/host/sdhci-uhs2.c +++ b/drivers/mmc/host/sdhci-uhs2.c @@ -10,7 +10,9 @@ * Author: AKASHI Takahiro <takahiro.akashi@linaro.org> */ +#include <linux/delay.h> #include <linux/module.h> +#include <linux/iopoll.h> #include "sdhci.h" #include "sdhci-uhs2.h" @@ -21,6 +23,8 @@ #define SDHCI_UHS2_DUMP(f, x...) \ pr_err("%s: " DRIVER_NAME ": " f, mmc_hostname(host->mmc), ## x) +#define UHS2_RESET_TIMEOUT_100MS 100000 + void sdhci_uhs2_dump_regs(struct sdhci_host *host) { if (!(mmc_card_uhs2(host->mmc))) @@ -49,6 +53,57 @@ 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) +{ + u32 val; + + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); + + if (mask & SDHCI_UHS2_SW_RESET_FULL) + host->clock = 0; + + /* hw clears the bit when it's done */ + if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10, + UHS2_RESET_TIMEOUT_100MS, true, host, SDHCI_UHS2_SW_RESET)) { + pr_warn("%s: %s: Reset 0x%x never completed. %s: clean reset bit.\n", __func__, + mmc_hostname(host->mmc), (int)mask, mmc_hostname(host->mmc)); + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); + return; + } +} +EXPORT_SYMBOL_GPL(sdhci_uhs2_reset); + +static void sdhci_uhs2_reset_cmd_data(struct mmc_host *mmc) +{ + struct sdhci_host *host = mmc_priv(mmc); + + sdhci_do_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA); + + if (host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A || + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_A_HD || + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B || + host->mmc->ios.timing == MMC_TIMING_UHS2_SPEED_B_HD) { + sdhci_uhs2_reset(host, SDHCI_UHS2_SW_RESET_SD); + + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); + sdhci_uhs2_clear_set_irqs(host, SDHCI_INT_ALL_MASK, SDHCI_UHS2_INT_ERROR_MASK); + } +} + /*****************************************************************************\ * * * Driver init/exit * @@ -57,6 +112,8 @@ EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); static int sdhci_uhs2_host_ops_init(struct sdhci_host *host) { + host->mmc_host_ops.uhs2_reset_cmd_data = sdhci_uhs2_reset_cmd_data; + return 0; } diff --git a/drivers/mmc/host/sdhci-uhs2.h b/drivers/mmc/host/sdhci-uhs2.h index 2bfe18d29bca..caaf9fba4975 100644 --- a/drivers/mmc/host/sdhci-uhs2.h +++ b/drivers/mmc/host/sdhci-uhs2.h @@ -177,5 +177,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 8fc3e001db74..f212da6dc2aa 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -236,7 +236,7 @@ void sdhci_reset(struct sdhci_host *host, u8 mask) } EXPORT_SYMBOL_GPL(sdhci_reset); -static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) +bool sdhci_do_reset(struct sdhci_host *host, u8 mask) { if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) { struct mmc_host *mmc = host->mmc; @@ -249,6 +249,7 @@ static bool sdhci_do_reset(struct sdhci_host *host, u8 mask) return true; } +EXPORT_SYMBOL_GPL(sdhci_do_reset); static void sdhci_reset_for_all(struct sdhci_host *host) { diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 13703f1a3710..83d994c8d89e 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -845,6 +845,7 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq); int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq); void sdhci_set_bus_width(struct sdhci_host *host, int width); void sdhci_reset(struct sdhci_host *host, u8 mask); +bool sdhci_do_reset(struct sdhci_host *host, u8 mask); void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing); int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode); int __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode);