[PATCH 0/2] mmc: sdhci-brcmstb: Add support for optional sdio_freq clock

[PATCH 0/2] mmc: sdhci-brcmstb: Add support for optional sdio_freq clock

[Kamal Dasu]

Sending the remaining 2 patches separately after implementing review comments
for the patches 3/4 and 4/4 as part of the following:
"mmc: sdhci-brcmstb: host controller clock enhancements"
https://lore.kernel.org/linux-arm-kernel/96fd3054-17b1-db42-9a44-a60485243807@linaro.org/t/
changes are rebased over
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git refs/heads/next

tested brcm,sdhci-brcmstb.yaml and driver with and without sdio_freq clock.

Al Cooper (1):
  mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

Kamal Dasu (1):
  dt-bindings: mmc: Add Broadcom optional sdio_freq clock

 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 25 ++++++++++++----
 drivers/mmc/host/sdhci-brcmstb.c              | 30 +++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH 0/2] mmc: sdhci-brcmstb: Add support for optional sdio_freq clock

[Kamal Dasu]

Sending the remaining 2 patches separately after implementing review comments
for the patches 3/4 and 4/4 as part of the following:
"mmc: sdhci-brcmstb: host controller clock enhancements"
https://lore.kernel.org/linux-arm-kernel/96fd3054-17b1-db42-9a44-a60485243807@linaro.org/t/
changes are rebased over
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git refs/heads/next

tested brcm,sdhci-brcmstb.yaml and driver with and without sdio_freq clock.

Al Cooper (1):
  mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

Kamal Dasu (1):
  dt-bindings: mmc: Add Broadcom optional sdio_freq clock

 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 25 ++++++++++++----
 drivers/mmc/host/sdhci-brcmstb.c              | 30 +++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock

[Kamal Dasu]

The 72116B0 has improved SDIO controllers that allow the max clock
rate to be increased from a max of 100MHz to a max of 150MHz.
Optional "sdio_freq" clock is used to drive the bus clock if present
optional property "max-frequency" specifies a base clock frequency
in Hz that overrides the base clock frequency in the CAPS registers.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
index b672202fff4e..3a4f6e75bf5e 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
@@ -10,9 +10,6 @@ maintainers:
   - Al Cooper <alcooperx@gmail.com>
   - Florian Fainelli <f.fainelli@gmail.com>
 
-allOf:
-  - $ref: mmc-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -42,23 +39,39 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
-    description:
-      handle to core clock for the sdhci controller.
+    minItems: 1
+    items:
+      - description: handle to core clock for the sdhci controller
+      - description: handle to improved 150Mhz clock for sdhci controller (Optional clock)
 
   clock-names:
+    minItems: 1
     items:
       - const: sw_sdio
+      - const: sdio_freq # Optional clock
 
   sdhci,auto-cmd12:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+allOf:
+  - $ref: mmc-controller.yaml#
+  - if:
+      properties:
+        clock-names:
+          contains:
+            const: sdio_freq
+
+    then:
+      required:
+        - max-frequency
+
 required:
   - compatible
   - reg
   - interrupts
   - clocks
+  - clock-names
 
 unevaluatedProperties: false
 
-- 
2.17.1

