On Sun, Oct 6, 2024 at 3:13 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > 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 I am enabling and configuring the spread spectrum. Normal clock: Without spread spectrum, the clock signal has a fixed and repetitive frequency (e.g., 100 MHz). This frequency generates an electromagnetic signal concentrated on a single frequency, and if strong enough, it can disturb other devices. Spread spectrum: With spread spectrum, the clock frequency is slightly "modulated," meaning it oscillates around a central value. For example, if the base frequency is 100 MHz, the clock might vary between 99.5 MHz and 100.5 MHz in a cyclic manner. This small variation spreads the energy over a wider range of frequencies (from 99.5 to 100.5 MHz), reducing the intensity of the signal at any one frequency. > and why assigned-clock-rates are > not working? The traditional clock properties, such as clocks, assigned-clocks-rates, etc retain their usual meaning even when spread spectrum is applied. However, to implement the spread spectrum mechanism in a circuit with a PLL (Phase-Locked Loop), additional specific parameters are introduced to properly configure the frequency modulation: - Modulation frequency: i. e. fsl,ssc-modfreq-hz - Modulation rate: i.e. fsl,ssc-modrate-percent - Modulation type: i. e. fsl,ssc-modmethod (center-spread, down-spread) Additionally, it should be noted that not all anatop PLLs are equipped with circuitry for spread spectrum, but only a small subset of them. This is the reason why I introduced the property "fsl, ssc-clocks". This is another commit [1] on enabling spread spectrum that I implemented some time ago for the am335x. The most evident difference is that in that case the node was a clock node and not a clock controller, as in the case of anatop. The parameters are also not exactly the same, but that depends on the platform. [1] 4a8bc2644ef0cbf8e ("dt-bindings: ti: dpll: add spread spectrum support") Thanks and regards, Dario > > Best regards, > Krzysztof > -- Dario Binacchi Senior Embedded Linux Developer dario.binacchi@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com