[RFC PATCH 0/4] Introduce new caps to improve the card's init sequence

[RFC PATCH 0/4] Introduce new caps to improve the card's init sequence

[Shawn Lin]

Hi all,

This patchset is gonna improve the card's init sequence
by exposing some caps to DT.

The basic idea is to skip sending specific init cmd inspired
by Carlo Caione's commit[0].

To make it possible, I firstly expose Carlo's MMC_CAP2_NO_SDIO
to DT and extend two new caps for similar usage of sd and mmc.
We probably need it because for most of the boards, each of the
slots should have a specific function when designed. It's impossible
for one slot which can either to be used as eMMC or a SD card for a
given board. The same for SDIO case.

We could have two ways to improve it
A) Skip sending specific commands if knowing we don't support
the specific card type.
B) Allow sending specific commands if knowing we do support
the specific card type.

A) and B) shouldn't have difference, but I take A) as the final
one as it looks more consistent with Carlo's way, which does not
seem to break anything as possible.

The only roadblock for this patchset to be landed should be the
improvement we gain from it. Theoretically sdio-card doesn't get
improvment as it's already in the first place to be attached.
But considering the sd and (e)MMC case, we should gain more benifit
from it.

>From the test, we can save nearly 2ms for attaching emmc against the
original 8ms. And we gain more than 30us improvement for sd card for
each insert.

[0]: http://permalink.gmane.org/gmane.linux.kernel.mmc/34774



Shawn Lin (4):
  Documentation: mmc: add description for new caps
  mmc: core: expose MMC_CAP2_NO_SDIO to dt-binding
  mmc: core: add cap-no-sd and cap-no-mmc properties
  mmc: core: improve initialization flow

 Documentation/devicetree/bindings/mmc/mmc.txt |  3 +++
 drivers/mmc/core/core.c                       | 10 +++++-----
 drivers/mmc/core/host.c                       |  6 ++++++
 include/linux/mmc/host.h                      |  2 ++
 4 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.3.7

[RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Shawn Lin]

Let's add some basic description for cap-no-sdio,
cap-no-sd and cap-no-mmc.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index ecc007a..e16d42b 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -39,6 +39,9 @@ Optional properties:
 - cap-power-off-card: powering off the card is safe
 - cap-mmc-hw-reset: eMMC hardware reset is supported
 - cap-sdio-irq: enable SDIO IRQ signalling on this interface
+- cap-no-sdio: skip sending sdio cmd during initialization
+- cap-no-sd: skip sending sd cmd during initialization
+- cap-no-mmc: skip sending mmc cmd during initialization
 - full-pwr-cycle: full power cycle of the card is supported
 - mmc-ddr-1_8v: eMMC high-speed DDR mode(1.8V I/O) is supported
 - mmc-ddr-1_2v: eMMC high-speed DDR mode(1.2V I/O) is supported
-- 
2.3.7

Re: [RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Rob Herring]

On Wed, Jun 08, 2016 at 04:20:21PM +0800, Shawn Lin wrote:
> Let's add some basic description for cap-no-sdio,
> cap-no-sd and cap-no-mmc.

This doesn't say why.

If you know what is there, then there's a better way to do this. Look at 
the child nodes because chances are if you have something soldered down 
like SDIO or eMMC, then you have a child node already for resets, 
regulators, etc.

Rob

Re: [RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Ulf Hansson]

On 8 June 2016 at 22:48, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jun 08, 2016 at 04:20:21PM +0800, Shawn Lin wrote:
>> Let's add some basic description for cap-no-sdio,
>> cap-no-sd and cap-no-mmc.
>
> This doesn't say why.
>
> If you know what is there, then there's a better way to do this. Look at
> the child nodes because chances are if you have something soldered down
> like SDIO or eMMC, then you have a child node already for resets,
> regulators, etc.
>

According to patch4, this is a description of the mmc controller,
perhaps together with some limitations as per SoC/platform and thus
has nothing to do with the card.

I have seen several drivers, which needs to validate every single
piece of command to avoid execute those it cannot support.

So, I personally thinks these new DT bindings would be nice to add.

Kind regards
Uffe

Re: [RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Doug Anderson]

Hi,