[PATCH 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock

[Kamal Dasu]

The 72116B0 has improved SDIO controllers that allow the max clock
rate to be increased from a max of 100MHz to a max of 150MHz.
Optional "sdio_freq" clock is used to drive the bus clock if present
optional property "max-frequency" specifies a base clock frequency
in Hz that overrides the base clock frequency in the CAPS registers.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
index b672202fff4e..3a4f6e75bf5e 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
@@ -10,9 +10,6 @@ maintainers:
   - Al Cooper <alcooperx@gmail.com>
   - Florian Fainelli <f.fainelli@gmail.com>
 
-allOf:
-  - $ref: mmc-controller.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -42,23 +39,39 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
-    description:
-      handle to core clock for the sdhci controller.
+    minItems: 1
+    items:
+      - description: handle to core clock for the sdhci controller
+      - description: handle to improved 150Mhz clock for sdhci controller (Optional clock)
 
   clock-names:
+    minItems: 1
     items:
       - const: sw_sdio
+      - const: sdio_freq # Optional clock
 
   sdhci,auto-cmd12:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+allOf:
+  - $ref: mmc-controller.yaml#
+  - if:
+      properties:
+        clock-names:
+          contains:
+            const: sdio_freq
+
+    then:
+      required:
+        - max-frequency
+
 required:
   - compatible
   - reg
   - interrupts
   - clocks
+  - clock-names
 
 unevaluatedProperties: false
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock

[Krzysztof Kozlowski]

On 13/05/2022 22:19, Kamal Dasu wrote:
> The 72116B0 has improved SDIO controllers that allow the max clock
> rate to be increased from a max of 100MHz to a max of 150MHz.
> Optional "sdio_freq" clock is used to drive the bus clock if present
> optional property "max-frequency" specifies a base clock frequency
> in Hz that overrides the base clock frequency in the CAPS registers.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock

[Krzysztof Kozlowski]

On 13/05/2022 22:19, Kamal Dasu wrote:
> The 72116B0 has improved SDIO controllers that allow the max clock
> rate to be increased from a max of 100MHz to a max of 150MHz.
> Optional "sdio_freq" clock is used to drive the bus clock if present
> optional property "max-frequency" specifies a base clock frequency
> in Hz that overrides the base clock frequency in the CAPS registers.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Kamal Dasu]

From: Al Cooper <alcooperx@gmail.com>

The 72116B0 has improved SDIO controllers that allow the max clock
rate to be increased from a max of 100MHz to a max of 150MHz. The
driver will need to get the clock and increase it's default rate
and override the caps register, that still indicates a max of 100MHz.
The new clock will be named "sdio_freq" in the DT node's "clock-names"
list. The driver will use a DT property, "clock-frequency", to
enable this functionality and will get the actual rate in MHz
from the property to allow various speeds to be requested.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 8eb57de48e0c..a1ffdd3f1640 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	const struct of_device_id *match;
 	struct sdhci_brcmstb_priv *priv;
+	struct clk *master_clk;
+	u32 base_clock_hz = 0;
+	u32 actual_clock_mhz;
 	struct sdhci_host *host;
 	struct resource *iomem;
 	struct clk *clk;
@@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+	/* Change the base clock frequency if the DT property exists */
+	if (device_property_read_u32(&pdev->dev, "max-frequency",
+				     &base_clock_hz) != 0)
+		goto add_host;
+
+	master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
+	if (IS_ERR(master_clk)) {
+		dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
+		goto add_host;
+	} else {
+		res = clk_prepare_enable(master_clk);
+		if (res)
+			goto err;
+	}
+
+	/* set improved clock rate */
+	clk_set_rate(master_clk, base_clock_hz);
+	actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
+
+	host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
+	host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
+	/* Disable presets because they are now incorrect */
+	host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
+	dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
+		actual_clock_mhz);
+
+add_host:
 	res = sdhci_brcmstb_add_host(host, priv);
 	if (res)
 		goto err;
-- 
2.17.1

[PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Kamal Dasu]

From: Al Cooper <alcooperx@gmail.com>

The 72116B0 has improved SDIO controllers that allow the max clock
rate to be increased from a max of 100MHz to a max of 150MHz. The
driver will need to get the clock and increase it's default rate
and override the caps register, that still indicates a max of 100MHz.
The new clock will be named "sdio_freq" in the DT node's "clock-names"
list. The driver will use a DT property, "clock-frequency", to
enable this functionality and will get the actual rate in MHz
from the property to allow various speeds to be requested.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 8eb57de48e0c..a1ffdd3f1640 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host;
 	const struct of_device_id *match;
 	struct sdhci_brcmstb_priv *priv;
+	struct clk *master_clk;
+	u32 base_clock_hz = 0;
+	u32 actual_clock_mhz;
 	struct sdhci_host *host;
 	struct resource *iomem;
 	struct clk *clk;
