mmc: sdhci-brcmstb: Add support for optional sdio_freq clock
Subsystems (5)
v2 cover mmc: sdhci-brcmstb: Add support for optional sdio_freq clock
v2 changes :
- Added Reviewed-by tag to PATCH v2 1/2
- In PATCH v2 2/2 Used host->mmc->f_max instead of parsing the device tree again, /s/clock-frequency/max-frequency in commit message
v1 Changes :
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/[email protected]/t/
changes are rebased overgit://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 | 28 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/2 dt-bindings: mmc: Add Broadcom optional sdio_freq clock
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.
--- .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-)
Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +19 -6
@@ -10,9 +10,6 @@ maintainers: - Al Cooper <[email protected]> - Florian Fainelli <[email protected]> -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
Signature · 1 line
2.17.1
Re: [PATCH v2 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock
Quoted 8 lines · 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 <[email protected]>Reviewed-by Krzysztof Kozlowski <[email protected]>
Signature · 1 line
Florian
PATCH v2 2/2 mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0
In reply to Kamal DasuFrom: Al Cooper <[email protected]>
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, "max-frequency", to enable this functionality and will get the actual rate in MHz from the property to allow various speeds to be requested.
--- drivers/mmc/host/sdhci-brcmstb.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
drivers/mmc/host/sdhci-brcmstb.c +28 -0
@@ -250,6 +250,8 @@ 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 actual_clock_mhz;struct sdhci_host *host;struct resource *iomem;struct clk *clk;
@@ -330,6 +332,32 @@ 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 (!(host->mmc->f_max)) + 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, host->mmc->f_max); + 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;
Signature · 1 line
2.17.1
Re: [PATCH v2 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0
Hi Kamal,
Quoted 31 lines · Kamal Dasu
From: Al Cooper <[email protected]>
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, "max-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 <[email protected]>Signed-off-by Kamal Dasu <[email protected]>--- drivers/mmc/host/sdhci-brcmstb.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)drivers/mmc/host/sdhci-brcmstb.c +2
-0@@ -250,6 +250,8 @@ 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 actual_clock_mhz;struct sdhci_host *host;struct resource *iomem;struct clk *clk;
drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 339
Quoted 8 lines
drivers/mmc/host/sdhci-brcmstb.c +5
-0@@ -330,6 +332,32 @@ 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 (!(host->mmc->f_max)) + goto add_host; + + master_clk =devm_clk_get (&pdev->dev,"sdio_freq" );
This looks like a candidate for devm_clk_get_optional() since the clock is optional. Then you can call clk_prepare_enable() unconditionally even if it is NULL/non-existent.
drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 346
Quoted 7 lines
drivers/mmc/host/sdhci-brcmstb.c +7
-0+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;
It looks like we may be leaving the clock enabled even when we did not want to (e.g.: error path) and do not we need to turn if off, respectively turn it back on in .suspend() and .resume()?
Quoted 7 lines
drivers/mmc/host/sdhci-brcmstb.c +7
-0+ } + +/* set improved clock rate */ +clk_set_rate (master_clk, host->mmc->f_max); + actual_clock_mhz =clk_get_rate (master_clk) /1000000 ; + + host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK ;
drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 363
Quoted 10 lines
drivers/mmc/host/sdhci-brcmstb.c +7
-0+ 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;
It looks like we would need to unwind the clk_prepare_enable(master_clk) in case of failures here.
Signature · 1 line
Florian