[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b847ccb1-1eb8-4119-8612-212804cb50d8@kernel.org>
Date: Sun, 6 Oct 2024 15:13:43 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: linux-kernel@...r.kernel.org, linux-amarula@...rulasolutions.com,
Conor Dooley <conor+dt@...nel.org>, Fabio Estevam <festevam@...il.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Michael Turquette <mturquette@...libre.com>, Peng Fan <peng.fan@....com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Rob Herring <robh@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>, Stephen Boyd <sboyd@...nel.org>,
devicetree@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH 1/6] dt-bindings: clock: imx8m-anatop: support spread
spectrum clocking
On 05/10/2024 10:57, Dario Binacchi wrote:
> On Thu, Oct 3, 2024 at 12:46 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>>
>> On 01/10/2024 08:29, Dario Binacchi wrote:
>>> On Mon, Sep 30, 2024 at 8:45 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>>>>
>>>> On 29/09/2024 22:00, Dario Binacchi wrote:
>>>>>>
>>>>>>
>>>>>>> + properties:
>>>>>>> + compatible:
>>>>>>> + contains:
>>>>>>> + enum:
>>>>>>> + - fsl,imx8mm-anatop
>>>>>>> +
>>>>>>> +then:
>>>>>>> + properties:
>>>>>>> + fsl,ssc-clocks:
>>>>>>
>>>>>> Nope. Properties must be defined in top-level.
>>>>>>
>>>>>>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>> + description:
>>>>>>> + The phandles to the PLLs with spread spectrum clock generation
>>>>>>> + hardware capability.
>>>>>>
>>>>>> These should be clocks.
>>>>>
>>>>> Sorry, but I can't understand what you're asking me.
>>>>> Could you kindly explain it to me in more detail?
>>>>
>>>> You added new property instead of using existing one for this purpose:
>>>> 'clocks'.
>>>
>>>>
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> I added this new property specifically for managing spread-spectrum.
>>> Indeed, not all clocks/PLLs
>>> managed by the node/peripheral support spread-spectrum, and the added
>>> properties specify
>>> parameters for enabling and tuning SSC for each individual PLL based
>>> on the index of each list.
>>> If I were to use the 'clocks' property and add a clock to this list
>>> that does not support SSC, IMHO
>>> the pairings would be less clear.
>>
>> You duplicate property with argument "pairings shall match". Well, I am
>> not happy with the duplication. Clocks have specific order, thus it is
>> explicit which one needs tuning. Your other properties can match them as
>> well, just index from clocks is offset...
>
> Just to check if I understood correctly what you are suggesting before
> submitting version 3 of the patch.
> Something, for example, like:
>
> clocks = <&clk, IMX8MP_AUDIO_PLL1>, <&clk, IMX8MP_AUDIO_PLL2>, <&clk
> IMX8MP_VIDEO_PLL1>;
> fsl,ssc-modfreq-hz = <0, 3517>, <2, 6818>;
Hm, what is 0? If clock index, then no, it's redundant. The first item
in cannot point to other clock.
Also, what exactly are you setting here and why assigned-clock-rates are
not working?
Best regards,
Krzysztof
Powered by blists - more mailing lists