@@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+	/* Change the base clock frequency if the DT property exists */
+	if (device_property_read_u32(&pdev->dev, "max-frequency",
+				     &base_clock_hz) != 0)
+		goto add_host;
+
+	master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
+	if (IS_ERR(master_clk)) {
+		dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
+		goto add_host;
+	} else {
+		res = clk_prepare_enable(master_clk);
+		if (res)
+			goto err;
+	}
+
+	/* set improved clock rate */
+	clk_set_rate(master_clk, base_clock_hz);
+	actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
+
+	host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
+	host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
+	/* Disable presets because they are now incorrect */
+	host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
+	dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
+		actual_clock_mhz);
+
+add_host:
 	res = sdhci_brcmstb_add_host(host, priv);
 	if (res)
 		goto err;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Ulf Hansson]

On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> From: Al Cooper <alcooperx@gmail.com>
>
> The 72116B0 has improved SDIO controllers that allow the max clock
> rate to be increased from a max of 100MHz to a max of 150MHz. The
> driver will need to get the clock and increase it's default rate
> and override the caps register, that still indicates a max of 100MHz.
> The new clock will be named "sdio_freq" in the DT node's "clock-names"
> list. The driver will use a DT property, "clock-frequency", to

/s/clock-frequency/max-frequency


> enable this functionality and will get the actual rate in MHz
> from the property to allow various speeds to be requested.
>
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index 8eb57de48e0c..a1ffdd3f1640 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>         struct sdhci_pltfm_host *pltfm_host;
>         const struct of_device_id *match;
>         struct sdhci_brcmstb_priv *priv;
> +       struct clk *master_clk;
> +       u32 base_clock_hz = 0;
> +       u32 actual_clock_mhz;
>         struct sdhci_host *host;
>         struct resource *iomem;
>         struct clk *clk;
> @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
>                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> +       /* Change the base clock frequency if the DT property exists */
> +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> +                                    &base_clock_hz) != 0)
> +               goto add_host;

The max-frequency DT property is already being parsed by
mmc_of_parse() and the value is put in host->mmc->f_max. You could
probably use that instead, right?

> +
> +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> +       if (IS_ERR(master_clk)) {
> +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> +               goto add_host;
> +       } else {
> +               res = clk_prepare_enable(master_clk);
> +               if (res)
> +                       goto err;
> +       }
> +
> +       /* set improved clock rate */
> +       clk_set_rate(master_clk, base_clock_hz);
> +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> +
> +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> +       /* Disable presets because they are now incorrect */
> +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> +               actual_clock_mhz);
> +
> +add_host:
>         res = sdhci_brcmstb_add_host(host, priv);
>         if (res)
>                 goto err;
> --
> 2.17.1
>

Kind regards
Uffe

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Ulf Hansson]

On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>
> From: Al Cooper <alcooperx@gmail.com>
>
> The 72116B0 has improved SDIO controllers that allow the max clock
> rate to be increased from a max of 100MHz to a max of 150MHz. The
> driver will need to get the clock and increase it's default rate
> and override the caps register, that still indicates a max of 100MHz.
> The new clock will be named "sdio_freq" in the DT node's "clock-names"
> list. The driver will use a DT property, "clock-frequency", to

/s/clock-frequency/max-frequency


> enable this functionality and will get the actual rate in MHz
> from the property to allow various speeds to be requested.
>
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index 8eb57de48e0c..a1ffdd3f1640 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>         struct sdhci_pltfm_host *pltfm_host;
>         const struct of_device_id *match;
>         struct sdhci_brcmstb_priv *priv;
> +       struct clk *master_clk;
> +       u32 base_clock_hz = 0;
> +       u32 actual_clock_mhz;
>         struct sdhci_host *host;
>         struct resource *iomem;
>         struct clk *clk;
> @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
>                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
>
> +       /* Change the base clock frequency if the DT property exists */
> +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> +                                    &base_clock_hz) != 0)
> +               goto add_host;

The max-frequency DT property is already being parsed by
mmc_of_parse() and the value is put in host->mmc->f_max. You could
probably use that instead, right?