On Wed, Jun 8, 2016 at 1:20 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> Let's add some basic description for cap-no-sdio,
> cap-no-sd and cap-no-mmc.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
>  Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index ecc007a..e16d42b 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -39,6 +39,9 @@ Optional properties:
>  - cap-power-off-card: powering off the card is safe
>  - cap-mmc-hw-reset: eMMC hardware reset is supported
>  - cap-sdio-irq: enable SDIO IRQ signalling on this interface
> +- cap-no-sdio: skip sending sdio cmd during initialization
> +- cap-no-sd: skip sending sd cmd during initialization
> +- cap-no-mmc: skip sending mmc cmd during initialization

In the past it has been determined that the "cap-" prefix is
unnecessary.  Some properties still have "cap-" because of backward
compatibility reasons, but as I understand it new properties shouldn't
start with "cap-".

See <https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/034664.html>
for reference.

-Doug

Re: [RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Shawn Lin]

在 2016/6/22 23:06, Doug Anderson 写道:
> Hi,
>
> On Wed, Jun 8, 2016 at 1:20 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>> Let's add some basic description for cap-no-sdio,
>> cap-no-sd and cap-no-mmc.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>>  Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
>> index ecc007a..e16d42b 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>> @@ -39,6 +39,9 @@ Optional properties:
>>  - cap-power-off-card: powering off the card is safe
>>  - cap-mmc-hw-reset: eMMC hardware reset is supported
>>  - cap-sdio-irq: enable SDIO IRQ signalling on this interface
>> +- cap-no-sdio: skip sending sdio cmd during initialization
>> +- cap-no-sd: skip sending sd cmd during initialization
>> +- cap-no-mmc: skip sending mmc cmd during initialization
>
> In the past it has been determined that the "cap-" prefix is
> unnecessary.  Some properties still have "cap-" because of backward
> compatibility reasons, but as I understand it new properties shouldn't
> start with "cap-".
>
> See <https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/034664.html>
> for reference.
>

Really?  was that the final decision? I just see Olof complained it but
don't see other comments from dt maintainer or from mmc
maintainer(Chris Ball)?

Hi Ulf and Rob,

What is your opinion?


> -Doug
>
>
>


-- 
Best Regards
Shawn Lin

Re: [RFC PATCH 1/4] Documentation: mmc: add description for new caps

[Doug Anderson]

Hi,

On Wed, Jun 22, 2016 at 6:29 PM, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> 在 2016/6/22 23:06, Doug Anderson 写道:
>>
>> Hi,
>>
>> On Wed, Jun 8, 2016 at 1:20 AM, Shawn Lin <shawn.lin@rock-chips.com>
>> wrote:
>>>
>>> Let's add some basic description for cap-no-sdio,
>>> cap-no-sd and cap-no-mmc.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt
>>> b/Documentation/devicetree/bindings/mmc/mmc.txt
>>> index ecc007a..e16d42b 100644
>>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>>> @@ -39,6 +39,9 @@ Optional properties:
>>>  - cap-power-off-card: powering off the card is safe
>>>  - cap-mmc-hw-reset: eMMC hardware reset is supported
>>>  - cap-sdio-irq: enable SDIO IRQ signalling on this interface
>>> +- cap-no-sdio: skip sending sdio cmd during initialization
>>> +- cap-no-sd: skip sending sd cmd during initialization
>>> +- cap-no-mmc: skip sending mmc cmd during initialization
>>
>>
>> In the past it has been determined that the "cap-" prefix is
>> unnecessary.  Some properties still have "cap-" because of backward
>> compatibility reasons, but as I understand it new properties shouldn't
>> start with "cap-".
>>
>> See
>> <https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/034664.html>
>> for reference.
>>
>
> Really?  was that the final decision? I just see Olof complained it but
> don't see other comments from dt maintainer or from mmc
> maintainer(Chris Ball)?
>
> Hi Ulf and Rob,
>
> What is your opinion?

Notice that most things added later (like mmc-hs200-1_8v) don't have "cap-"

[RFC PATCH 2/4] mmc: core: expose MMC_CAP2_NO_SDIO to dt-binding

[Shawn Lin]

Add cap-no-sdio property, so we can use MMC_CAP2_NO_SDIO
for different slots from dt.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/host.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index d7e86f9..933eeea 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -296,6 +296,8 @@ int mmc_of_parse(struct mmc_host *host)
 		host->caps |= MMC_CAP_SDIO_IRQ;
 	if (of_property_read_bool(np, "full-pwr-cycle"))
 		host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE;
