Linux-mmc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
@ 2019-11-28  7:45 Ludovic Desroches
  2019-11-28  7:45 ` [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device Ludovic Desroches
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ludovic Desroches @ 2019-11-28  7:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, nicolas.ferre, adrian.hunter, linux-kernel, robh+dt,
	mark.rutland, claudiu.beznea, Eugen.Hristev, alexandre.belloni,
	Ludovic Desroches, Rob Herring

There is a new compatible string for the SAM9X60 sdhci device. It involves
an update of the properties about the clocks stuff.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Changes:
- v3: rebase due to conflict with Nicolas' patch
- v2: remove the extra example and fix node label

 .../devicetree/bindings/mmc/sdhci-atmel.txt         | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
index 503c6dbac1b2..69edfd4d3922 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
@@ -5,11 +5,16 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the properties used by the
 sdhci-of-at91 driver.
 
 Required properties:
-- compatible:		Must be "atmel,sama5d2-sdhci".
+- compatible:		Must be "atmel,sama5d2-sdhci" or "microchip,sam9x60-sdhci".
 - clocks:		Phandlers to the clocks.
-- clock-names:		Must be "hclock", "multclk", "baseclk";
+- clock-names:		Must be "hclock", "multclk", "baseclk" for
+			"atmel,sama5d2-sdhci".
+			Must be "hclock", "multclk" for "microchip,sam9x60-sdhci".
 
 Optional properties:
+- assigned-clocks:	The same with "multclk".
+- assigned-clock-rates	The rate of "multclk" in order to not rely on the
+			gck configuration set by previous components.
 - microchip,sdcal-inverted: when present, polarity on the SDCAL SoC pin is
   inverted. The default polarity for this signal is described in the datasheet.
   For instance on SAMA5D2, the pin is usually tied to the GND with a resistor
@@ -17,10 +22,12 @@ Optional properties:
 
 Example:
 
-sdmmc0: sdio-host@a0000000 {
+mmc0: sdio-host@a0000000 {
 	compatible = "atmel,sama5d2-sdhci";
 	reg = <0xa0000000 0x300>;
 	interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
 	clocks = <&sdmmc0_hclk>, <&sdmmc0_gclk>, <&main>;
 	clock-names = "hclock", "multclk", "baseclk";
+	assigned-clocks = <&sdmmc0_gclk>;
+	assigned-clock-rates = <480000000>;
 };
-- 
2.24.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device
  2019-11-28  7:45 [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ludovic Desroches
@ 2019-11-28  7:45 ` Ludovic Desroches
  2019-12-10  9:50   ` Ulf Hansson
  2019-11-28  7:45 ` [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency Ludovic Desroches
  2019-12-10  9:50 ` [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ulf Hansson
  2 siblings, 1 reply; 6+ messages in thread
From: Ludovic Desroches @ 2019-11-28  7:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, nicolas.ferre, adrian.hunter, linux-kernel, robh+dt,
	mark.rutland, claudiu.beznea, Eugen.Hristev, alexandre.belloni,
	Ludovic Desroches

In the SAM9x60 SoC, there are only two clocks instead of three for the
SDHCI device. The base clk is no longer provided, it is generated
internally from the mult clk.

The values of the base clk and mul in the capabilities registers may not
reflect the reality as the mult clk is a programmable clock which can take
several rates. As we can't trust those values, take them from the clock
tree and update the capabilities according to.

As we can have the same pitfall, in some cases, with the SAMA5D2 Soc,
stop relying on capabilities too.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---

Changes:
- v4: fix typo for the capabilities register and remove extra
  parentheses
- v3: none
- v2: none

 drivers/mmc/host/sdhci-of-at91.c | 105 +++++++++++++++++--------------
 1 file changed, 58 insertions(+), 47 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 5959e394b416..b2a8c45c9c23 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -33,7 +33,14 @@
 
 #define SDHCI_AT91_PRESET_COMMON_CONF	0x400 /* drv type B, programmable clock mode */
 
+struct sdhci_at91_soc_data {
+	const struct sdhci_pltfm_data *pdata;
+	bool baseclk_is_generated_internally;
+	unsigned int divider_for_baseclk;
+};
+
 struct sdhci_at91_priv {
+	const struct sdhci_at91_soc_data *soc_data;
 	struct clk *hclock;
 	struct clk *gck;
 	struct clk *mainck;
@@ -141,12 +148,24 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
 	.set_power		= sdhci_at91_set_power,
 };
 
-static const struct sdhci_pltfm_data soc_data_sama5d2 = {
+static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
 	.ops = &sdhci_at91_sama5d2_ops,
 };
 
+static const struct sdhci_at91_soc_data soc_data_sama5d2 = {
+	.pdata = &sdhci_sama5d2_pdata,
+	.baseclk_is_generated_internally = false,
+};
+
+static const struct sdhci_at91_soc_data soc_data_sam9x60 = {
+	.pdata = &sdhci_sama5d2_pdata,
+	.baseclk_is_generated_internally = true,
+	.divider_for_baseclk = 2,
+};
+
 static const struct of_device_id sdhci_at91_dt_match[] = {
 	{ .compatible = "atmel,sama5d2-sdhci", .data = &soc_data_sama5d2 },
+	{ .compatible = "microchip,sam9x60-sdhci", .data = &soc_data_sam9x60 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
@@ -156,50 +175,37 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
 	struct sdhci_host *host = dev_get_drvdata(dev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
-	int ret;
 	unsigned int			caps0, caps1;
 	unsigned int			clk_base, clk_mul;
-	unsigned int			gck_rate, real_gck_rate;
+	unsigned int			gck_rate, clk_base_rate;
 	unsigned int			preset_div;
 
-	/*
-	 * The mult clock is provided by as a generated clock by the PMC
-	 * controller. In order to set the rate of gck, we have to get the
-	 * base clock rate and the clock mult from capabilities.
-	 */
 	clk_prepare_enable(priv->hclock);
 	caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
 	caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
-	clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
-	clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
-	gck_rate = clk_base * 1000000 * (clk_mul + 1);
-	ret = clk_set_rate(priv->gck, gck_rate);
-	if (ret < 0) {
-		dev_err(dev, "failed to set gck");
-		clk_disable_unprepare(priv->hclock);
-		return ret;
-	}
-	/*
-	 * We need to check if we have the requested rate for gck because in
-	 * some cases this rate could be not supported. If it happens, the rate
-	 * is the closest one gck can provide. We have to update the value
-	 * of clk mul.
-	 */
-	real_gck_rate = clk_get_rate(priv->gck);
-	if (real_gck_rate != gck_rate) {
-		clk_mul = real_gck_rate / (clk_base * 1000000) - 1;
-		caps1 &= (~SDHCI_CLOCK_MUL_MASK);
-		caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) &
-			  SDHCI_CLOCK_MUL_MASK);
-		/* Set capabilities in r/w mode. */
-		writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN,
-		       host->ioaddr + SDMMC_CACR);
-		writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
-		/* Set capabilities in ro mode. */
-		writel(0, host->ioaddr + SDMMC_CACR);
-		dev_info(dev, "update clk mul to %u as gck rate is %u Hz\n",
-			 clk_mul, real_gck_rate);
-	}
+
+	gck_rate = clk_get_rate(priv->gck);
+	if (priv->soc_data->baseclk_is_generated_internally)
+		clk_base_rate = gck_rate / priv->soc_data->divider_for_baseclk;
+	else
+		clk_base_rate = clk_get_rate(priv->mainck);
+
+	clk_base = clk_base_rate / 1000000;
+	clk_mul = gck_rate / clk_base_rate - 1;
+
+	caps0 &= ~SDHCI_CLOCK_V3_BASE_MASK;
+	caps0 |= (clk_base << SDHCI_CLOCK_BASE_SHIFT) & SDHCI_CLOCK_V3_BASE_MASK;
+	caps1 &= ~SDHCI_CLOCK_MUL_MASK;
+	caps1 |= (clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK;
+	/* Set capabilities in r/w mode. */
+	writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR);
+	writel(caps0, host->ioaddr + SDHCI_CAPABILITIES);
+	writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
+	/* Set capabilities in ro mode. */
+	writel(0, host->ioaddr + SDMMC_CACR);
+
+	dev_info(dev, "update clk mul to %u as gck rate is %u Hz and clk base is %u Hz\n",
+		 clk_mul, gck_rate, clk_base_rate);
 
 	/*
 	 * We have to set preset values because it depends on the clk_mul
@@ -207,19 +213,19 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
 	 * maximum sd clock value is 120 MHz instead of 208 MHz. For that
 	 * reason, we need to use presets to support SDR104.
 	 */
-	preset_div = DIV_ROUND_UP(real_gck_rate, 24000000) - 1;
+	preset_div = DIV_ROUND_UP(gck_rate, 24000000) - 1;
 	writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
 	       host->ioaddr + SDHCI_PRESET_FOR_SDR12);
-	preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1;
+	preset_div = DIV_ROUND_UP(gck_rate, 50000000) - 1;
 	writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
 	       host->ioaddr + SDHCI_PRESET_FOR_SDR25);
-	preset_div = DIV_ROUND_UP(real_gck_rate, 100000000) - 1;
+	preset_div = DIV_ROUND_UP(gck_rate, 100000000) - 1;
 	writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
 	       host->ioaddr + SDHCI_PRESET_FOR_SDR50);
-	preset_div = DIV_ROUND_UP(real_gck_rate, 120000000) - 1;
+	preset_div = DIV_ROUND_UP(gck_rate, 120000000) - 1;
 	writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
 	       host->ioaddr + SDHCI_PRESET_FOR_SDR104);
-	preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1;
+	preset_div = DIV_ROUND_UP(gck_rate, 50000000) - 1;
 	writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
 	       host->ioaddr + SDHCI_PRESET_FOR_DDR50);
 
@@ -314,7 +320,7 @@ static const struct dev_pm_ops sdhci_at91_dev_pm_ops = {
 static int sdhci_at91_probe(struct platform_device *pdev)
 {
 	const struct of_device_id	*match;
-	const struct sdhci_pltfm_data	*soc_data;
+	const struct sdhci_at91_soc_data	*soc_data;
 	struct sdhci_host		*host;
 	struct sdhci_pltfm_host		*pltfm_host;
 	struct sdhci_at91_priv		*priv;
@@ -325,17 +331,22 @@ static int sdhci_at91_probe(struct platform_device *pdev)
 		return -EINVAL;
 	soc_data = match->data;
 
-	host = sdhci_pltfm_init(pdev, soc_data, sizeof(*priv));
+	host = sdhci_pltfm_init(pdev, soc_data->pdata, sizeof(*priv));
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
 	pltfm_host = sdhci_priv(host);
 	priv = sdhci_pltfm_priv(pltfm_host);
+	priv->soc_data = soc_data;
 
 	priv->mainck = devm_clk_get(&pdev->dev, "baseclk");
 	if (IS_ERR(priv->mainck)) {
-		dev_err(&pdev->dev, "failed to get baseclk\n");
-		return PTR_ERR(priv->mainck);
+		if (soc_data->baseclk_is_generated_internally) {
+			priv->mainck = NULL;
+		} else {
+			dev_err(&pdev->dev, "failed to get baseclk\n");
+			return PTR_ERR(priv->mainck);
+		}
 	}
 
 	priv->hclock = devm_clk_get(&pdev->dev, "hclock");
-- 
2.24.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency
  2019-11-28  7:45 [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ludovic Desroches
  2019-11-28  7:45 ` [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device Ludovic Desroches
@ 2019-11-28  7:45 ` Ludovic Desroches
  2019-12-09 23:43   ` Alexandre Belloni
  2019-12-10  9:50 ` [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ulf Hansson
  2 siblings, 1 reply; 6+ messages in thread
From: Ludovic Desroches @ 2019-11-28  7:45 UTC (permalink / raw)
  To: linux-arm-kernel, linux-mmc, devicetree
  Cc: ulf.hansson, nicolas.ferre, adrian.hunter, linux-kernel, robh+dt,
	mark.rutland, claudiu.beznea, Eugen.Hristev, alexandre.belloni,
	Ludovic Desroches

Set the frequency of the generated clock used by sdmmc devices in order
to not rely on the configuration done by previous components.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---

Changes:
- v4: none
- v3: none
- v2: none

 arch/arm/boot/dts/sama5d2.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 565204816e34..7665263af907 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -300,6 +300,8 @@ sdmmc0: sdio-host@a0000000 {
 			interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 31>, <&pmc PMC_TYPE_GCK 31>, <&pmc PMC_TYPE_CORE PMC_MAIN>;
 			clock-names = "hclock", "multclk", "baseclk";
+			assigned-clocks = <&pmc PMC_TYPE_GCK 31>;
+			assigned-clock-rates = <480000000>;
 			status = "disabled";
 		};
 
@@ -309,6 +311,8 @@ sdmmc1: sdio-host@b0000000 {
 			interrupts = <32 IRQ_TYPE_LEVEL_HIGH 0>;
 			clocks = <&pmc PMC_TYPE_PERIPHERAL 32>, <&pmc PMC_TYPE_GCK 32>, <&pmc PMC_TYPE_CORE PMC_MAIN>;
 			clock-names = "hclock", "multclk", "baseclk";
+			assigned-clocks = <&pmc PMC_TYPE_GCK 32>;
+			assigned-clock-rates = <480000000>;
 			status = "disabled";
 		};
 
-- 
2.24.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency
  2019-11-28  7:45 ` [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency Ludovic Desroches
@ 2019-12-09 23:43   ` Alexandre Belloni
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2019-12-09 23:43 UTC (permalink / raw)
  To: Ludovic Desroches
  Cc: linux-arm-kernel, linux-mmc, devicetree, ulf.hansson,
	nicolas.ferre, adrian.hunter, linux-kernel, robh+dt,
	mark.rutland, claudiu.beznea, Eugen.Hristev

On 28/11/2019 08:45:22+0100, Ludovic Desroches wrote:
> Set the frequency of the generated clock used by sdmmc devices in order
> to not rely on the configuration done by previous components.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> 
> Changes:
> - v4: none
> - v3: none
> - v2: none
> 
>  arch/arm/boot/dts/sama5d2.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties
  2019-11-28  7:45 [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ludovic Desroches
  2019-11-28  7:45 ` [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device Ludovic Desroches
  2019-11-28  7:45 ` [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency Ludovic Desroches
@ 2019-12-10  9:50 ` Ulf Hansson
  2 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-12-10  9:50 UTC (permalink / raw)
  To: Ludovic Desroches
  Cc: Linux ARM, linux-mmc, DTML, Nicolas Ferre, Adrian Hunter,
	Linux Kernel Mailing List, Rob Herring, Mark Rutland,
	claudiu.beznea, Eugen.Hristev, Alexandre Belloni, Rob Herring

On Thu, 28 Nov 2019 at 08:45, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
>
> There is a new compatible string for the SAM9X60 sdhci device. It involves
> an update of the properties about the clocks stuff.
>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied for next, thanks!

Kind regards
Uffe


> ---
>
> Changes:
> - v3: rebase due to conflict with Nicolas' patch
> - v2: remove the extra example and fix node label
>
>  .../devicetree/bindings/mmc/sdhci-atmel.txt         | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> index 503c6dbac1b2..69edfd4d3922 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
> @@ -5,11 +5,16 @@ Documentation/devicetree/bindings/mmc/mmc.txt and the properties used by the
>  sdhci-of-at91 driver.
>
>  Required properties:
> -- compatible:          Must be "atmel,sama5d2-sdhci".
> +- compatible:          Must be "atmel,sama5d2-sdhci" or "microchip,sam9x60-sdhci".
>  - clocks:              Phandlers to the clocks.
> -- clock-names:         Must be "hclock", "multclk", "baseclk";
> +- clock-names:         Must be "hclock", "multclk", "baseclk" for
> +                       "atmel,sama5d2-sdhci".
> +                       Must be "hclock", "multclk" for "microchip,sam9x60-sdhci".
>
>  Optional properties:
> +- assigned-clocks:     The same with "multclk".
> +- assigned-clock-rates The rate of "multclk" in order to not rely on the
> +                       gck configuration set by previous components.
>  - microchip,sdcal-inverted: when present, polarity on the SDCAL SoC pin is
>    inverted. The default polarity for this signal is described in the datasheet.
>    For instance on SAMA5D2, the pin is usually tied to the GND with a resistor
> @@ -17,10 +22,12 @@ Optional properties:
>
>  Example:
>
> -sdmmc0: sdio-host@a0000000 {
> +mmc0: sdio-host@a0000000 {
>         compatible = "atmel,sama5d2-sdhci";
>         reg = <0xa0000000 0x300>;
>         interrupts = <31 IRQ_TYPE_LEVEL_HIGH 0>;
>         clocks = <&sdmmc0_hclk>, <&sdmmc0_gclk>, <&main>;
>         clock-names = "hclock", "multclk", "baseclk";
> +       assigned-clocks = <&sdmmc0_gclk>;
> +       assigned-clock-rates = <480000000>;
>  };
> --
> 2.24.0
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device
  2019-11-28  7:45 ` [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device Ludovic Desroches
@ 2019-12-10  9:50   ` Ulf Hansson
  0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-12-10  9:50 UTC (permalink / raw)
  To: Ludovic Desroches
  Cc: Linux ARM, linux-mmc, DTML, Nicolas Ferre, Adrian Hunter,
	Linux Kernel Mailing List, Rob Herring, Mark Rutland,
	claudiu.beznea, Eugen.Hristev, Alexandre Belloni

On Thu, 28 Nov 2019 at 08:45, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
>
> In the SAM9x60 SoC, there are only two clocks instead of three for the
> SDHCI device. The base clk is no longer provided, it is generated
> internally from the mult clk.
>
> The values of the base clk and mul in the capabilities registers may not
> reflect the reality as the mult clk is a programmable clock which can take
> several rates. As we can't trust those values, take them from the clock
> tree and update the capabilities according to.
>
> As we can have the same pitfall, in some cases, with the SAMA5D2 Soc,
> stop relying on capabilities too.
>
> Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>


Applied for next, thanks!

Kind regards
Uffe


> ---
>
> Changes:
> - v4: fix typo for the capabilities register and remove extra
>   parentheses
> - v3: none
> - v2: none
>
>  drivers/mmc/host/sdhci-of-at91.c | 105 +++++++++++++++++--------------
>  1 file changed, 58 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 5959e394b416..b2a8c45c9c23 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -33,7 +33,14 @@
>
>  #define SDHCI_AT91_PRESET_COMMON_CONF  0x400 /* drv type B, programmable clock mode */
>
> +struct sdhci_at91_soc_data {
> +       const struct sdhci_pltfm_data *pdata;
> +       bool baseclk_is_generated_internally;
> +       unsigned int divider_for_baseclk;
> +};
> +
>  struct sdhci_at91_priv {
> +       const struct sdhci_at91_soc_data *soc_data;
>         struct clk *hclock;
>         struct clk *gck;
>         struct clk *mainck;
> @@ -141,12 +148,24 @@ static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>         .set_power              = sdhci_at91_set_power,
>  };
>
> -static const struct sdhci_pltfm_data soc_data_sama5d2 = {
> +static const struct sdhci_pltfm_data sdhci_sama5d2_pdata = {
>         .ops = &sdhci_at91_sama5d2_ops,
>  };
>
> +static const struct sdhci_at91_soc_data soc_data_sama5d2 = {
> +       .pdata = &sdhci_sama5d2_pdata,
> +       .baseclk_is_generated_internally = false,
> +};
> +
> +static const struct sdhci_at91_soc_data soc_data_sam9x60 = {
> +       .pdata = &sdhci_sama5d2_pdata,
> +       .baseclk_is_generated_internally = true,
> +       .divider_for_baseclk = 2,
> +};
> +
>  static const struct of_device_id sdhci_at91_dt_match[] = {
>         { .compatible = "atmel,sama5d2-sdhci", .data = &soc_data_sama5d2 },
> +       { .compatible = "microchip,sam9x60-sdhci", .data = &soc_data_sam9x60 },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, sdhci_at91_dt_match);
> @@ -156,50 +175,37 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
>         struct sdhci_host *host = dev_get_drvdata(dev);
>         struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>         struct sdhci_at91_priv *priv = sdhci_pltfm_priv(pltfm_host);
> -       int ret;
>         unsigned int                    caps0, caps1;
>         unsigned int                    clk_base, clk_mul;
> -       unsigned int                    gck_rate, real_gck_rate;
> +       unsigned int                    gck_rate, clk_base_rate;
>         unsigned int                    preset_div;
>
> -       /*
> -        * The mult clock is provided by as a generated clock by the PMC
> -        * controller. In order to set the rate of gck, we have to get the
> -        * base clock rate and the clock mult from capabilities.
> -        */
>         clk_prepare_enable(priv->hclock);
>         caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
>         caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
> -       clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
> -       clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
> -       gck_rate = clk_base * 1000000 * (clk_mul + 1);
> -       ret = clk_set_rate(priv->gck, gck_rate);
> -       if (ret < 0) {
> -               dev_err(dev, "failed to set gck");
> -               clk_disable_unprepare(priv->hclock);
> -               return ret;
> -       }
> -       /*
> -        * We need to check if we have the requested rate for gck because in
> -        * some cases this rate could be not supported. If it happens, the rate
> -        * is the closest one gck can provide. We have to update the value
> -        * of clk mul.
> -        */
> -       real_gck_rate = clk_get_rate(priv->gck);
> -       if (real_gck_rate != gck_rate) {
> -               clk_mul = real_gck_rate / (clk_base * 1000000) - 1;
> -               caps1 &= (~SDHCI_CLOCK_MUL_MASK);
> -               caps1 |= ((clk_mul << SDHCI_CLOCK_MUL_SHIFT) &
> -                         SDHCI_CLOCK_MUL_MASK);
> -               /* Set capabilities in r/w mode. */
> -               writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN,
> -                      host->ioaddr + SDMMC_CACR);
> -               writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
> -               /* Set capabilities in ro mode. */
> -               writel(0, host->ioaddr + SDMMC_CACR);
> -               dev_info(dev, "update clk mul to %u as gck rate is %u Hz\n",
> -                        clk_mul, real_gck_rate);
> -       }
> +
> +       gck_rate = clk_get_rate(priv->gck);
> +       if (priv->soc_data->baseclk_is_generated_internally)
> +               clk_base_rate = gck_rate / priv->soc_data->divider_for_baseclk;
> +       else
> +               clk_base_rate = clk_get_rate(priv->mainck);
> +
> +       clk_base = clk_base_rate / 1000000;
> +       clk_mul = gck_rate / clk_base_rate - 1;
> +
> +       caps0 &= ~SDHCI_CLOCK_V3_BASE_MASK;
> +       caps0 |= (clk_base << SDHCI_CLOCK_BASE_SHIFT) & SDHCI_CLOCK_V3_BASE_MASK;
> +       caps1 &= ~SDHCI_CLOCK_MUL_MASK;
> +       caps1 |= (clk_mul << SDHCI_CLOCK_MUL_SHIFT) & SDHCI_CLOCK_MUL_MASK;
> +       /* Set capabilities in r/w mode. */
> +       writel(SDMMC_CACR_KEY | SDMMC_CACR_CAPWREN, host->ioaddr + SDMMC_CACR);
> +       writel(caps0, host->ioaddr + SDHCI_CAPABILITIES);
> +       writel(caps1, host->ioaddr + SDHCI_CAPABILITIES_1);
> +       /* Set capabilities in ro mode. */
> +       writel(0, host->ioaddr + SDMMC_CACR);
> +
> +       dev_info(dev, "update clk mul to %u as gck rate is %u Hz and clk base is %u Hz\n",
> +                clk_mul, gck_rate, clk_base_rate);
>
>         /*
>          * We have to set preset values because it depends on the clk_mul
> @@ -207,19 +213,19 @@ static int sdhci_at91_set_clks_presets(struct device *dev)
>          * maximum sd clock value is 120 MHz instead of 208 MHz. For that
>          * reason, we need to use presets to support SDR104.
>          */
> -       preset_div = DIV_ROUND_UP(real_gck_rate, 24000000) - 1;
> +       preset_div = DIV_ROUND_UP(gck_rate, 24000000) - 1;
>         writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                host->ioaddr + SDHCI_PRESET_FOR_SDR12);
> -       preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1;
> +       preset_div = DIV_ROUND_UP(gck_rate, 50000000) - 1;
>         writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                host->ioaddr + SDHCI_PRESET_FOR_SDR25);
> -       preset_div = DIV_ROUND_UP(real_gck_rate, 100000000) - 1;
> +       preset_div = DIV_ROUND_UP(gck_rate, 100000000) - 1;
>         writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                host->ioaddr + SDHCI_PRESET_FOR_SDR50);
> -       preset_div = DIV_ROUND_UP(real_gck_rate, 120000000) - 1;
> +       preset_div = DIV_ROUND_UP(gck_rate, 120000000) - 1;
>         writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                host->ioaddr + SDHCI_PRESET_FOR_SDR104);
> -       preset_div = DIV_ROUND_UP(real_gck_rate, 50000000) - 1;
> +       preset_div = DIV_ROUND_UP(gck_rate, 50000000) - 1;
>         writew(SDHCI_AT91_PRESET_COMMON_CONF | preset_div,
>                host->ioaddr + SDHCI_PRESET_FOR_DDR50);
>
> @@ -314,7 +320,7 @@ static const struct dev_pm_ops sdhci_at91_dev_pm_ops = {
>  static int sdhci_at91_probe(struct platform_device *pdev)
>  {
>         const struct of_device_id       *match;
> -       const struct sdhci_pltfm_data   *soc_data;
> +       const struct sdhci_at91_soc_data        *soc_data;
>         struct sdhci_host               *host;
>         struct sdhci_pltfm_host         *pltfm_host;
>         struct sdhci_at91_priv          *priv;
> @@ -325,17 +331,22 @@ static int sdhci_at91_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         soc_data = match->data;
>
> -       host = sdhci_pltfm_init(pdev, soc_data, sizeof(*priv));
> +       host = sdhci_pltfm_init(pdev, soc_data->pdata, sizeof(*priv));
>         if (IS_ERR(host))
>                 return PTR_ERR(host);
>
>         pltfm_host = sdhci_priv(host);
>         priv = sdhci_pltfm_priv(pltfm_host);
> +       priv->soc_data = soc_data;
>
>         priv->mainck = devm_clk_get(&pdev->dev, "baseclk");
>         if (IS_ERR(priv->mainck)) {
> -               dev_err(&pdev->dev, "failed to get baseclk\n");
> -               return PTR_ERR(priv->mainck);
> +               if (soc_data->baseclk_is_generated_internally) {
> +                       priv->mainck = NULL;
> +               } else {
> +                       dev_err(&pdev->dev, "failed to get baseclk\n");
> +                       return PTR_ERR(priv->mainck);
> +               }
>         }
>
>         priv->hclock = devm_clk_get(&pdev->dev, "hclock");
> --
> 2.24.0
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28  7:45 [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ludovic Desroches
2019-11-28  7:45 ` [PATCH v4 2/3] mmc: sdhci-of-at91: rework clocks management to support SAM9x60 device Ludovic Desroches
2019-12-10  9:50   ` Ulf Hansson
2019-11-28  7:45 ` [PATCH v4 3/3] ARM: dts: at91: sama5d2: set the sdmmc gclk frequency Ludovic Desroches
2019-12-09 23:43   ` Alexandre Belloni
2019-12-10  9:50 ` [PATCH v4 1/3] dt-bindings: sdhci-of-at91: new compatible string and update properties Ulf Hansson

Linux-mmc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mmc/0 linux-mmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mmc linux-mmc/ https://lore.kernel.org/linux-mmc \
		linux-mmc@vger.kernel.org
	public-inbox-index linux-mmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git