> +
> +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> +       if (IS_ERR(master_clk)) {
> +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> +               goto add_host;
> +       } else {
> +               res = clk_prepare_enable(master_clk);
> +               if (res)
> +                       goto err;
> +       }
> +
> +       /* set improved clock rate */
> +       clk_set_rate(master_clk, base_clock_hz);
> +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> +
> +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> +       /* Disable presets because they are now incorrect */
> +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> +               actual_clock_mhz);
> +
> +add_host:
>         res = sdhci_brcmstb_add_host(host, priv);
>         if (res)
>                 goto err;
> --
> 2.17.1
>

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Alan Cooper]

This seems confusing and seems to overload the meaning of
"max-frequency" which is typically used to limit the clock rate to
something slower than what's in the CAPs register as the base clock.
Instead we're trying to overclock the controller because the hardware
team has verified that it can be run faster than 100MHz which is how
the system is configured and is in the CAPs register as the base
clock.

Al

On Tue, May 17, 2022 at 8:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> >
> > From: Al Cooper <alcooperx@gmail.com>
> >
> > The 72116B0 has improved SDIO controllers that allow the max clock
> > rate to be increased from a max of 100MHz to a max of 150MHz. The
> > driver will need to get the clock and increase it's default rate
> > and override the caps register, that still indicates a max of 100MHz.
> > The new clock will be named "sdio_freq" in the DT node's "clock-names"
> > list. The driver will use a DT property, "clock-frequency", to
>
> /s/clock-frequency/max-frequency
>
>
> > enable this functionality and will get the actual rate in MHz
> > from the property to allow various speeds to be requested.
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > index 8eb57de48e0c..a1ffdd3f1640 100644
> > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> >         struct sdhci_pltfm_host *pltfm_host;
> >         const struct of_device_id *match;
> >         struct sdhci_brcmstb_priv *priv;
> > +       struct clk *master_clk;
> > +       u32 base_clock_hz = 0;
> > +       u32 actual_clock_mhz;
> >         struct sdhci_host *host;
> >         struct resource *iomem;
> >         struct clk *clk;
> > @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> >         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> >                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> >
> > +       /* Change the base clock frequency if the DT property exists */
> > +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> > +                                    &base_clock_hz) != 0)
> > +               goto add_host;
>
> The max-frequency DT property is already being parsed by
> mmc_of_parse() and the value is put in host->mmc->f_max. You could
> probably use that instead, right?
>
> > +
> > +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> > +       if (IS_ERR(master_clk)) {
> > +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> > +               goto add_host;
> > +       } else {
> > +               res = clk_prepare_enable(master_clk);
> > +               if (res)
> > +                       goto err;
> > +       }
> > +
> > +       /* set improved clock rate */
> > +       clk_set_rate(master_clk, base_clock_hz);
> > +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> > +
> > +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> > +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> > +       /* Disable presets because they are now incorrect */
> > +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> > +               actual_clock_mhz);
> > +
> > +add_host:
> >         res = sdhci_brcmstb_add_host(host, priv);
> >         if (res)
> >                 goto err;
> > --
> > 2.17.1
> >
>
> Kind regards
> Uffe

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Alan Cooper]

This seems confusing and seems to overload the meaning of
"max-frequency" which is typically used to limit the clock rate to
something slower than what's in the CAPs register as the base clock.
Instead we're trying to overclock the controller because the hardware
team has verified that it can be run faster than 100MHz which is how
the system is configured and is in the CAPs register as the base
clock.

Al