+	if (of_property_read_bool(np, "cap-no-sdio"))
+		host->caps2 |= MMC_CAP2_NO_SDIO;
 	if (of_property_read_bool(np, "keep-power-in-suspend"))
 		host->pm_caps |= MMC_PM_KEEP_POWER;
 	if (of_property_read_bool(np, "wakeup-source") ||
-- 
2.3.7

[RFC PATCH 3/4] mmc: core: add cap-no-sd and cap-no-mmc properties

[Shawn Lin]

Introduce cap-no-sd and cap-no-mmc properties to skip
sending sd and mmc commands during initialization
respectively.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/host.c  | 4 ++++
 include/linux/mmc/host.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 933eeea..c760097 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -298,6 +298,10 @@ int mmc_of_parse(struct mmc_host *host)
 		host->caps2 |= MMC_CAP2_FULL_PWR_CYCLE;
 	if (of_property_read_bool(np, "cap-no-sdio"))
 		host->caps2 |= MMC_CAP2_NO_SDIO;
+	if (of_property_read_bool(np, "cap-no-sd"))
+		host->caps2 |= MMC_CAP2_NO_SD;
+	if (of_property_read_bool(np, "cap-no-mmc"))
+		host->caps2 |= MMC_CAP2_NO_MMC;
 	if (of_property_read_bool(np, "keep-power-in-suspend"))
 		host->pm_caps |= MMC_PM_KEEP_POWER;
 	if (of_property_read_bool(np, "wakeup-source") ||
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index d72c0c3..1bd77c7 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -309,6 +309,8 @@ struct mmc_host {
 #define MMC_CAP2_NO_WRITE_PROTECT (1 << 18)	/* No physical write protect pin, assume that card is always read-write */
 #define MMC_CAP2_NO_SDIO	(1 << 19)	/* Do not send SDIO commands during initialization */
 #define MMC_CAP2_HS400_ES	(1 << 20)	/* Host supports enhanced strobe */
+#define MMC_CAP2_NO_SD		(1 << 21)	/* Do not send SD commands during initialization */
+#define MMC_CAP2_NO_MMC		(1 << 22)	/* Do not send MMC commands during initialization */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
-- 
2.3.7

[RFC PATCH 4/4] mmc: core: improve initialization flow

[Shawn Lin]

We should skip sending some unnecessary cmds
during initialization if we know that this controller
can't support the claimed function.

In this way, we provide the capabilities for DT to
decide whether they need to reduce booting time if they
know a slot is just only for one card type.

We also don't want to break the backward compatibility
if not assigning any one of these caps for existing dts
or dtb. So this patch should be safe.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index e864187..663bf18 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2503,13 +2503,13 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 	mmc_send_if_cond(host, host->ocr_avail);
 
 	/* Order's important: probe SDIO, then SD, then MMC */
-	if (!(host->caps2 & MMC_CAP2_NO_SDIO))
-		if (!mmc_attach_sdio(host))
-			return 0;
+	if (!(host->caps2 & MMC_CAP2_NO_SDIO) && !mmc_attach_sdio(host))
+		return 0;
 
-	if (!mmc_attach_sd(host))
+	if (!(host->caps2 & MMC_CAP2_NO_SD) && !mmc_attach_sd(host))
 		return 0;
-	if (!mmc_attach_mmc(host))
+
+	if (!(host->caps2 & MMC_CAP2_NO_MMC) && !mmc_attach_mmc(host))
 		return 0;
 
 	mmc_power_off(host);
-- 
2.3.7

Re: [RFC PATCH 0/4] Introduce new caps to improve the card's init sequence

[Ulf Hansson]

On 8 June 2016 at 10:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> Hi all,
>
> This patchset is gonna improve the card's init sequence
> by exposing some caps to DT.
>
> The basic idea is to skip sending specific init cmd inspired
> by Carlo Caione's commit[0].
>
> To make it possible, I firstly expose Carlo's MMC_CAP2_NO_SDIO
> to DT and extend two new caps for similar usage of sd and mmc.
> We probably need it because for most of the boards, each of the
> slots should have a specific function when designed. It's impossible
> for one slot which can either to be used as eMMC or a SD card for a
> given board. The same for SDIO case.
>
> We could have two ways to improve it
> A) Skip sending specific commands if knowing we don't support
> the specific card type.
> B) Allow sending specific commands if knowing we do support
> the specific card type.
>
> A) and B) shouldn't have difference, but I take A) as the final
> one as it looks more consistent with Carlo's way, which does not
> seem to break anything as possible.
>
> The only roadblock for this patchset to be landed should be the
> improvement we gain from it. Theoretically sdio-card doesn't get
> improvment as it's already in the first place to be attached.
> But considering the sd and (e)MMC case, we should gain more benifit
> from it.
>
> From the test, we can save nearly 2ms for attaching emmc against the
> original 8ms. And we gain more than 30us improvement for sd card for
> each insert.
>
> [0]: http://permalink.gmane.org/gmane.linux.kernel.mmc/34774
>
>
>
> Shawn Lin (4):
>   Documentation: mmc: add description for new caps
>   mmc: core: expose MMC_CAP2_NO_SDIO to dt-binding
>   mmc: core: add cap-no-sd and cap-no-mmc properties
>   mmc: core: improve initialization flow
>

