On 05/10/2024 10:57, Dario Binacchi wrote: > On Thu, Oct 3, 2024 at 12:46 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >> >> On 01/10/2024 08:29, Dario Binacchi wrote: >>> On Mon, Sep 30, 2024 at 8:45 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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