On Tue, May 17, 2022 at 8:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> >
> > From: Al Cooper <alcooperx@gmail.com>
> >
> > The 72116B0 has improved SDIO controllers that allow the max clock
> > rate to be increased from a max of 100MHz to a max of 150MHz. The
> > driver will need to get the clock and increase it's default rate
> > and override the caps register, that still indicates a max of 100MHz.
> > The new clock will be named "sdio_freq" in the DT node's "clock-names"
> > list. The driver will use a DT property, "clock-frequency", to
>
> /s/clock-frequency/max-frequency
>
>
> > enable this functionality and will get the actual rate in MHz
> > from the property to allow various speeds to be requested.
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > ---
> >  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > index 8eb57de48e0c..a1ffdd3f1640 100644
> > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> >         struct sdhci_pltfm_host *pltfm_host;
> >         const struct of_device_id *match;
> >         struct sdhci_brcmstb_priv *priv;
> > +       struct clk *master_clk;
> > +       u32 base_clock_hz = 0;
> > +       u32 actual_clock_mhz;
> >         struct sdhci_host *host;
> >         struct resource *iomem;
> >         struct clk *clk;
> > @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> >         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> >                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> >
> > +       /* Change the base clock frequency if the DT property exists */
> > +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> > +                                    &base_clock_hz) != 0)
> > +               goto add_host;
>
> The max-frequency DT property is already being parsed by
> mmc_of_parse() and the value is put in host->mmc->f_max. You could
> probably use that instead, right?
>
> > +
> > +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> > +       if (IS_ERR(master_clk)) {
> > +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> > +               goto add_host;
> > +       } else {
> > +               res = clk_prepare_enable(master_clk);
> > +               if (res)
> > +                       goto err;
> > +       }
> > +
> > +       /* set improved clock rate */
> > +       clk_set_rate(master_clk, base_clock_hz);
> > +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> > +
> > +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> > +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> > +       /* Disable presets because they are now incorrect */
> > +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> > +               actual_clock_mhz);
> > +
> > +add_host:
> >         res = sdhci_brcmstb_add_host(host, priv);
> >         if (res)
> >                 goto err;
> > --
> > 2.17.1
> >
>
> Kind regards
> Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Kamal Dasu]

I think I seem to agree now that overloading the meaning of
max-frequency for the sdio_freq clock might not be a good idea. If
Uffe is Ok I think I will revert back to using the 'clock-frequency'
field in the brcmstb,sdhci-brcmstb dt node and implement other
suggestions as well

Kamal

On Thu, May 19, 2022 at 10:16 PM Alan Cooper <alcooperx@gmail.com> wrote:
>
> This seems confusing and seems to overload the meaning of
> "max-frequency" which is typically used to limit the clock rate to
> something slower than what's in the CAPs register as the base clock.
> Instead we're trying to overclock the controller because the hardware
> team has verified that it can be run faster than 100MHz which is how
> the system is configured and is in the CAPs register as the base
> clock.
>
> Al
>
> On Tue, May 17, 2022 at 8:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> > >
> > > From: Al Cooper <alcooperx@gmail.com>
> > >
> > > The 72116B0 has improved SDIO controllers that allow the max clock
> > > rate to be increased from a max of 100MHz to a max of 150MHz. The
> > > driver will need to get the clock and increase it's default rate
> > > and override the caps register, that still indicates a max of 100MHz.
> > > The new clock will be named "sdio_freq" in the DT node's "clock-names"
> > > list. The driver will use a DT property, "clock-frequency", to
> >
> > /s/clock-frequency/max-frequency
> >
> >
> > > enable this functionality and will get the actual rate in MHz
> > > from the property to allow various speeds to be requested.
> > >
> > > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > > ---
> > >  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > > index 8eb57de48e0c..a1ffdd3f1640 100644
> > > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > > @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > >         struct sdhci_pltfm_host *pltfm_host;
> > >         const struct of_device_id *match;
> > >         struct sdhci_brcmstb_priv *priv;
> > > +       struct clk *master_clk;
> > > +       u32 base_clock_hz = 0;
> > > +       u32 actual_clock_mhz;
> > >         struct sdhci_host *host;
> > >         struct resource *iomem;
> > >         struct clk *clk;
> > > @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > >         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> > >                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> > >
> > > +       /* Change the base clock frequency if the DT property exists */
> > > +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> > > +                                    &base_clock_hz) != 0)
> > > +               goto add_host;
> >
> > The max-frequency DT property is already being parsed by
> > mmc_of_parse() and the value is put in host->mmc->f_max. You could
> > probably use that instead, right?
> >
> > > +
> > > +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> > > +       if (IS_ERR(master_clk)) {
> > > +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> > > +               goto add_host;
> > > +       } else {
> > > +               res = clk_prepare_enable(master_clk);
> > > +               if (res)
> > > +                       goto err;
> > > +       }
> > > +
> > > +       /* set improved clock rate */
> > > +       clk_set_rate(master_clk, base_clock_hz);
> > > +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> > > +
> > > +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> > > +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> > > +       /* Disable presets because they are now incorrect */
> > > +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > > +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> > > +               actual_clock_mhz);
> > > +
> > > +add_host:
> > >         res = sdhci_brcmstb_add_host(host, priv);
> > >         if (res)
> > >                 goto err;
> > > --
> > > 2.17.1
> > >
> >
> > Kind regards
> > Uffe

Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0