I would prefer that you restructure this series a bit.

Start by adding each new MMC_CAP2* separately and without involving
the DT parsing.

Make the DT documentation patch come after the above, and add a new
separate patch which exposes the caps to device tree.

In that way, I can pick up the non DT parts and wait for Rob's
ack/nack of the DT bindings.

Kind regards
Uffe

>  Documentation/devicetree/bindings/mmc/mmc.txt |  3 +++
>  drivers/mmc/core/core.c                       | 10 +++++-----
>  drivers/mmc/core/host.c                       |  6 ++++++
>  include/linux/mmc/host.h                      |  2 ++
>  4 files changed, 16 insertions(+), 5 deletions(-)
>
> --
> 2.3.7
>
>
> --
> 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

Re: [RFC PATCH 0/4] Introduce new caps to improve the card's init sequence

[Shawn Lin]

在 2016/6/22 20:37, Ulf Hansson 写道:
> On 8 June 2016 at 10:19, Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>
>> Hi all,
>>
>> This patchset is gonna improve the card's init sequence
>> by exposing some caps to DT.
>>
>> The basic idea is to skip sending specific init cmd inspired
>> by Carlo Caione's commit[0].
>>
>> To make it possible, I firstly expose Carlo's MMC_CAP2_NO_SDIO
>> to DT and extend two new caps for similar usage of sd and mmc.
>> We probably need it because for most of the boards, each of the
>> slots should have a specific function when designed. It's impossible
>> for one slot which can either to be used as eMMC or a SD card for a
>> given board. The same for SDIO case.
>>
>> We could have two ways to improve it
>> A) Skip sending specific commands if knowing we don't support
>> the specific card type.
>> B) Allow sending specific commands if knowing we do support
>> the specific card type.
>>
>> A) and B) shouldn't have difference, but I take A) as the final
>> one as it looks more consistent with Carlo's way, which does not
>> seem to break anything as possible.
>>
>> The only roadblock for this patchset to be landed should be the
>> improvement we gain from it. Theoretically sdio-card doesn't get
>> improvment as it's already in the first place to be attached.
>> But considering the sd and (e)MMC case, we should gain more benifit
>> from it.
>>
>> From the test, we can save nearly 2ms for attaching emmc against the
>> original 8ms. And we gain more than 30us improvement for sd card for
>> each insert.
>>
>> [0]: http://permalink.gmane.org/gmane.linux.kernel.mmc/34774
>>
>>
>>
>> Shawn Lin (4):
>>   Documentation: mmc: add description for new caps
>>   mmc: core: expose MMC_CAP2_NO_SDIO to dt-binding
>>   mmc: core: add cap-no-sd and cap-no-mmc properties
>>   mmc: core: improve initialization flow
>>
>
> I would prefer that you restructure this series a bit.

I will do it and I think Doug was asking we to drop the "cap-"
prefix which I think need to be clarified before sending
non-RFC one. :)

>
> Start by adding each new MMC_CAP2* separately and without involving
> the DT parsing.
>
> Make the DT documentation patch come after the above, and add a new
> separate patch which exposes the caps to device tree.
>
> In that way, I can pick up the non DT parts and wait for Rob's
> ack/nack of the DT bindings.
>
> Kind regards
> Uffe
>
>>  Documentation/devicetree/bindings/mmc/mmc.txt |  3 +++
>>  drivers/mmc/core/core.c                       | 10 +++++-----
>>  drivers/mmc/core/host.c                       |  6 ++++++
>>  include/linux/mmc/host.h                      |  2 ++
>>  4 files changed, 16 insertions(+), 5 deletions(-)
>>
>> --
>> 2.3.7
>>
>>
>> --
>> 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
>
>
>


-- 
Best Regards
Shawn Lin