| Message ID | 1522959594-3411-6-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdio: Enable SW reset of SDIO cards | expand |
Hi Ulf, On 2018/4/6 4:19, Ulf Hansson wrote: > Let's implement the ->sw_reset() bus ops to allow SDIO func drivers, in > particular, to make a SW reset without doing a full power cycle of the SDIO > card. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/sdio.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > index 0124e0e..088c80c 100644 > --- a/drivers/mmc/core/sdio.c > +++ b/drivers/mmc/core/sdio.c > @@ -1044,6 +1044,18 @@ static int mmc_sdio_hw_reset(struct mmc_host *host) > return mmc_sdio_power_restore(host); > } > > +static int mmc_sdio_sw_reset(struct mmc_host *host) > +{ > + mmc_set_clock(host, host->f_init); This reminds me to think that why we don't fold it into mmc_set_initial_state()? See mmc_power_up() calls mmc_set_initial_state () and then the clock is set to host->f_init later. > + sdio_reset(host); > + mmc_go_idle(host); > + mmc_sdio_reinit_card() will reset I/O and memory portion internally. So maybe we don't need these extra reset ahead? > + mmc_set_initial_state(host); > + mmc_set_initial_signal_voltage(host); > + > + return mmc_sdio_reinit_card(host, 0); > +} > + > static const struct mmc_bus_ops mmc_sdio_ops = { > .remove = mmc_sdio_remove, > .detect = mmc_sdio_detect, > @@ -1055,6 +1067,7 @@ static const struct mmc_bus_ops mmc_sdio_ops = { > .power_restore = mmc_sdio_power_restore, > .alive = mmc_sdio_alive, > .hw_reset = mmc_sdio_hw_reset, > + .sw_reset = mmc_sdio_sw_reset, > }; > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11 April 2018 at 03:16, Shawn Lin <shawn.lin@rock-chips.com> wrote: > Hi Ulf, > > On 2018/4/6 4:19, Ulf Hansson wrote: >> >> Let's implement the ->sw_reset() bus ops to allow SDIO func drivers, in >> particular, to make a SW reset without doing a full power cycle of the >> SDIO >> card. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> drivers/mmc/core/sdio.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >> index 0124e0e..088c80c 100644 >> --- a/drivers/mmc/core/sdio.c >> +++ b/drivers/mmc/core/sdio.c >> @@ -1044,6 +1044,18 @@ static int mmc_sdio_hw_reset(struct mmc_host *host) >> return mmc_sdio_power_restore(host); >> } >> +static int mmc_sdio_sw_reset(struct mmc_host *host) >> +{ >> + mmc_set_clock(host, host->f_init); > > > This reminds me to think that why we don't fold it > into mmc_set_initial_state()? See mmc_power_up() calls > mmc_set_initial_state () and then the clock is set to host->f_init > later. I see your point, however mmc_set_inital_state() is also called from mmc_power_off(), which is when the clock is set to zero. > >> + sdio_reset(host); >> + mmc_go_idle(host); >> + > > > mmc_sdio_reinit_card() will reset I/O and memory portion internally. > So maybe we don't need these extra reset ahead? Are you suggesting the mmc_sdio_reinit_card() should not be doing sdio_reset() + mmc_go_idle() - or that the above calls should go away? I don't think dropping the above calls are a good idea (at least both of them), as changing to the initial state and initial signal voltage, without first doing a reset may put the card in some undefined error state. Don't you think? > > >> + mmc_set_initial_state(host); >> + mmc_set_initial_signal_voltage(host); >> + >> + return mmc_sdio_reinit_card(host, 0); >> +} >> + >> static const struct mmc_bus_ops mmc_sdio_ops = { >> .remove = mmc_sdio_remove, >> .detect = mmc_sdio_detect, >> @@ -1055,6 +1067,7 @@ static const struct mmc_bus_ops mmc_sdio_ops = { >> .power_restore = mmc_sdio_power_restore, >> .alive = mmc_sdio_alive, >> .hw_reset = mmc_sdio_hw_reset, >> + .sw_reset = mmc_sdio_sw_reset, >> }; >> > > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018/4/11 20:39, Ulf Hansson wrote: > On 11 April 2018 at 03:16, Shawn Lin <shawn.lin@rock-chips.com> wrote: >> Hi Ulf, >> >> On 2018/4/6 4:19, Ulf Hansson wrote: >>> >>> Let's implement the ->sw_reset() bus ops to allow SDIO func drivers, in >>> particular, to make a SW reset without doing a full power cycle of the >>> SDIO >>> card. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >>> --- >>> drivers/mmc/core/sdio.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>> index 0124e0e..088c80c 100644 >>> --- a/drivers/mmc/core/sdio.c >>> +++ b/drivers/mmc/core/sdio.c >>> @@ -1044,6 +1044,18 @@ static int mmc_sdio_hw_reset(struct mmc_host *host) >>> return mmc_sdio_power_restore(host); >>> } >>> +static int mmc_sdio_sw_reset(struct mmc_host *host) >>> +{ >>> + mmc_set_clock(host, host->f_init); >> >> >> This reminds me to think that why we don't fold it >> into mmc_set_initial_state()? See mmc_power_up() calls >> mmc_set_initial_state () and then the clock is set to host->f_init >> later. > > I see your point, however mmc_set_inital_state() is also called from > mmc_power_off(), which is when the clock is set to zero. > Oh, I missed that. >> >>> + sdio_reset(host); >>> + mmc_go_idle(host); >>> + >> >> >> mmc_sdio_reinit_card() will reset I/O and memory portion internally. >> So maybe we don't need these extra reset ahead? > > Are you suggesting the mmc_sdio_reinit_card() should not be doing > sdio_reset() + mmc_go_idle() - or that the above calls should go away? > > I don't think dropping the above calls are a good idea (at least both > of them), as changing to the initial state and initial signal voltage, > without first doing a reset may put the card in some undefined error > state. Don't you think? Fair enough. > >> >> >>> + mmc_set_initial_state(host); >>> + mmc_set_initial_signal_voltage(host); >>> + >>> + return mmc_sdio_reinit_card(host, 0); >>> +} >>> + >>> static const struct mmc_bus_ops mmc_sdio_ops = { >>> .remove = mmc_sdio_remove, >>> .detect = mmc_sdio_detect, >>> @@ -1055,6 +1067,7 @@ static const struct mmc_bus_ops mmc_sdio_ops = { >>> .power_restore = mmc_sdio_power_restore, >>> .alive = mmc_sdio_alive, >>> .hw_reset = mmc_sdio_hw_reset, >>> + .sw_reset = mmc_sdio_sw_reset, >>> }; >>> >> >> > > Kind regards > Uffe > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 0124e0e..088c80c 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -1044,6 +1044,18 @@ static int mmc_sdio_hw_reset(struct mmc_host *host) return mmc_sdio_power_restore(host); } +static int mmc_sdio_sw_reset(struct mmc_host *host) +{ + mmc_set_clock(host, host->f_init); + sdio_reset(host); + mmc_go_idle(host); + + mmc_set_initial_state(host); + mmc_set_initial_signal_voltage(host); + + return mmc_sdio_reinit_card(host, 0); +} + static const struct mmc_bus_ops mmc_sdio_ops = { .remove = mmc_sdio_remove, .detect = mmc_sdio_detect, @@ -1055,6 +1067,7 @@ static const struct mmc_bus_ops mmc_sdio_ops = { .power_restore = mmc_sdio_power_restore, .alive = mmc_sdio_alive, .hw_reset = mmc_sdio_hw_reset, + .sw_reset = mmc_sdio_sw_reset, };
Let's implement the ->sw_reset() bus ops to allow SDIO func drivers, in particular, to make a SW reset without doing a full power cycle of the SDIO card. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/core/sdio.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) -- 2.7.4