On Fri, Nov 8, 2024 at 6:37 PM Peng Fan <peng.fan@xxxxxxx> wrote: > > > Subject: Re: [PATCH v3 1/8] dt-bindings: clock: imx8m-clock: support > > spread spectrum clocking > > > > On 08/11/2024 13:50, Peng Fan wrote: > > >> Subject: Re: [PATCH v3 1/8] dt-bindings: clock: imx8m-clock: > > support > > >> spread spectrum clocking > > >> > > >> On 07/11/2024 15:57, Dario Binacchi wrote: > > >>> clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>, > > >>> <&clk_ext3>, <&clk_ext4>; > > >>> clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2", > > >>> "clk_ext3", "clk_ext4"; > > >>> assigned-clocks = <&clk IMX8MN_CLK_A53_SRC>, > > >>> <&clk IMX8MN_CLK_A53_CORE>, > > >>> <&clk IMX8MN_CLK_NOC>, > > >>> <&clk IMX8MN_CLK_AUDIO_AHB>, > > >>> <&clk IMX8MN_CLK_IPG_AUDIO_ROOT>, > > >>> <&clk IMX8MN_SYS_PLL3>, > > >>> <&clk IMX8MN_AUDIO_PLL1>, > > >>> <&clk IMX8MN_AUDIO_PLL2>; > > >>> assigned-clock-parents = <&clk IMX8MN_SYS_PLL1_800M>, > > >>> <&clk IMX8MN_ARM_PLL_OUT>, > > >>> <&clk IMX8MN_SYS_PLL3_OUT>, > > >>> <&clk IMX8MN_SYS_PLL1_800M>; > > >>> assigned-clock-rates = <0>, <0>, <0>, > > >>> <400000000>, > > >>> <400000000>, > > >>> <600000000>, > > >>> <393216000>, > > >>> <361267200>; }; > > >>> > > >>> The spread spectrum is not configurable on these clocks or, more > > >>> generally, may not be configurable (only 4 PLLs have this > > capability). > > >>> Therefore, I need the "fsl,ssc-clocks" > > >> > > >> No. That's not true. You do not need it. > > >> > > > > > > i.MX8M clock hardware is similar as: > > > > > > OSC->ANATOP->CCM > > > > > > ANATOP will produce PLLs. > > > CCM use PLLs as input source. > > > > > > Currently there is no dedicated ANATOP driver in linux. > > > The CCM linux driver will parse the ANATOP node and register clk_hw > > > for the PLLs. > > > > I do not know what is CCM and how does it fit here. What's more, I > > don't get driver context here. We talk about bindings. > > > CCM: Clock Control Module, it accepts PLL from anatop as inputs, > and outputs clocks to various modules, I2C, CAN, NET, SAI and ... > > > > > > > > > > > > > >> First, the clock inputs for this device are listed in clocks *only*. > > >> What is no there, is not an input to the device. Including also Linux > > >> aspect (missing devlinks etc). Therefore how can you configure > > spread > > >> spectrum on clocks which are not connected to this device? > > > > > > I not understand this well, you mean > > > add clocks = <xx CLK_IMX8MM_VIDEO_PLL> in the ccm dtb node? > > > > Yes. Let me re-iterate and please respond to this exactly comment > > instead of ignoring it. > > > > How a device can care about spread spectrum of a clock which is not > > supplied to this device? > > I hope we are on same page of what spread spectrum means. > spread spectrum of a clock is the clock could produce freq in a range, > saying [500MHz - 100KHz, 500MHz + 100KHz]. software only need > to configure the middle frequency and choose the up/down border > range(100KHz here) and enable spread spectrum. > > device: I suppose you mean the Clock Control Module(CCM) here. > CCM does not care, it just accepts the PLL as input, and output > divided clock to various IPs(Video here). The video IPs care about > the spread spectrum of the clock. > > The clock hardware path is as below: > > OSC(24M) --> Anatop(produce PLL with spread spectrum) -> > Clock Control Module(output clock to modules) -> Video IP > > From hardware perspective, Clock Control Module does not > care spread spectrum. Video IP cares spread spectrum. > > > > > > Why would you care about spread spectrum of some clock which is not > > coming to this device? > > device, I suppose you mean clock control module(CCM). > > There is no 'clocks = <&ccm CLK_IMX8M_VIDEO_PLL>' under ccm node. > Because in current design, ccm is taken as producer of > CLK_IMX8M_VIDEO_PLL, not consumer. > > > > > Please address these precisely because we talk about this for weeks in > > multiple places. > > Sorry for coming into this feature in late stage. > > Dario, thanks for working on such feature, good to see. Spread Spectrum > is indeed good feature what makes clock quality high. I am also excited to see the spread-spectum clocks enabled. We've struggled with EMC testing in the past, and I want to reevaluate at least one board with the spread-spectrum enabled to see how it compares. Thank you for working on this. adam > > Thanks, > Peng. > > I finish with this patchset if you do not provide such > > context. > > > > Best regards, > > Krzysztof >