[5/5] mmc: core: Implement ->sw_reset bus ops for SDIO

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
Related show

Commit Message

Ulf Hansson April 5, 2018, 8:19 p.m.
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

Comments

Shawn Lin April 11, 2018, 1:16 a.m. | #1
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
Ulf Hansson April 11, 2018, 12:39 p.m. | #2
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
Shawn Lin April 12, 2018, 12:49 a.m. | #3
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

Patch

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,
 };