On 10/17/18 10:29 PM, Jon Hunter wrote: > > On 17/10/2018 15:43, Dmitry Osipenko wrote: >> On 10/17/18 5:14 PM, Jon Hunter wrote: >>> >>> On 17/10/2018 14:46, Dmitry Osipenko wrote: >>>> On 10/17/18 4:34 PM, Jon Hunter wrote: >>>>> >>>>> On 17/10/2018 14:07, Dmitry Osipenko wrote: >>>>>> On 10/17/18 3:59 PM, Jon Hunter wrote: >>>>>>> >>>>>>> On 17/10/2018 13:37, Dmitry Osipenko wrote: >>>>>>>> On 10/17/18 11:40 AM, Jon Hunter wrote: >>>>>>>>> >>>>>>>>> On 30/08/2018 20:43, Dmitry Osipenko wrote: >>>>>>>>>> Add device-tree binding that describes CPU frequency-scaling hardware >>>>>>>>>> found on NVIDIA Tegra20/30 SoC's. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Dmitry Osipenko <[email protected]> >>>>>>>>>> --- >>>>>>>>>> .../cpufreq/nvidia,tegra20-cpufreq.txt | 38 >>>>>>>>>> +++++++++++++++++++ >>>>>>>>>> 1 file changed, 38 insertions(+) >>>>>>>>>> create mode 100644 >>>>>>>>>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt >>>>>>>>>> >>>>>>>>>> diff --git >>>>>>>>>> a/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt >>>>>>>>>> >>>>>>>>>> b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt >>>>>>>>>> new file mode 100644 >>>>>>>>>> index 000000000000..2c51f676e958 >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ >>>>>>>>>> b/Documentation/devicetree/bindings/cpufreq/nvidia,tegra20-cpufreq.txt >>>>>>>>>> @@ -0,0 +1,38 @@ >>>>>>>>>> +Binding for NVIDIA Tegra20 CPUFreq >>>>>>>>>> +================================== >>>>>>>>>> + >>>>>>>>>> +Required properties: >>>>>>>>>> +- clocks: Must contain an entry for each entry in clock-names. >>>>>>>>>> + See ../clocks/clock-bindings.txt for details. >>>>>>>>>> +- clock-names: Must include the following entries: >>>>>>>>>> + - pll_x: main-parent for CPU clock, must be the first entry >>>>>>>>>> + - backup: intermediate-parent for CPU clock >>>>>>>>>> + - cpu: the CPU clock >>>>>>>>> >>>>>>>>> Is it likely that 'backup' will be anything other that pll_p? If not >>>>>>>>> why >>>>>>>>> not just call it pll_p? Personally, I don't 'backup' to descriptive >>>>>>>>> even >>>>>>>>> though I can see what you mean. >>>>>>>>> >>>>>>>>> I can see that you want to make this flexible, but if the likelihood >>>>>>>>> is >>>>>>>>> that we will just use pll_p then I am not sure it is warranted at this >>>>>>>>> point. >>>>>>>> >>>>>>>> That won't describe HW, but software. And device tree should describe >>>>>>>> HW. >>>>>>> >>>>>>> Hmm ... well that's my point exactly. So why call it 'backup'? Sounds >>>>>>> like a software description to me. >>>>>> >>>>>> Because HW is designed the way that CPU parent need to be switched to >>>>>> the backup clock source while main clock changes its rate. HW also allow >>>>>> to select among different parents, pll_p is one of those parents. >>>>> >>>>> Yes that part is understood. I am just splitting hairs over the actual >>>>> name. We do the same for tegra124 but we just call it 'pll_p'. See ... >>>>> >>>>> Documentation/devicetree/bindings/cpufreq/nvidia,tegra124-cpufreq.txt >>>> >>>> tegra124-cpufreq choose to hardwire to the pll_p, but it could be other >>>> clocks. Technically abstracting backup clock should be more correct, but >>>> result is the same in the end. >>> >>> Yes and that is probably intentional as that is the configuration that >>> has been validated. So unless we are planning to test and verify every >>> possibility, my preference is to keep it simple. But yes the result is >>> the same. >>> >>> I was simply curious of your intention here because of the other series >>> you posted with regard to the cpu clocks. >> >> Actually I tried other parents on T30 and they worked with no problems. The >> intention of having 'backup' is to describe HW properly in the device tree, >> that's it. We'll have backup set to pll_p by default. ACK? > > Yes but I don't find the name 'backup' very descriptive because like you > say it is an intermediate clock for switching frequency. But at the same > time I don't have a good suggestion. Anyway it is more of a > bike-shedding comment.
I'll consider renaming 'backup' to 'intermediate' in the v2 which I'll probably manage to send out tomorrow as RFC (CPUFreq now depends on yet-not-reviewed coupled-regulators patches).
