[PATCH 0/4] mmc: sunxi: Make sample clocks optional

[PATCH 0/4] mmc: sunxi: Make sample clocks optional

[Hans de Goede]

Hi All,

It turns out that sun4i / sun5i do not have sample clocks at all,
so make them optional. Also remove the DDR capability when there
are no sample clocks because DDR modes do not work reliably without
them (observed on an UTOO P66 tablet with eMMC).

Ulf, can you please merge the first 2 patches, patches 3-4 should
be merged through Maxime's sunxi/dts tree, but not before the first
2 have hit linux-next.

Regards,

Hans

[PATCH 1/4] mmc: sunxi: Disable sample clks on remove

[Hans de Goede]

When support for the sample clks was added calls to prepare_enable
were added to the probe path, but matching calls to disable_unprepare
were forgotten in the remove path, this fixes this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mmc/host/sunxi-mmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index a4d2b63..71a480b 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1160,6 +1160,8 @@ static int sunxi_mmc_remove(struct platform_device *pdev)
 	if (!IS_ERR(host->reset))
 		reset_control_assert(host->reset);
 
+	clk_disable_unprepare(host->clk_sample);
+	clk_disable_unprepare(host->clk_output);
 	clk_disable_unprepare(host->clk_mmc);
 	clk_disable_unprepare(host->clk_ahb);
 
-- 
2.7.4

[PATCH 2/4] mmc: sunxi: Make sample clocks optional

[Hans de Goede]

It turns out that sun4i (A10) and sun5i (A13 & co) do not have sample
clocks, so make them optional.

Since these do not have sample clocks, they cannot (reliably) do
DDR rates, so only set MMC_CAP_1_8V_DDR when we do have sample clks.

Note this patch only changes the devm_clk_get error checking and sets
the clocks to NULL if they don't exists. All the clk_foo calls accept
a NULL clk and will return success when called with a NULL clk, so this
is all that is necessary.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mmc/host/sunxi-mmc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 71a480b..4552ee0 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1016,14 +1016,20 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
 
 	host->clk_output = devm_clk_get(&pdev->dev, "output");
 	if (IS_ERR(host->clk_output)) {
-		dev_err(&pdev->dev, "Could not get output clock\n");
-		return PTR_ERR(host->clk_output);
+		if (PTR_ERR(host->clk_output) != -ENOENT) {
+			dev_err(&pdev->dev, "Could not get output clock\n");
+			return PTR_ERR(host->clk_output);
+		}
+		host->clk_output = NULL;
 	}
 
 	host->clk_sample = devm_clk_get(&pdev->dev, "sample");
 	if (IS_ERR(host->clk_sample)) {
-		dev_err(&pdev->dev, "Could not get sample clock\n");
-		return PTR_ERR(host->clk_sample);
+		if (PTR_ERR(host->clk_sample) != -ENOENT) {
+			dev_err(&pdev->dev, "Could not get sample clock\n");
+			return PTR_ERR(host->clk_sample);
+		}
+		host->clk_sample = NULL;
 	}
 
 	host->reset = devm_reset_control_get_optional(&pdev->dev, "ahb");
@@ -1126,9 +1132,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
 	mmc->f_min		=   400000;
 	mmc->f_max		= 52000000;
 	mmc->caps	       |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
-				  MMC_CAP_1_8V_DDR |
 				  MMC_CAP_ERASE | MMC_CAP_SDIO_IRQ;
 
+	if (host->clk_output && host->clk_sample)
+		mmc->caps      |= MMC_CAP_1_8V_DDR;
+
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto error_free_dma;
-- 
2.7.4

Re: [PATCH 2/4] mmc: sunxi: Make sample clocks optional

[Ulf Hansson]

