The patch adds the DT bindings for enabling and tuning spread spectrum
clocking generation.
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---
Changes in v3:
- Added in v3
- The dt-bindings have been moved from fsl,imx8m-anatop.yaml to
imx8m-clock.yaml. The anatop device (fsl,imx8m-anatop.yaml) is
indeed more or less a syscon, so it represents a memory area
accessible by ccm (imx8m-clock.yaml) to setup the PLLs.
.../bindings/clock/imx8m-clock.yaml | 46 +++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
index c643d4a81478..7920393e518e 100644
--- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
@@ -43,6 +43,40 @@ properties:
ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
for the full list of i.MX8M clock IDs.
+ fsl,ssc-clocks:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ Phandles of the PLL with spread spectrum generation hardware capability.
+ minItems: 1
+ maxItems: 4
+
+ fsl,ssc-modfreq-hz:
+ description:
+ The values of modulation frequency (Hz unit) of spread spectrum
+ clocking for each PLL.
+ minItems: 1
+ maxItems: 4
+
+ fsl,ssc-modrate-percent:
+ description:
+ The percentage values of modulation rate of spread spectrum
+ clocking for each PLL.
+ minItems: 1
+ maxItems: 4
+
+ fsl,ssc-modmethod:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description:
+ The modulation techniques of spread spectrum clocking for
+ each PLL.
+ minItems: 1
+ maxItems: 4
+ items:
+ enum:
+ - down-spread
+ - up-spread
+ - center-spread
+
required:
- compatible
- reg
@@ -76,6 +110,11 @@ allOf:
- const: clk_ext2
- const: clk_ext3
- const: clk_ext4
+ fsl,ssc-clocks: false
+ fsl,ssc-modfreq-hz: false
+ fsl,ssc-modrate-percent: false
+ fsl,ssc-modmethod: false
+
else:
properties:
clocks:
@@ -101,6 +140,8 @@ additionalProperties: false
examples:
# Clock Control Module node:
- |
+ #include <dt-bindings/clock/imx8mm-clock.h>
+
clock-controller@30380000 {
compatible = "fsl,imx8mm-ccm";
reg = <0x30380000 0x10000>;
@@ -109,6 +150,11 @@ examples:
<&clk_ext3>, <&clk_ext4>;
clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2",
"clk_ext3", "clk_ext4";
+ fsl,ssc-clocks = <&clk IMX8MM_AUDIO_PLL1>,
+ <&clk IMX8MM_VIDEO_PLL1>;
+ fsl,ssc-modfreq-hz = <6818>, <2419>;
+ fsl,ssc-modrate-percent = <3>, <7>;
+ fsl,ssc-modmethod = "down-spread", "center-spread";
};
- |
--
2.43.0On Wed, Nov 06, 2024 at 09:57:57AM +0100, Dario Binacchi wrote: > The patch adds the DT bindings for enabling and tuning spread spectrum > clocking generation. We had long talks about this but nothing of it got reflected in commit msg. Sorry, I don't remember what I was talking in some particular patch month ago, so you will get the same questions over and over... > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > --- > > Changes in v3: > - Added in v3 > - The dt-bindings have been moved from fsl,imx8m-anatop.yaml to > imx8m-clock.yaml. The anatop device (fsl,imx8m-anatop.yaml) is > indeed more or less a syscon, so it represents a memory area > accessible by ccm (imx8m-clock.yaml) to setup the PLLs. > > .../bindings/clock/imx8m-clock.yaml | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml > index c643d4a81478..7920393e518e 100644 > --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml > +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml > @@ -43,6 +43,40 @@ properties: > ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h > for the full list of i.MX8M clock IDs. > > + fsl,ssc-clocks: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + Phandles of the PLL with spread spectrum generation hardware capability. > + minItems: 1 > + maxItems: 4 1. How is it possible that you change spread spectrum of some clocks from main Clock Controller, while this device is not a consumer of them? Basically this means that this device does not have these clocks but yet you claim that it needs to configure spread for them! It's contradictory to me and nohing got explained in commit msg about it. I am pretty sure I asked about this alrady. 2. Why is this array flexible in size? Best regards, Krzysztof
On 06/11/2024 15:10, Krzysztof Kozlowski wrote: > On Wed, Nov 06, 2024 at 09:57:57AM +0100, Dario Binacchi wrote: >> The patch adds the DT bindings for enabling and tuning spread spectrum >> clocking generation. > > We had long talks about this but nothing of it got reflected in commit > msg. Sorry, I don't remember what I was talking in some particular patch > month ago, so you will get the same questions over and over... > >> >> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> >> >> --- >> >> Changes in v3: >> - Added in v3 >> - The dt-bindings have been moved from fsl,imx8m-anatop.yaml to >> imx8m-clock.yaml. The anatop device (fsl,imx8m-anatop.yaml) is >> indeed more or less a syscon, so it represents a memory area >> accessible by ccm (imx8m-clock.yaml) to setup the PLLs. >> >> .../bindings/clock/imx8m-clock.yaml | 46 +++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml >> index c643d4a81478..7920393e518e 100644 >> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml >> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml >> @@ -43,6 +43,40 @@ properties: >> ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h >> for the full list of i.MX8M clock IDs. >> >> + fsl,ssc-clocks: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + Phandles of the PLL with spread spectrum generation hardware capability. >> + minItems: 1 >> + maxItems: 4 > > 1. How is it possible that you change spread spectrum of some clocks from > main Clock Controller, while this device is not a consumer of them? > Basically this means that this device does not have these clocks but yet > you claim that it needs to configure spread for them! It's contradictory > to me and nohing got explained in commit msg about it. I am pretty sure > I asked about this alrady. I digged my previous answer and it was pretty clear here: 18:44 <krzk> You can, but I still have the same concerns. How this device - which does not take any clock input, has no clocks at all - can depend on spread spectrum of some PLLs? Thsi device does not have clocks. 18:50 <krzk> device has no clocks, I checked now third time 18:50 <krzk> If device has clocks, it must have clocks property So again, you do not need this property at all. I repeated it multiple times - you are supposed to use clocks property. > > 2. Why is this array flexible in size? > > Best regards, > Krzysztof > Best regards, Krzysztof
Hello Krzysztof,
On Wed, Nov 6, 2024 at 3:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 06/11/2024 15:10, Krzysztof Kozlowski wrote:
> > On Wed, Nov 06, 2024 at 09:57:57AM +0100, Dario Binacchi wrote:
> >> The patch adds the DT bindings for enabling and tuning spread spectrum
> >> clocking generation.
> >
> > We had long talks about this but nothing of it got reflected in commit
> > msg. Sorry, I don't remember what I was talking in some particular patch
> > month ago, so you will get the same questions over and over...
> >
> >>
> >> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> >>
> >> ---
> >>
> >> Changes in v3:
> >> - Added in v3
> >> - The dt-bindings have been moved from fsl,imx8m-anatop.yaml to
> >> imx8m-clock.yaml. The anatop device (fsl,imx8m-anatop.yaml) is
> >> indeed more or less a syscon, so it represents a memory area
> >> accessible by ccm (imx8m-clock.yaml) to setup the PLLs.
> >>
> >> .../bindings/clock/imx8m-clock.yaml | 46 +++++++++++++++++++
> >> 1 file changed, 46 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> index c643d4a81478..7920393e518e 100644
> >> --- a/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/imx8m-clock.yaml
> >> @@ -43,6 +43,40 @@ properties:
> >> ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx8m-clock.h
> >> for the full list of i.MX8M clock IDs.
> >>
> >> + fsl,ssc-clocks:
> >> + $ref: /schemas/types.yaml#/definitions/phandle-array
> >> + description:
> >> + Phandles of the PLL with spread spectrum generation hardware capability.
> >> + minItems: 1
> >> + maxItems: 4
> >
> > 1. How is it possible that you change spread spectrum of some clocks from
> > main Clock Controller, while this device is not a consumer of them?
> > Basically this means that this device does not have these clocks but yet
> > you claim that it needs to configure spread for them! It's contradictory
> > to me and nohing got explained in commit msg about it. I am pretty sure
> > I asked about this alrady.
>
> I digged my previous answer and it was pretty clear here:
>
> 18:44 <krzk> You can, but I still have the same concerns. How this
> device - which does not take any clock input, has no clocks at all - can
> depend on spread spectrum of some PLLs? Thsi device does not have clocks.
> 18:50 <krzk> device has no clocks, I checked now third time
> 18:50 <krzk> If device has clocks, it must have clocks property
>
The device where the spread spectrum properties are to be set already
contains "clocks" properties:
clk: clock-controller@30380000 {
compatible = "fsl,imx8mn-ccm";
reg = <0x30380000 0x10000>;
interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
#clock-cells = <1>;
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"
property to list the PLLs on which I want to enable and configure
spread spectrum.
Furthermore, spread spectrum cannot be considered a new device but
rather a property
available only for some of the clocks managed by the clock controller
manager (CCM).
Thanks and regards,
Dario
> So again, you do not need this property at all. I repeated it multiple
> times - you are supposed to use clocks property.
>
> >
> > 2. Why is this array flexible in size?
> >
> > Best regards,
> > Krzysztof
> >
>
> Best regards,
> Krzysztof
>
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
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. 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? Second, I do no ask you to configure spread spectrum on other clocks, only on the ones you intent to. List is fixed and ordered, so no problem with that. > property to list the PLLs on which I want to enable and configure > spread spectrum. > > Furthermore, spread spectrum cannot be considered a new device but > rather a property > available only for some of the clocks managed by the clock controller > manager (CCM). > My comment stands and that's a disagreement from me. Feel free to get second DT maintainer opinion, though. Best regards, Krzysztof
© 2016 - 2024 Red Hat, Inc.