[Kamal Dasu]

I think I seem to agree now that overloading the meaning of
max-frequency for the sdio_freq clock might not be a good idea. If
Uffe is Ok I think I will revert back to using the 'clock-frequency'
field in the brcmstb,sdhci-brcmstb dt node and implement other
suggestions as well

Kamal

On Thu, May 19, 2022 at 10:16 PM Alan Cooper <alcooperx@gmail.com> wrote:
>
> This seems confusing and seems to overload the meaning of
> "max-frequency" which is typically used to limit the clock rate to
> something slower than what's in the CAPs register as the base clock.
> Instead we're trying to overclock the controller because the hardware
> team has verified that it can be run faster than 100MHz which is how
> the system is configured and is in the CAPs register as the base
> clock.
>
> Al
>
> On Tue, May 17, 2022 at 8:50 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> > >
> > > From: Al Cooper <alcooperx@gmail.com>
> > >
> > > The 72116B0 has improved SDIO controllers that allow the max clock
> > > rate to be increased from a max of 100MHz to a max of 150MHz. The
> > > driver will need to get the clock and increase it's default rate
> > > and override the caps register, that still indicates a max of 100MHz.
> > > The new clock will be named "sdio_freq" in the DT node's "clock-names"
> > > list. The driver will use a DT property, "clock-frequency", to
> >
> > /s/clock-frequency/max-frequency
> >
> >
> > > enable this functionality and will get the actual rate in MHz
> > > from the property to allow various speeds to be requested.
> > >
> > > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > > Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> > > ---
> > >  drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > > index 8eb57de48e0c..a1ffdd3f1640 100644
> > > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > > @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > >         struct sdhci_pltfm_host *pltfm_host;
> > >         const struct of_device_id *match;
> > >         struct sdhci_brcmstb_priv *priv;
> > > +       struct clk *master_clk;
> > > +       u32 base_clock_hz = 0;
> > > +       u32 actual_clock_mhz;
> > >         struct sdhci_host *host;
> > >         struct resource *iomem;
> > >         struct clk *clk;
> > > @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > >         if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> > >                 host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> > >
> > > +       /* Change the base clock frequency if the DT property exists */
> > > +       if (device_property_read_u32(&pdev->dev, "max-frequency",
> > > +                                    &base_clock_hz) != 0)
> > > +               goto add_host;
> >
> > The max-frequency DT property is already being parsed by
> > mmc_of_parse() and the value is put in host->mmc->f_max. You could
> > probably use that instead, right?
> >
> > > +
> > > +       master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> > > +       if (IS_ERR(master_clk)) {
> > > +               dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> > > +               goto add_host;
> > > +       } else {
> > > +               res = clk_prepare_enable(master_clk);
> > > +               if (res)
> > > +                       goto err;
> > > +       }
> > > +
> > > +       /* set improved clock rate */
> > > +       clk_set_rate(master_clk, base_clock_hz);
> > > +       actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> > > +
> > > +       host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> > > +       host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> > > +       /* Disable presets because they are now incorrect */
> > > +       host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > > +       dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> > > +               actual_clock_mhz);
> > > +
> > > +add_host:
> > >         res = sdhci_brcmstb_add_host(host, priv);
> > >         if (res)
> > >                 goto err;
> > > --
> > > 2.17.1
> > >
> >
> > Kind regards
> > Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel