[PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Chris Brandt]

At first this started out as a simple typo fix, until I realized
that the SDHI in the RZ/A1 has 2 clocks per channel and both need
to be turned on/off.

This patch series adds the ability to specify 2 clocks instead of
just 1, and does so for the RZ/A1 r7s72100.

This patch has been tested on an RZ/A1 RSK board. Card detect works
fine as well as bind/unbind.

v6:
* more detailed descritpion in tmio_mmc.txt

v5:
* Take implementation details out of DT documentation and put into
  the driver code.

v4:
* No code changes, only text udpates to explain why there are 2 clocks
  and why we need to treat them as 1 clock.

Chris Brandt (3):
  mmc: sh_mobile_sdhi: add support for 2 clocks
  mmc: sh_mobile_sdhi: explain clock bindings
  ARM: dts: r7s72100: update sdhi clock bindings

 Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 13 ++++++++++++
 arch/arm/boot/dts/r7s72100.dtsi                    | 17 +++++++++++-----
 drivers/mmc/host/sh_mobile_sdhi.c                  | 23 ++++++++++++++++++++++
 include/dt-bindings/clock/r7s72100-clock.h         |  6 ++++--
 4 files changed, 52 insertions(+), 7 deletions(-)

-- 
2.10.1

[PATCH v6 1/3] mmc: sh_mobile_sdhi: add support for 2 clocks

[Chris Brandt]

Some controllers have 2 clock sources instead of 1. The 2nd clock
is for the internal card detect logic and must be enabled/disabled
along with the main core clock for proper operation.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v5:
* call clk_disable_unprepare even if clk_cd is NULL
v4:
* add technical explanation within probe routine
v3:
* add more clarification to the commit log
v2:
* changed clk2 to clk_cd
* disable clk if clk_cd enable fails
* changed clock name from "carddetect" to "cd"
---
 drivers/mmc/host/sh_mobile_sdhi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 59db14b..d964a0d 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -143,6 +143,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
 
 struct sh_mobile_sdhi {
 	struct clk *clk;
+	struct clk *clk_cd;
 	struct tmio_mmc_data mmc_data;
 	struct tmio_mmc_dma dma_priv;
 	struct pinctrl *pinctrl;
@@ -190,6 +191,12 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
 	if (ret < 0)
 		return ret;
 
+	ret = clk_prepare_enable(priv->clk_cd);
+	if (ret < 0) {
+		clk_disable_unprepare(priv->clk);
+		return ret;
+	}
+
 	/*
 	 * The clock driver may not know what maximum frequency
 	 * actually works, so it should be set with the max-frequency
@@ -255,6 +262,7 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
 
 	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_cd);
 }
 
 static int sh_mobile_sdhi_card_busy(struct mmc_host *mmc)
@@ -572,6 +580,21 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 	}
 
+	/*
+	 * Some controllers provide a 2nd clock just to run the internal card
+	 * detection logic. Unfortunately, the existing driver architecture does
+	 * not support a separation of clocks for runtime PM usage. When
+	 * native hotplug is used, the tmio driver assumes that the core
+	 * must continue to run for card detect to stay active, so we cannot
+	 * disable it.
+	 * Additionally, it is prohibited to supply a clock to the core but not
+	 * to the card detect circuit. That leaves us with if separate clocks
+	 * are presented, we must treat them both as virtually 1 clock.
+	 */
+	priv->clk_cd = devm_clk_get(&pdev->dev, "cd");
+	if (IS_ERR(priv->clk_cd))
+		priv->clk_cd = NULL;
+
 	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (!IS_ERR(priv->pinctrl)) {
 		priv->pins_default = pinctrl_lookup_state(priv->pinctrl,
-- 
2.10.1

[PATCH v6 2/3] mmc: sh_mobile_sdhi: explain clock bindings

[Chris Brandt]

In the case of a single clock source, you don't need names. However,
if the controller has 2 clock sources, you need to name them correctly
so the driver can find the 2nd one. The 2nd clock is for the internal
card detect logic.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5:
* add Reviewed-by: Geert Uytterhoeven
* list number of clocks for each SoC
* remove example because it already exists in mmc.txt
v4:
* just explain there might be 2 clocks, don't explain how
  we will use them in the driver
v3:
* add more clarification about why there are sometimes 2 clocks
  and what you should do with them.
* remove 'status = "disabled"' from example
v2:
* fix spelling and change wording
* changed clock name from "carddetect" to "cd"
---
 Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
index a1650ed..4fd8b7a 100644
--- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
@@ -25,6 +25,19 @@ Required properties:
 		"renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
 		"renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
 
+- clocks: Most controllers only have 1 clock source per channel. However, on
+	  some variations of this controller, the internal card detection
+	  logic that exists in this controller is sectioned off to be run by a
+	  separate second clock source to allow the main core clock to be turned
+	  off to save power.
+	  If 2 clocks are specified by the hardware, you must name them as
+	  "core" and "cd". If the controller only has 1 clock, naming is not
+	  required.
+	  Below is the number clocks for each supported SoC:
+	   1: SH73A0, R8A73A4, R8A7740, R8A7778, R8A7779, R8A7790
+	      R8A7791, R8A7792, R8A7793, R8A7794, R8A7795, R8A7796
+	   2: R7S72100
+
 Optional properties:
 - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
 - pinctrl-names: should be "default", "state_uhs"
-- 
2.10.1

[PATCH v6 3/3] ARM: dts: r7s72100: update sdhi clock bindings

[Chris Brandt]

The SDHI controller in the RZ/A1 has 2 clock sources per channel and both
need to be enabled/disabled for proper operation. This fixes the fact that
the define for R7S72100_CLK_SDHI1 was not correct to begin with (typo), and
that all 4 clock sources need to be defined an used.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
* add missing clock sources instead of just fixing typo
* changed clock name from "carddetect" to "cd"
---
 arch/arm/boot/dts/r7s72100.dtsi            | 17 ++++++++++++-----
 include/dt-bindings/clock/r7s72100-clock.h |  6 ++++--
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index 3dd427d..9d0b8d0 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -153,9 +153,12 @@
 			#clock-cells = <1>;
 			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
 			reg = <0xfcfe0444 4>;
-			clocks = <&p1_clk>, <&p1_clk>;
-			clock-indices = <R7S72100_CLK_SDHI1 R7S72100_CLK_SDHI0>;
-			clock-output-names = "sdhi1", "sdhi0";
+			clocks = <&p1_clk>, <&p1_clk>, <&p1_clk>, <&p1_clk>;
+			clock-indices = <
+				R7S72100_CLK_SDHI00 R7S72100_CLK_SDHI01
+				R7S72100_CLK_SDHI10 R7S72100_CLK_SDHI11
+			>;
+			clock-output-names = "sdhi00", "sdhi01", "sdhi10", "sdhi11";
 		};
 	};
 
@@ -478,7 +481,9 @@
 			      GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH
 			      GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
 
-		clocks = <&mstp12_clks R7S72100_CLK_SDHI0>;
+		clocks = <&mstp12_clks R7S72100_CLK_SDHI00>,
+			 <&mstp12_clks R7S72100_CLK_SDHI01>;
+		clock-names = "core", "cd";
 		cap-sd-highspeed;
 		cap-sdio-irq;
 		status = "disabled";
@@ -491,7 +496,9 @@
 			      GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH
 			      GIC_SPI 275 IRQ_TYPE_LEVEL_HIGH>;
 
-		clocks = <&mstp12_clks R7S72100_CLK_SDHI1>;
+		clocks = <&mstp12_clks R7S72100_CLK_SDHI10>,
+			 <&mstp12_clks R7S72100_CLK_SDHI11>;
+		clock-names = "core", "cd";
 		cap-sd-highspeed;
 		cap-sdio-irq;
 		status = "disabled";
diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index 29e01ed..f2d8428 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -45,7 +45,9 @@
 #define R7S72100_CLK_SPI4	3
 
 /* MSTP12 */
-#define R7S72100_CLK_SDHI0	3
-#define R7S72100_CLK_SDHI1	2
+#define R7S72100_CLK_SDHI00	3
+#define R7S72100_CLK_SDHI01	2
+#define R7S72100_CLK_SDHI10	1
+#define R7S72100_CLK_SDHI11	0
 
 #endif /* __DT_BINDINGS_CLOCK_R7S72100_H__ */
-- 
2.10.1

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Ulf Hansson]

On 25 January 2017 at 21:28, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> At first this started out as a simple typo fix, until I realized
> that the SDHI in the RZ/A1 has 2 clocks per channel and both need
> to be turned on/off.
>
> This patch series adds the ability to specify 2 clocks instead of
> just 1, and does so for the RZ/A1 r7s72100.
>
> This patch has been tested on an RZ/A1 RSK board. Card detect works
> fine as well as bind/unbind.
>
> v6:
> * more detailed descritpion in tmio_mmc.txt
>
> v5:
> * Take implementation details out of DT documentation and put into
>   the driver code.
>
> v4:
> * No code changes, only text udpates to explain why there are 2 clocks
>   and why we need to treat them as 1 clock.
>
> Chris Brandt (3):
>   mmc: sh_mobile_sdhi: add support for 2 clocks
>   mmc: sh_mobile_sdhi: explain clock bindings
>   ARM: dts: r7s72100: update sdhi clock bindings
>
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 13 ++++++++++++
>  arch/arm/boot/dts/r7s72100.dtsi                    | 17 +++++++++++-----
>  drivers/mmc/host/sh_mobile_sdhi.c                  | 23 ++++++++++++++++++++++
>  include/dt-bindings/clock/r7s72100-clock.h         |  6 ++++--
>  4 files changed, 52 insertions(+), 7 deletions(-)
>
> --
> 2.10.1
>
>

Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
guess!? (And I can add Rob's ack afterwards).

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 237 bytes --]


> Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> guess!? (And I can add Rob's ack afterwards).

Can you add my tags as well. They got dropped somehow:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Simon Horman]

On Thu, Jan 26, 2017 at 09:07:45AM +0100, Ulf Hansson wrote:
> On 25 January 2017 at 21:28, Chris Brandt <chris.brandt@renesas.com> wrote:
> > At first this started out as a simple typo fix, until I realized
> > that the SDHI in the RZ/A1 has 2 clocks per channel and both need
> > to be turned on/off.
> >
> > This patch series adds the ability to specify 2 clocks instead of
> > just 1, and does so for the RZ/A1 r7s72100.
> >
> > This patch has been tested on an RZ/A1 RSK board. Card detect works
> > fine as well as bind/unbind.
> >
> > v6:
> > * more detailed descritpion in tmio_mmc.txt
> >
> > v5:
> > * Take implementation details out of DT documentation and put into
> >   the driver code.
> >
> > v4:
> > * No code changes, only text udpates to explain why there are 2 clocks
> >   and why we need to treat them as 1 clock.
> >
> > Chris Brandt (3):
> >   mmc: sh_mobile_sdhi: add support for 2 clocks
> >   mmc: sh_mobile_sdhi: explain clock bindings
> >   ARM: dts: r7s72100: update sdhi clock bindings
> >
> >  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 13 ++++++++++++
> >  arch/arm/boot/dts/r7s72100.dtsi                    | 17 +++++++++++-----
> >  drivers/mmc/host/sh_mobile_sdhi.c                  | 23 ++++++++++++++++++++++
> >  include/dt-bindings/clock/r7s72100-clock.h         |  6 ++++--
> >  4 files changed, 52 insertions(+), 7 deletions(-)
> >
> > --
> > 2.10.1
> >
> >
> 
> Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> guess!? (And I can add Rob's ack afterwards).

Thanks, I will take care of the 3rd patch.

Chris, is it safe to apply the 3rd patch without the first 2 patches
present?

RE: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Chris Brandt]

On Thursday, January 26, 2017, Wolfram Sang wrote:
> Subject: Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100
> clocks
> 
> 
> > Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> > guess!? (And I can add Rob's ack afterwards).
> 
> Can you add my tags as well. They got dropped somehow:
> 
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Sorry, that was my fault. It looks like you added that back on patch set
v3, but I forgot to add that when sending v4+.


Chris

RE: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Chris Brandt]

Hi Simon,

On Thursday, January 26, 2017, Simon Horman wrote:
> > Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> > guess!? (And I can add Rob's ack afterwards).
> 
> Thanks, I will take care of the 3rd patch.
> 
> Chris, is it safe to apply the 3rd patch without the first 2 patches
> present?

Yes, it is safe.

Chris

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Simon Horman]

On Thu, Jan 26, 2017 at 11:12:11AM +0100, Wolfram Sang wrote:
> 
> > Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> > guess!? (And I can add Rob's ack afterwards).
> 
> Can you add my tags as well. They got dropped somehow:
> 
> Reviewed-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

On Thu, Jan 26, 2017 at 12:53:10PM +0000, Chris Brandt wrote:
> Hi Simon,
> 
> On Thursday, January 26, 2017, Simon Horman wrote:
> > > Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
> > > guess!? (And I can add Rob's ack afterwards).
> > 
> > Thanks, I will take care of the 3rd patch.
> > 
> > Chris, is it safe to apply the 3rd patch without the first 2 patches
> > present?
> 
> Yes, it is safe.

Thanks, I have queued this up for v4.12 with Wolfram's tag.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/3] mmc: sh_mobile_sdhi: fix missing r7s72100 clocks

[Ulf Hansson]

On 26 January 2017 at 11:12, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Thanks, applied patch1 and patch2 for next. Patch3 is for Simon, I
>> guess!? (And I can add Rob's ack afterwards).
>
> Can you add my tags as well. They got dropped somehow:
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>

Done, thanks!

Kind regards
Uffe

Re: [PATCH v6 2/3] mmc: sh_mobile_sdhi: explain clock bindings

[Rob Herring]

On Wed, Jan 25, 2017 at 03:28:09PM -0500, Chris Brandt wrote:
> In the case of a single clock source, you don't need names. However,
> if the controller has 2 clock sources, you need to name them correctly
> so the driver can find the 2nd one. The 2nd clock is for the internal
> card detect logic.
> 
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> v5:
> * add Reviewed-by: Geert Uytterhoeven
> * list number of clocks for each SoC
> * remove example because it already exists in mmc.txt
> v4:
> * just explain there might be 2 clocks, don't explain how
>   we will use them in the driver
> v3:
> * add more clarification about why there are sometimes 2 clocks
>   and what you should do with them.
> * remove 'status = "disabled"' from example
> v2:
> * fix spelling and change wording
> * changed clock name from "carddetect" to "cd"
> ---
>  Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html