On 16 July 2016 at 12:46, Hans de Goede <hdegoede@redhat.com> wrote:
> It turns out that sun4i (A10) and sun5i (A13 & co) do not have sample
> clocks, so make them optional.
>
> Since these do not have sample clocks, they cannot (reliably) do
> DDR rates, so only set MMC_CAP_1_8V_DDR when we do have sample clks.
>
> Note this patch only changes the devm_clk_get error checking and sets
> the clocks to NULL if they don't exists. All the clk_foo calls accept
> a NULL clk and will return success when called with a NULL clk, so this
> is all that is necessary.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mmc/host/sunxi-mmc.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 71a480b..4552ee0 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -1016,14 +1016,20 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
>
>         host->clk_output = devm_clk_get(&pdev->dev, "output");
>         if (IS_ERR(host->clk_output)) {
> -               dev_err(&pdev->dev, "Could not get output clock\n");
> -               return PTR_ERR(host->clk_output);
> +               if (PTR_ERR(host->clk_output) != -ENOENT) {

This is new to me for how to implement support for optional clocks.

Is this how the clock maintainer think it should be implemented?

> +                       dev_err(&pdev->dev, "Could not get output clock\n");
> +                       return PTR_ERR(host->clk_output);
> +               }
> +               host->clk_output = NULL;
>         }
>
>         host->clk_sample = devm_clk_get(&pdev->dev, "sample");
>         if (IS_ERR(host->clk_sample)) {
> -               dev_err(&pdev->dev, "Could not get sample clock\n");
> -               return PTR_ERR(host->clk_sample);
> +               if (PTR_ERR(host->clk_sample) != -ENOENT) {
> +                       dev_err(&pdev->dev, "Could not get sample clock\n");
> +                       return PTR_ERR(host->clk_sample);
> +               }
> +               host->clk_sample = NULL;
>         }
>
>         host->reset = devm_reset_control_get_optional(&pdev->dev, "ahb");
> @@ -1126,9 +1132,11 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
>         mmc->f_min              =   400000;
>         mmc->f_max              = 52000000;
>         mmc->caps              |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> -                                 MMC_CAP_1_8V_DDR |
>                                   MMC_CAP_ERASE | MMC_CAP_SDIO_IRQ;
>
> +       if (host->clk_output && host->clk_sample)
> +               mmc->caps      |= MMC_CAP_1_8V_DDR;
> +
>         ret = mmc_of_parse(mmc);
>         if (ret)
>                 goto error_free_dma;
> --
> 2.7.4
>

Kind regards
Uffe

Re: [PATCH 2/4] mmc: sunxi: Make sample clocks optional

[Maxime Ripard]

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

Hi,

On Sat, Jul 16, 2016 at 12:46:02PM +0200, Hans de Goede wrote:
> It turns out that sun4i (A10) and sun5i (A13 & co) do not have sample
> clocks, so make them optional.
> 
> Since these do not have sample clocks, they cannot (reliably) do
> DDR rates, so only set MMC_CAP_1_8V_DDR when we do have sample clks.
> 
> Note this patch only changes the devm_clk_get error checking and sets
> the clocks to NULL if they don't exists. All the clk_foo calls accept
> a NULL clk and will return success when called with a NULL clk, so this
> is all that is necessary.

The clocks aren't really optional. They're not needed for the A10 /
A13, and mandatory for the A20 and later.

Having a new compatible for the A20 that would require that clock and
not require it for the A10/A13 anymore seems more appropriate.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH 2/4] mmc: sunxi: Make sample clocks optional

[Hans de Goede]

Hi,

On 25-07-16 10:18, Maxime Ripard wrote:
> Hi,
>
> On Sat, Jul 16, 2016 at 12:46:02PM +0200, Hans de Goede wrote:
>> It turns out that sun4i (A10) and sun5i (A13 & co) do not have sample
>> clocks, so make them optional.
>>
>> Since these do not have sample clocks, they cannot (reliably) do
>> DDR rates, so only set MMC_CAP_1_8V_DDR when we do have sample clks.
>>
>> Note this patch only changes the devm_clk_get error checking and sets
>> the clocks to NULL if they don't exists. All the clk_foo calls accept
>> a NULL clk and will return success when called with a NULL clk, so this
>> is all that is necessary.
>
> The clocks aren't really optional. They're not needed for the A10 /
> A13, and mandatory for the A20 and later.
>
> Having a new compatible for the A20 that would require that clock and
> not require it for the A10/A13 anymore seems more appropriate.

Ok, I've prepared a new version using a new compatible.

Regards,

Hans
--
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

[PATCH 3/4] ARM: dts: sun4i: Remove sample clocks from mmc host nodes

[Hans de Goede]

It turns out the A10 does not have mmc sample clocks (the bits for
this in the register always read back 0, not listed in the datasheet),
so remove the sample clocks from the mmc host nodes.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 7e7dfc2..f6bf851 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -783,13 +783,9 @@
 			compatible = "allwinner,sun4i-a10-mmc";
 			reg = <0x01c0f000 0x1000>;
 			clocks = <&ahb_gates 8>,
