Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT

[Rob Herring <robh@kernel.org>]

On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Rob,
>
> Thanks for review.
>
> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>
>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>
>>> This adds support for sdhc-msm controllers to get supported
>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>> ops to be implemented. For this, supported clk-rates needs
>>> to be populated in sdhci_msm_pltfm_data.
>>>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  .../devicetree/bindings/mmc/sdhci-msm.txt          |  1 +
>>>  drivers/mmc/host/sdhci-msm.c                       | 48
>>> ++++++++++++++++++++++
>>>  2 files changed, 49 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> index 485483a..6a83b38 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> @@ -17,6 +17,7 @@ Required properties:
>>>         "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>> (required)
>>>         "core"  - SDC MMC clock (MCLK) (required)
>>>         "bus"   - SDCC bus voter clock (optional)
>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units -
>>> Hz.
>>
>>
>> Why can't some combination of assigned-clock-rates and querying the
>> clock provider for rates be used here?
>
> From what I understood, assigned-clock-rates would only work for setting
> some default clock rates for certain clocks by calling
> of_clk_set_defaults.
>
> Whereas the requirement here is -
> That since SDHC msm directly controls the clk(core clock) at source, it's
> sdhci-msm driver needs to know the supported clk-rates by the underlying
> platform to configure the nearest floor value supported on this platform
> (when the request arrives from the core layer to switch the clock).

Why does clk_round_rate not work for you? That will round down to the
nearest frequency supported.

> Hence the table of clk-rates is provided for sdhci-msm.
>
>>
>> Minimally this would need unit suffix and either be made common or have
>> a vendor prefix.
>
> Sure will this work in that case - "qcom-clk-rates"

"qcom,clk-rates", but I'm not yet convinced this is right.

Rob