| Message ID | 20230915094351.11120-11-victorshihgli@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support UHS-II for GL9755 | expand |
On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> > Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > > Updates in V8: > - Adjust the position of matching brackets. > > Updates in V6: > - Remove unnecessary functions and simplify code. > > --- > > drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ > drivers/mmc/host/sdhci-uhs2.h | 2 ++ > 2 files changed, 47 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > index e339821d3504..dfc80a7f1bad 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" > @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > } > EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > > +/*****************************************************************************\ > + * * > + * Low level functions * > + * * > +\*****************************************************************************/ > + > +bool sdhci_uhs2_mode(struct sdhci_host *host) > +{ > + return host->mmc->flags & MMC_UHS2_SUPPORT; The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we should be using mmc->ios.timings, which already indicates whether we are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added this. That said, I think we should drop the sdhci_uhs2_mode() function altogether and instead use mmc_card_uhs2(), which means we should move it to include/linux/mmc/host.h, so it becomes available for host drivers. > +} > + > +/** > + * 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; > + u32 val; > + > + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); > + > + if (mask & SDHCI_UHS2_SW_RESET_FULL) > + host->clock = 0; > + > + /* Wait max 100 ms */ > + timeout = 100000; Please convert into using a define and use that directly, below instead. > + > + /* hw clears the bit when it's done */ > + if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10, > + timeout, true, host, SDHCI_UHS2_SW_RESET)) { > + 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)); It looks silly to do two pr_err immediately after each other, please combine them into one. Moreover, I think we should probably convert into using a pr_warn() instead. > + sdhci_writeb(host, 0, SDHCI_UHS2_SW_RESET); > + return; > + } > +} > +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 2bfe18d29bca..8253d50f7852 100644 > --- a/drivers/mmc/host/sdhci-uhs2.h > +++ b/drivers/mmc/host/sdhci-uhs2.h > @@ -177,5 +177,7 @@ > struct sdhci_host; > > void sdhci_uhs2_dump_regs(struct sdhci_host *host); > +bool sdhci_uhs2_mode(struct sdhci_host *host); > +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask); > > #endif /* __SDHCI_UHS2_H */ > -- > 2.25.1 > Kind regards Uffe
On 3/10/23 13:30, Ulf Hansson wrote: > On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> >> Acked-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> >> Updates in V8: >> - Adjust the position of matching brackets. >> >> Updates in V6: >> - Remove unnecessary functions and simplify code. >> >> --- >> >> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ >> drivers/mmc/host/sdhci-uhs2.h | 2 ++ >> 2 files changed, 47 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >> index e339821d3504..dfc80a7f1bad 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" >> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) >> } >> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); >> >> +/*****************************************************************************\ >> + * * >> + * Low level functions * >> + * * >> +\*****************************************************************************/ >> + >> +bool sdhci_uhs2_mode(struct sdhci_host *host) >> +{ >> + return host->mmc->flags & MMC_UHS2_SUPPORT; > > The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we > should be using mmc->ios.timings, which already indicates whether we > are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added > this. > > That said, I think we should drop the sdhci_uhs2_mode() function > altogether and instead use mmc_card_uhs2(), which means we should move > it to include/linux/mmc/host.h, so it becomes available for host > drivers. > UHS2 mode starts at UHS2 initialization and ends either when UHS2 initialization fails, or the card is removed. So it includes re-initialization and reset when the transfer mode currently transitions through MMC_TIMING_LEGACY. So mmc_card_uhs2() won't work correctly for the host callbacks unless something is done about that.
On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 3/10/23 13:30, Ulf Hansson wrote: > > On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> > >> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > >> --- > >> > >> Updates in V8: > >> - Adjust the position of matching brackets. > >> > >> Updates in V6: > >> - Remove unnecessary functions and simplify code. > >> > >> --- > >> > >> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ > >> drivers/mmc/host/sdhci-uhs2.h | 2 ++ > >> 2 files changed, 47 insertions(+) > >> > >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > >> index e339821d3504..dfc80a7f1bad 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" > >> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > >> } > >> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > >> > >> +/*****************************************************************************\ > >> + * * > >> + * Low level functions * > >> + * * > >> +\*****************************************************************************/ > >> + > >> +bool sdhci_uhs2_mode(struct sdhci_host *host) > >> +{ > >> + return host->mmc->flags & MMC_UHS2_SUPPORT; > > > > The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we > > should be using mmc->ios.timings, which already indicates whether we > > are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added > > this. > > > > That said, I think we should drop the sdhci_uhs2_mode() function > > altogether and instead use mmc_card_uhs2(), which means we should move > > it to include/linux/mmc/host.h, so it becomes available for host > > drivers. > > > > UHS2 mode starts at UHS2 initialization and ends either when UHS2 > initialization fails, or the card is removed. > > So it includes re-initialization and reset when the transfer mode > currently transitions through MMC_TIMING_LEGACY. > > So mmc_card_uhs2() won't work correctly for the host callbacks > unless something is done about that. Right, thanks for clarifying! In that case I wonder if we couldn't change the way we update the ->ios.timing for UHS2. It seems silly to have two (similar) ways to indicate that we have moved to UHS2. Kind regards Uffe
On 3/10/23 15:22, Ulf Hansson wrote: > On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 3/10/23 13:30, Ulf Hansson wrote: >>> On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> >>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com> >>>> --- >>>> >>>> Updates in V8: >>>> - Adjust the position of matching brackets. >>>> >>>> Updates in V6: >>>> - Remove unnecessary functions and simplify code. >>>> >>>> --- >>>> >>>> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ >>>> drivers/mmc/host/sdhci-uhs2.h | 2 ++ >>>> 2 files changed, 47 insertions(+) >>>> >>>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >>>> index e339821d3504..dfc80a7f1bad 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" >>>> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) >>>> } >>>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); >>>> >>>> +/*****************************************************************************\ >>>> + * * >>>> + * Low level functions * >>>> + * * >>>> +\*****************************************************************************/ >>>> + >>>> +bool sdhci_uhs2_mode(struct sdhci_host *host) >>>> +{ >>>> + return host->mmc->flags & MMC_UHS2_SUPPORT; >>> >>> The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we >>> should be using mmc->ios.timings, which already indicates whether we >>> are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added >>> this. >>> >>> That said, I think we should drop the sdhci_uhs2_mode() function >>> altogether and instead use mmc_card_uhs2(), which means we should move >>> it to include/linux/mmc/host.h, so it becomes available for host >>> drivers. >>> >> >> UHS2 mode starts at UHS2 initialization and ends either when UHS2 >> initialization fails, or the card is removed. >> >> So it includes re-initialization and reset when the transfer mode >> currently transitions through MMC_TIMING_LEGACY. >> >> So mmc_card_uhs2() won't work correctly for the host callbacks >> unless something is done about that. > > Right, thanks for clarifying! > > In that case I wonder if we couldn't change the way we update the > ->ios.timing for UHS2. It seems silly to have two (similar) ways to > indicate that we have moved to UHS2. Perhaps something like below: diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c index aacefdd6bc9e..e39d63d46041 100644 --- a/drivers/mmc/core/sd_uhs2.c +++ b/drivers/mmc/core/sd_uhs2.c @@ -70,7 +70,8 @@ static int sd_uhs2_power_off(struct mmc_host *host) host->ios.vdd = 0; host->ios.clock = 0; - host->ios.timing = MMC_TIMING_LEGACY; + /* Must set UHS2 timing to identify UHS2 mode */ + host->ios.timing = MMC_TIMING_UHS2_SPEED_A; host->ios.power_mode = MMC_POWER_OFF; if (host->flags & MMC_UHS2_SD_TRAN) host->flags &= ~MMC_UHS2_SD_TRAN; @@ -1095,7 +1096,8 @@ static void sd_uhs2_detect(struct mmc_host *host) mmc_claim_host(host); mmc_detach_bus(host); sd_uhs2_power_off(host); - host->flags &= ~MMC_UHS2_SUPPORT; + /* Remove UHS2 timing to indicate the end of UHS2 mode */ + host->ios.timing = MMC_TIMING_LEGACY; mmc_release_host(host); } } @@ -1338,7 +1340,8 @@ static int sd_uhs2_attach(struct mmc_host *host) err: mmc_detach_bus(host); sd_uhs2_power_off(host); - host->flags &= ~MMC_UHS2_SUPPORT; + /* Remove UHS2 timing to indicate the end of UHS2 mode */ + host->ios.timing = MMC_TIMING_LEGACY; return err; } diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c index 517c497112f4..d1f3318b7d3a 100644 --- a/drivers/mmc/host/sdhci-uhs2.c +++ b/drivers/mmc/host/sdhci-uhs2.c @@ -267,10 +267,11 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) /* UHS2 timing. Note, UHS2 timing is disabled when powering off */ ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); - if (ios->timing == MMC_TIMING_UHS2_SPEED_A || - ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || - ios->timing == MMC_TIMING_UHS2_SPEED_B || - ios->timing == MMC_TIMING_UHS2_SPEED_B_HD) + if (ios->power_mode != MMC_POWER_OFF && + (ios->timing == MMC_TIMING_UHS2_SPEED_A || + ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || + ios->timing == MMC_TIMING_UHS2_SPEED_B || + ios->timing == MMC_TIMING_UHS2_SPEED_B_HD)) ctrl_2 |= SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE; else ctrl_2 &= ~(SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE);
On Tue, 3 Oct 2023 at 17:03, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 3/10/23 15:22, Ulf Hansson wrote: > > On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 3/10/23 13:30, Ulf Hansson wrote: > >>> On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> > >>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > >>>> --- > >>>> > >>>> Updates in V8: > >>>> - Adjust the position of matching brackets. > >>>> > >>>> Updates in V6: > >>>> - Remove unnecessary functions and simplify code. > >>>> > >>>> --- > >>>> > >>>> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ > >>>> drivers/mmc/host/sdhci-uhs2.h | 2 ++ > >>>> 2 files changed, 47 insertions(+) > >>>> > >>>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > >>>> index e339821d3504..dfc80a7f1bad 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" > >>>> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > >>>> } > >>>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > >>>> > >>>> +/*****************************************************************************\ > >>>> + * * > >>>> + * Low level functions * > >>>> + * * > >>>> +\*****************************************************************************/ > >>>> + > >>>> +bool sdhci_uhs2_mode(struct sdhci_host *host) > >>>> +{ > >>>> + return host->mmc->flags & MMC_UHS2_SUPPORT; > >>> > >>> The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we > >>> should be using mmc->ios.timings, which already indicates whether we > >>> are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added > >>> this. > >>> > >>> That said, I think we should drop the sdhci_uhs2_mode() function > >>> altogether and instead use mmc_card_uhs2(), which means we should move > >>> it to include/linux/mmc/host.h, so it becomes available for host > >>> drivers. > >>> > >> > >> UHS2 mode starts at UHS2 initialization and ends either when UHS2 > >> initialization fails, or the card is removed. > >> > >> So it includes re-initialization and reset when the transfer mode > >> currently transitions through MMC_TIMING_LEGACY. > >> > >> So mmc_card_uhs2() won't work correctly for the host callbacks > >> unless something is done about that. > > > > Right, thanks for clarifying! > > > > In that case I wonder if we couldn't change the way we update the > > ->ios.timing for UHS2. It seems silly to have two (similar) ways to > > indicate that we have moved to UHS2. > > Perhaps something like below: > > diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c > index aacefdd6bc9e..e39d63d46041 100644 > --- a/drivers/mmc/core/sd_uhs2.c > +++ b/drivers/mmc/core/sd_uhs2.c > @@ -70,7 +70,8 @@ static int sd_uhs2_power_off(struct mmc_host *host) > > host->ios.vdd = 0; > host->ios.clock = 0; > - host->ios.timing = MMC_TIMING_LEGACY; > + /* Must set UHS2 timing to identify UHS2 mode */ > + host->ios.timing = MMC_TIMING_UHS2_SPEED_A; > host->ios.power_mode = MMC_POWER_OFF; > if (host->flags & MMC_UHS2_SD_TRAN) > host->flags &= ~MMC_UHS2_SD_TRAN; > @@ -1095,7 +1096,8 @@ static void sd_uhs2_detect(struct mmc_host *host) > mmc_claim_host(host); > mmc_detach_bus(host); > sd_uhs2_power_off(host); > - host->flags &= ~MMC_UHS2_SUPPORT; > + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > + host->ios.timing = MMC_TIMING_LEGACY; > mmc_release_host(host); > } > } > @@ -1338,7 +1340,8 @@ static int sd_uhs2_attach(struct mmc_host *host) > err: > mmc_detach_bus(host); > sd_uhs2_power_off(host); > - host->flags &= ~MMC_UHS2_SUPPORT; > + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > + host->ios.timing = MMC_TIMING_LEGACY; > return err; > } I wouldn't mind changing to the above. But, maybe an even better option is to use the ->timing variable in the struct sdhci_host, as it's there already to keep track of the current/previous timing state. Would that work too? > > diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > index 517c497112f4..d1f3318b7d3a 100644 > --- a/drivers/mmc/host/sdhci-uhs2.c > +++ b/drivers/mmc/host/sdhci-uhs2.c > @@ -267,10 +267,11 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > > /* UHS2 timing. Note, UHS2 timing is disabled when powering off */ > ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > - if (ios->timing == MMC_TIMING_UHS2_SPEED_A || > - ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > - ios->timing == MMC_TIMING_UHS2_SPEED_B || > - ios->timing == MMC_TIMING_UHS2_SPEED_B_HD) > + if (ios->power_mode != MMC_POWER_OFF && > + (ios->timing == MMC_TIMING_UHS2_SPEED_A || > + ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > + ios->timing == MMC_TIMING_UHS2_SPEED_B || > + ios->timing == MMC_TIMING_UHS2_SPEED_B_HD)) > ctrl_2 |= SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE; > else > ctrl_2 &= ~(SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE); > > Kind regards Uffe
On 4/10/23 11:35, Ulf Hansson wrote: > On Tue, 3 Oct 2023 at 17:03, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 3/10/23 15:22, Ulf Hansson wrote: >>> On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> >>>> On 3/10/23 13:30, Ulf Hansson wrote: >>>>> On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> >>>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com> >>>>>> --- >>>>>> >>>>>> Updates in V8: >>>>>> - Adjust the position of matching brackets. >>>>>> >>>>>> Updates in V6: >>>>>> - Remove unnecessary functions and simplify code. >>>>>> >>>>>> --- >>>>>> >>>>>> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ >>>>>> drivers/mmc/host/sdhci-uhs2.h | 2 ++ >>>>>> 2 files changed, 47 insertions(+) >>>>>> >>>>>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >>>>>> index e339821d3504..dfc80a7f1bad 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" >>>>>> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); >>>>>> >>>>>> +/*****************************************************************************\ >>>>>> + * * >>>>>> + * Low level functions * >>>>>> + * * >>>>>> +\*****************************************************************************/ >>>>>> + >>>>>> +bool sdhci_uhs2_mode(struct sdhci_host *host) >>>>>> +{ >>>>>> + return host->mmc->flags & MMC_UHS2_SUPPORT; >>>>> >>>>> The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we >>>>> should be using mmc->ios.timings, which already indicates whether we >>>>> are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added >>>>> this. >>>>> >>>>> That said, I think we should drop the sdhci_uhs2_mode() function >>>>> altogether and instead use mmc_card_uhs2(), which means we should move >>>>> it to include/linux/mmc/host.h, so it becomes available for host >>>>> drivers. >>>>> >>>> >>>> UHS2 mode starts at UHS2 initialization and ends either when UHS2 >>>> initialization fails, or the card is removed. >>>> >>>> So it includes re-initialization and reset when the transfer mode >>>> currently transitions through MMC_TIMING_LEGACY. >>>> >>>> So mmc_card_uhs2() won't work correctly for the host callbacks >>>> unless something is done about that. >>> >>> Right, thanks for clarifying! >>> >>> In that case I wonder if we couldn't change the way we update the >>> ->ios.timing for UHS2. It seems silly to have two (similar) ways to >>> indicate that we have moved to UHS2. >> >> Perhaps something like below: >> >> diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c >> index aacefdd6bc9e..e39d63d46041 100644 >> --- a/drivers/mmc/core/sd_uhs2.c >> +++ b/drivers/mmc/core/sd_uhs2.c >> @@ -70,7 +70,8 @@ static int sd_uhs2_power_off(struct mmc_host *host) >> >> host->ios.vdd = 0; >> host->ios.clock = 0; >> - host->ios.timing = MMC_TIMING_LEGACY; >> + /* Must set UHS2 timing to identify UHS2 mode */ >> + host->ios.timing = MMC_TIMING_UHS2_SPEED_A; >> host->ios.power_mode = MMC_POWER_OFF; >> if (host->flags & MMC_UHS2_SD_TRAN) >> host->flags &= ~MMC_UHS2_SD_TRAN; >> @@ -1095,7 +1096,8 @@ static void sd_uhs2_detect(struct mmc_host *host) >> mmc_claim_host(host); >> mmc_detach_bus(host); >> sd_uhs2_power_off(host); >> - host->flags &= ~MMC_UHS2_SUPPORT; >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ >> + host->ios.timing = MMC_TIMING_LEGACY; >> mmc_release_host(host); >> } >> } >> @@ -1338,7 +1340,8 @@ static int sd_uhs2_attach(struct mmc_host *host) >> err: >> mmc_detach_bus(host); >> sd_uhs2_power_off(host); >> - host->flags &= ~MMC_UHS2_SUPPORT; >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ >> + host->ios.timing = MMC_TIMING_LEGACY; >> return err; >> } > > I wouldn't mind changing to the above. But, maybe an even better > option is to use the ->timing variable in the struct sdhci_host, as > it's there already to keep track of the current/previous timing state. > Would that work too? The host does not really have enough information. > >> >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c >> index 517c497112f4..d1f3318b7d3a 100644 >> --- a/drivers/mmc/host/sdhci-uhs2.c >> +++ b/drivers/mmc/host/sdhci-uhs2.c >> @@ -267,10 +267,11 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> >> /* UHS2 timing. Note, UHS2 timing is disabled when powering off */ >> ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> - if (ios->timing == MMC_TIMING_UHS2_SPEED_A || >> - ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || >> - ios->timing == MMC_TIMING_UHS2_SPEED_B || >> - ios->timing == MMC_TIMING_UHS2_SPEED_B_HD) >> + if (ios->power_mode != MMC_POWER_OFF && >> + (ios->timing == MMC_TIMING_UHS2_SPEED_A || >> + ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || >> + ios->timing == MMC_TIMING_UHS2_SPEED_B || >> + ios->timing == MMC_TIMING_UHS2_SPEED_B_HD)) >> ctrl_2 |= SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE; >> else >> ctrl_2 &= ~(SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE); >> >> > > Kind regards > Uffe
On Tue, 10 Oct 2023 at 12:29, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 4/10/23 11:35, Ulf Hansson wrote: > > On Tue, 3 Oct 2023 at 17:03, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 3/10/23 15:22, Ulf Hansson wrote: > >>> On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>> > >>>> On 3/10/23 13:30, Ulf Hansson wrote: > >>>>> On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> > >>>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > >>>>>> --- > >>>>>> > >>>>>> Updates in V8: > >>>>>> - Adjust the position of matching brackets. > >>>>>> > >>>>>> Updates in V6: > >>>>>> - Remove unnecessary functions and simplify code. > >>>>>> > >>>>>> --- > >>>>>> > >>>>>> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ > >>>>>> drivers/mmc/host/sdhci-uhs2.h | 2 ++ > >>>>>> 2 files changed, 47 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > >>>>>> index e339821d3504..dfc80a7f1bad 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" > >>>>>> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > >>>>>> } > >>>>>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > >>>>>> > >>>>>> +/*****************************************************************************\ > >>>>>> + * * > >>>>>> + * Low level functions * > >>>>>> + * * > >>>>>> +\*****************************************************************************/ > >>>>>> + > >>>>>> +bool sdhci_uhs2_mode(struct sdhci_host *host) > >>>>>> +{ > >>>>>> + return host->mmc->flags & MMC_UHS2_SUPPORT; > >>>>> > >>>>> The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we > >>>>> should be using mmc->ios.timings, which already indicates whether we > >>>>> are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added > >>>>> this. > >>>>> > >>>>> That said, I think we should drop the sdhci_uhs2_mode() function > >>>>> altogether and instead use mmc_card_uhs2(), which means we should move > >>>>> it to include/linux/mmc/host.h, so it becomes available for host > >>>>> drivers. > >>>>> > >>>> > >>>> UHS2 mode starts at UHS2 initialization and ends either when UHS2 > >>>> initialization fails, or the card is removed. > >>>> > >>>> So it includes re-initialization and reset when the transfer mode > >>>> currently transitions through MMC_TIMING_LEGACY. > >>>> > >>>> So mmc_card_uhs2() won't work correctly for the host callbacks > >>>> unless something is done about that. > >>> > >>> Right, thanks for clarifying! > >>> > >>> In that case I wonder if we couldn't change the way we update the > >>> ->ios.timing for UHS2. It seems silly to have two (similar) ways to > >>> indicate that we have moved to UHS2. > >> > >> Perhaps something like below: > >> > >> diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c > >> index aacefdd6bc9e..e39d63d46041 100644 > >> --- a/drivers/mmc/core/sd_uhs2.c > >> +++ b/drivers/mmc/core/sd_uhs2.c > >> @@ -70,7 +70,8 @@ static int sd_uhs2_power_off(struct mmc_host *host) > >> > >> host->ios.vdd = 0; > >> host->ios.clock = 0; > >> - host->ios.timing = MMC_TIMING_LEGACY; > >> + /* Must set UHS2 timing to identify UHS2 mode */ > >> + host->ios.timing = MMC_TIMING_UHS2_SPEED_A; > >> host->ios.power_mode = MMC_POWER_OFF; > >> if (host->flags & MMC_UHS2_SD_TRAN) > >> host->flags &= ~MMC_UHS2_SD_TRAN; > >> @@ -1095,7 +1096,8 @@ static void sd_uhs2_detect(struct mmc_host *host) > >> mmc_claim_host(host); > >> mmc_detach_bus(host); > >> sd_uhs2_power_off(host); > >> - host->flags &= ~MMC_UHS2_SUPPORT; > >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > >> + host->ios.timing = MMC_TIMING_LEGACY; > >> mmc_release_host(host); > >> } > >> } > >> @@ -1338,7 +1340,8 @@ static int sd_uhs2_attach(struct mmc_host *host) > >> err: > >> mmc_detach_bus(host); > >> sd_uhs2_power_off(host); > >> - host->flags &= ~MMC_UHS2_SUPPORT; > >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > >> + host->ios.timing = MMC_TIMING_LEGACY; > >> return err; > >> } > > > > I wouldn't mind changing to the above. But, maybe an even better > > option is to use the ->timing variable in the struct sdhci_host, as > > it's there already to keep track of the current/previous timing state. > > Would that work too? > > The host does not really have enough information. Okay, let's go with the approach you suggested above/below then! > > > > >> > >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > >> index 517c497112f4..d1f3318b7d3a 100644 > >> --- a/drivers/mmc/host/sdhci-uhs2.c > >> +++ b/drivers/mmc/host/sdhci-uhs2.c > >> @@ -267,10 +267,11 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > >> > >> /* UHS2 timing. Note, UHS2 timing is disabled when powering off */ > >> ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > >> - if (ios->timing == MMC_TIMING_UHS2_SPEED_A || > >> - ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > >> - ios->timing == MMC_TIMING_UHS2_SPEED_B || > >> - ios->timing == MMC_TIMING_UHS2_SPEED_B_HD) > >> + if (ios->power_mode != MMC_POWER_OFF && > >> + (ios->timing == MMC_TIMING_UHS2_SPEED_A || > >> + ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > >> + ios->timing == MMC_TIMING_UHS2_SPEED_B || > >> + ios->timing == MMC_TIMING_UHS2_SPEED_B_HD)) > >> ctrl_2 |= SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE; > >> else > >> ctrl_2 &= ~(SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE); > >> > >> Kind regards Uffe
On Tue, Oct 10, 2023 at 7:08 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Tue, 10 Oct 2023 at 12:29, Adrian Hunter <adrian.hunter@intel.com> wrote: > > > > On 4/10/23 11:35, Ulf Hansson wrote: > > > On Tue, 3 Oct 2023 at 17:03, Adrian Hunter <adrian.hunter@intel.com> wrote: > > >> > > >> On 3/10/23 15:22, Ulf Hansson wrote: > > >>> On Tue, 3 Oct 2023 at 13:37, Adrian Hunter <adrian.hunter@intel.com> wrote: > > >>>> > > >>>> On 3/10/23 13:30, Ulf Hansson wrote: > > >>>>> On Fri, 15 Sept 2023 at 11:44, Victor Shih <victorshihgli@gmail.com> 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> > > >>>>>> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > > >>>>>> --- > > >>>>>> > > >>>>>> Updates in V8: > > >>>>>> - Adjust the position of matching brackets. > > >>>>>> > > >>>>>> Updates in V6: > > >>>>>> - Remove unnecessary functions and simplify code. > > >>>>>> > > >>>>>> --- > > >>>>>> > > >>>>>> drivers/mmc/host/sdhci-uhs2.c | 45 +++++++++++++++++++++++++++++++++++ > > >>>>>> drivers/mmc/host/sdhci-uhs2.h | 2 ++ > > >>>>>> 2 files changed, 47 insertions(+) > > >>>>>> > > >>>>>> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > > >>>>>> index e339821d3504..dfc80a7f1bad 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" > > >>>>>> @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) > > >>>>>> } > > >>>>>> EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); > > >>>>>> > > >>>>>> +/*****************************************************************************\ > > >>>>>> + * * > > >>>>>> + * Low level functions * > > >>>>>> + * * > > >>>>>> +\*****************************************************************************/ > > >>>>>> + > > >>>>>> +bool sdhci_uhs2_mode(struct sdhci_host *host) > > >>>>>> +{ > > >>>>>> + return host->mmc->flags & MMC_UHS2_SUPPORT; > > >>>>> > > >>>>> The MMC_UHS2_SUPPORT bit looks redundant to me. Instead, I think we > > >>>>> should be using mmc->ios.timings, which already indicates whether we > > >>>>> are using UHS2 (MMC_TIMING_UHS2_SPEED_*). See patch2 where we added > > >>>>> this. > > >>>>> > > >>>>> That said, I think we should drop the sdhci_uhs2_mode() function > > >>>>> altogether and instead use mmc_card_uhs2(), which means we should move > > >>>>> it to include/linux/mmc/host.h, so it becomes available for host > > >>>>> drivers. > > >>>>> > > >>>> > > >>>> UHS2 mode starts at UHS2 initialization and ends either when UHS2 > > >>>> initialization fails, or the card is removed. > > >>>> > > >>>> So it includes re-initialization and reset when the transfer mode > > >>>> currently transitions through MMC_TIMING_LEGACY. > > >>>> > > >>>> So mmc_card_uhs2() won't work correctly for the host callbacks > > >>>> unless something is done about that. > > >>> > > >>> Right, thanks for clarifying! > > >>> > > >>> In that case I wonder if we couldn't change the way we update the > > >>> ->ios.timing for UHS2. It seems silly to have two (similar) ways to > > >>> indicate that we have moved to UHS2. > > >> > > >> Perhaps something like below: > > >> > > >> diff --git a/drivers/mmc/core/sd_uhs2.c b/drivers/mmc/core/sd_uhs2.c > > >> index aacefdd6bc9e..e39d63d46041 100644 > > >> --- a/drivers/mmc/core/sd_uhs2.c > > >> +++ b/drivers/mmc/core/sd_uhs2.c > > >> @@ -70,7 +70,8 @@ static int sd_uhs2_power_off(struct mmc_host *host) > > >> > > >> host->ios.vdd = 0; > > >> host->ios.clock = 0; > > >> - host->ios.timing = MMC_TIMING_LEGACY; > > >> + /* Must set UHS2 timing to identify UHS2 mode */ > > >> + host->ios.timing = MMC_TIMING_UHS2_SPEED_A; > > >> host->ios.power_mode = MMC_POWER_OFF; > > >> if (host->flags & MMC_UHS2_SD_TRAN) > > >> host->flags &= ~MMC_UHS2_SD_TRAN; > > >> @@ -1095,7 +1096,8 @@ static void sd_uhs2_detect(struct mmc_host *host) > > >> mmc_claim_host(host); > > >> mmc_detach_bus(host); > > >> sd_uhs2_power_off(host); > > >> - host->flags &= ~MMC_UHS2_SUPPORT; > > >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > > >> + host->ios.timing = MMC_TIMING_LEGACY; > > >> mmc_release_host(host); > > >> } > > >> } > > >> @@ -1338,7 +1340,8 @@ static int sd_uhs2_attach(struct mmc_host *host) > > >> err: > > >> mmc_detach_bus(host); > > >> sd_uhs2_power_off(host); > > >> - host->flags &= ~MMC_UHS2_SUPPORT; > > >> + /* Remove UHS2 timing to indicate the end of UHS2 mode */ > > >> + host->ios.timing = MMC_TIMING_LEGACY; > > >> return err; > > >> } > > > > > > I wouldn't mind changing to the above. But, maybe an even better > > > option is to use the ->timing variable in the struct sdhci_host, as > > > it's there already to keep track of the current/previous timing state. > > > Would that work too? > > > > The host does not really have enough information. > > Okay, let's go with the approach you suggested above/below then! > Hi, Ulf and Adrian I will update this in version 13. Thanks, Victor Shih > > > > > > > >> > > >> diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c > > >> index 517c497112f4..d1f3318b7d3a 100644 > > >> --- a/drivers/mmc/host/sdhci-uhs2.c > > >> +++ b/drivers/mmc/host/sdhci-uhs2.c > > >> @@ -267,10 +267,11 @@ static void __sdhci_uhs2_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > > >> > > >> /* UHS2 timing. Note, UHS2 timing is disabled when powering off */ > > >> ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > >> - if (ios->timing == MMC_TIMING_UHS2_SPEED_A || > > >> - ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > > >> - ios->timing == MMC_TIMING_UHS2_SPEED_B || > > >> - ios->timing == MMC_TIMING_UHS2_SPEED_B_HD) > > >> + if (ios->power_mode != MMC_POWER_OFF && > > >> + (ios->timing == MMC_TIMING_UHS2_SPEED_A || > > >> + ios->timing == MMC_TIMING_UHS2_SPEED_A_HD || > > >> + ios->timing == MMC_TIMING_UHS2_SPEED_B || > > >> + ios->timing == MMC_TIMING_UHS2_SPEED_B_HD)) > > >> ctrl_2 |= SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE; > > >> else > > >> ctrl_2 &= ~(SDHCI_CTRL_UHS2 | SDHCI_CTRL_UHS2_ENABLE); > > >> > > >> > > Kind regards > Uffe
diff --git a/drivers/mmc/host/sdhci-uhs2.c b/drivers/mmc/host/sdhci-uhs2.c index e339821d3504..dfc80a7f1bad 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" @@ -49,6 +51,49 @@ void sdhci_uhs2_dump_regs(struct sdhci_host *host) } EXPORT_SYMBOL_GPL(sdhci_uhs2_dump_regs); +/*****************************************************************************\ + * * + * Low level functions * + * * +\*****************************************************************************/ + +bool sdhci_uhs2_mode(struct sdhci_host *host) +{ + return host->mmc->flags & MMC_UHS2_SUPPORT; +} + +/** + * 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; + u32 val; + + sdhci_writew(host, mask, SDHCI_UHS2_SW_RESET); + + if (mask & SDHCI_UHS2_SW_RESET_FULL) + host->clock = 0; + + /* Wait max 100 ms */ + timeout = 100000; + + /* hw clears the bit when it's done */ + if (read_poll_timeout_atomic(sdhci_readw, val, !(val & mask), 10, + timeout, true, host, SDHCI_UHS2_SW_RESET)) { + 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; + } +} +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 2bfe18d29bca..8253d50f7852 100644 --- a/drivers/mmc/host/sdhci-uhs2.h +++ b/drivers/mmc/host/sdhci-uhs2.h @@ -177,5 +177,7 @@ struct sdhci_host; void sdhci_uhs2_dump_regs(struct sdhci_host *host); +bool sdhci_uhs2_mode(struct sdhci_host *host); +void sdhci_uhs2_reset(struct sdhci_host *host, u16 mask); #endif /* __SDHCI_UHS2_H */