-				 <&mmc0_clk 0>,
-				 <&mmc0_clk 1>,
-				 <&mmc0_clk 2>;
+				 <&mmc0_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <32>;
 			status = "disabled";
 			#address-cells = <1>;
@@ -800,13 +796,9 @@
 			compatible = "allwinner,sun4i-a10-mmc";
 			reg = <0x01c10000 0x1000>;
 			clocks = <&ahb_gates 9>,
-				 <&mmc1_clk 0>,
-				 <&mmc1_clk 1>,
-				 <&mmc1_clk 2>;
+				 <&mmc1_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <33>;
 			status = "disabled";
 			#address-cells = <1>;
@@ -817,13 +809,9 @@
 			compatible = "allwinner,sun4i-a10-mmc";
 			reg = <0x01c11000 0x1000>;
 			clocks = <&ahb_gates 10>,
-				 <&mmc2_clk 0>,
-				 <&mmc2_clk 1>,
-				 <&mmc2_clk 2>;
+				 <&mmc2_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <34>;
 			status = "disabled";
 			#address-cells = <1>;
@@ -834,13 +822,9 @@
 			compatible = "allwinner,sun4i-a10-mmc";
 			reg = <0x01c12000 0x1000>;
 			clocks = <&ahb_gates 11>,
-				 <&mmc3_clk 0>,
-				 <&mmc3_clk 1>,
-				 <&mmc3_clk 2>;
+				 <&mmc3_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <35>;
 			status = "disabled";
 			#address-cells = <1>;
-- 
2.7.4

[PATCH 4/4] ARM: dts: sun5i: Remove sample clocks from mmc host nodes

[Hans de Goede]

It turns out that sun5i does not have mmc sample clocks (the bits for
this in the register always read back 0, not listed in the datasheet),
so remove the sample clocks from the mmc host nodes.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/sun5i.dtsi | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index e374f4f..86044f9 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -431,13 +431,9 @@
 			compatible = "allwinner,sun5i-a13-mmc";
 			reg = <0x01c0f000 0x1000>;
 			clocks = <&ahb_gates 8>,
-				 <&mmc0_clk 0>,
-				 <&mmc0_clk 1>,
-				 <&mmc0_clk 2>;
+				 <&mmc0_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <32>;
 			status = "disabled";
 			#address-cells = <1>;
@@ -448,13 +444,9 @@
 			compatible = "allwinner,sun5i-a13-mmc";
 			reg = <0x01c10000 0x1000>;
 			clocks = <&ahb_gates 9>,
-				 <&mmc1_clk 0>,
-				 <&mmc1_clk 1>,
-				 <&mmc1_clk 2>;
+				 <&mmc1_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <33>;
 			status = "disabled";
 			#address-cells = <1>;
@@ -465,13 +457,9 @@
 			compatible = "allwinner,sun5i-a13-mmc";
 			reg = <0x01c11000 0x1000>;
 			clocks = <&ahb_gates 10>,
-				 <&mmc2_clk 0>,
-				 <&mmc2_clk 1>,
-				 <&mmc2_clk 2>;
+				 <&mmc2_clk 0>;
 			clock-names = "ahb",
-				      "mmc",
-				      "output",
-				      "sample";
+				      "mmc";
 			interrupts = <34>;
 			status = "disabled";
 			#address-cells = <1>;
-- 
2.7.4

--
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 0/4] mmc: sunxi: Make sample clocks optional

[Icenowy Zheng]

16.07.2016, 18:50, "Hans de Goede" <hdegoede@redhat.com>:
> Hi All,
>
> It turns out that sun4i / sun5i do not have sample clocks at all,
> so make them optional. Also remove the DDR capability when there
> are no sample clocks because DDR modes do not work reliably without
> them (observed on an UTOO P66 tablet with eMMC).
>
> Ulf, can you please merge the first 2 patches, patches 3-4 should
> be merged through Maxime's sunxi/dts tree, but not before the first
> 2 have hit linux-next.
>
> Regards,
>
> Hans
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Interesting...

A64 have also no external sample/output clock... (The sample/output clock is integrated into MMC IP core now)