mmc: sdhci-brcmstb: Add support for optional sdio_freq clock
Subsystems (5)
cover mmc: sdhci-brcmstb: Add support for optional sdio_freq clock
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 | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.1
PATCH 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 1/2] dt-bindings: mmc: Add Broadcom optional sdio_freq clock
Quoted 7 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]>
Best regards,
Krzysztof
PATCH 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, "clock-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 | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
drivers/mmc/host/sdhci-brcmstb.c +30 -0
@@ -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;
Signature · 1 line
2.17.1
Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0
Quoted 9 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, "clock-frequency", to
/s/clock-frequency/max-frequency
Quoted 24 lines
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 | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci-brcmstb.c +3
-0@@ -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;
drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 339
Quoted 7 lines
drivers/mmc/host/sdhci-brcmstb.c +4
-0@@ -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?
Quoted 27 lines
drivers/mmc/host/sdhci-brcmstb.c +23
-1+ + 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; --
Quoted 2 lines
2.17.1
Kind regards
Uffe
Alan Cooper
maintainerRe: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0
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
Quoted 83 lines · Ulf Hansson
Quoted 9 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, "clock-frequency", to/s/clock-frequency/max-frequency
Quoted 24 lines
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 | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci-brcmstb.c +3
-0@@ -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;drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 339
Quoted 7 lines
drivers/mmc/host/sdhci-brcmstb.c +4
-0@@ -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 inhost->mmc->f_max. You could probably use that instead, right?Quoted 27 lines
drivers/mmc/host/sdhci-brcmstb.c +23
-1+ + 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; --Quoted 2 lines
2.17.1
Kind regards
Uffe
Re: [PATCH 2/2] mmc: sdhci-brcmstb: Add ability to increase max clock rate for 72116b0
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
Quoted 95 lines · 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
Quoted 83 lines · Ulf Hansson
Quoted 9 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, "clock-frequency", to/s/clock-frequency/max-frequency
Quoted 24 lines
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 | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci-brcmstb.c +3
-0@@ -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;drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 339
Quoted 7 lines
drivers/mmc/host/sdhci-brcmstb.c +4
-0@@ -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 inhost->mmc->f_max. You could probably use that instead, right?Quoted 27 lines
drivers/mmc/host/sdhci-brcmstb.c +23
-1+ + 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; --Quoted 2 lines
2.17.1
Kind regards
Uffe