| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABGWkvrYJL9=zrPSFuEAgKO+9gDHD6RmCJM6Br6Le_eh578ETQ@mail.gmail.com>
Date: Thu, 7 Nov 2024 15:57:21 +0100
From: Dario Binacchi <dario.binacchi@...rulasolutions.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-amarula@...rulasolutions.com,
Abel Vesa <abelvesa@...nel.org>, 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 v3 1/8] dt-bindings: clock: imx8m-clock: support spread
spectrum clocking
Hello Krzysztof,
On Wed, Nov 6, 2024 at 3:13 PM Krzysztof Kozlowski <krzk@...nel.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@...rulasolutions.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@...80000 {
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@...rulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@...rulasolutions.com
www.amarulasolutions.com
Powered by blists - more mailing lists