* [PATCH] dt-bindings: clk: Make example a bit clearer @ 2019-08-15 7:46 Uwe Kleine-König 2019-08-15 8:37 ` Alexander Dahl 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2019-08-15 7:46 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd Cc: Alexander Dahl, kernel, linux-clk, Uwe Kleine-König From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Previously the example used <&pll 2> in two places which made it harder than necessary to understand why this clock gets the parent of <&clkcon 0>. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Documentation/devicetree/bindings/clock/clock-bindings.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt index b646bbcf7f92..30aa63a2eecb 100644 --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt @@ -154,11 +154,11 @@ set to 0, or can be omitted if it is not followed by any non-zero entry. clock-names = "baud", "register"; assigned-clocks = <&clkcon 0>, <&pll 2>; - assigned-clock-parents = <&pll 2>; + assigned-clock-parents = <&pll 1>; assigned-clock-rates = <0>, <460800>; }; -In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> and +In this example the <&pll 1> clock is set as parent of clock <&clkcon 0> and the <&pll 2> clock is assigned a frequency value of 460800 Hz. Configuring a clock's parent and rate through the device node that consumes -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: clk: Make example a bit clearer 2019-08-15 7:46 [PATCH] dt-bindings: clk: Make example a bit clearer Uwe Kleine-König @ 2019-08-15 8:37 ` Alexander Dahl 2019-08-15 9:50 ` Uwe Kleine-König 0 siblings, 1 reply; 4+ messages in thread From: Alexander Dahl @ 2019-08-15 8:37 UTC (permalink / raw) To: Uwe Kleine-König Cc: Michael Turquette, Stephen Boyd, kernel, linux-clk, Uwe Kleine-König Hello Uwe, thanks for your effort on improving this. See below. Am Donnerstag, 15. August 2019, 09:46:04 CEST schrieb Uwe Kleine-König: > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Previously the example used <&pll 2> in two places which made it harder > than necessary to understand why this clock gets the parent of > <&clkcon 0>. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/devicetree/bindings/clock/clock-bindings.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt > b/Documentation/devicetree/bindings/clock/clock-bindings.txt index > b646bbcf7f92..30aa63a2eecb 100644 > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt > @@ -154,11 +154,11 @@ set to 0, or can be omitted if it is not followed by > any non-zero entry. clock-names = "baud", "register"; > > assigned-clocks = <&clkcon 0>, <&pll 2>; > - assigned-clock-parents = <&pll 2>; > + assigned-clock-parents = <&pll 1>; > assigned-clock-rates = <0>, <460800>; > }; > > -In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> > and +In this example the <&pll 1> clock is set as parent of clock <&clkcon > 0> and the <&pll 2> clock is assigned a frequency value of 460800 Hz. > > Configuring a clock's parent and rate through the device node that consumes With your change the assigned-clock-parent is a clock also listed with the clocks a few lines above, don't know if that's less puzzling? Let me repeat for the audience here (I initially asked on #armlinux IRC channel), what I did not understand, maybe someone else has an idea how to better explain it. First: I'm experimenting on a SAMA5D27-SOM1-EK board and want to use one of the PCKx to output on some easy accessible pin (to see the signal on a scope). On our real hardware that clock will be input for another IC on the board connected to the EBI external memory interface. I wrote a dummy driver to prepare/enable that clock, nothing sophisticated. The dts nodes working now are these: ahb { ebi: ebi@10000000 { status = "okay"; dummy_fpga: dummy-fpga@2 { compatible = "thorsis,dummy-fpga"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_dummy_fpga_default>; status = "okay"; reg = <0x2 0x0 0x10000>; clocks = <&pck1>; assigned-clocks = <&prog1>; assigned-clock-parents = <&main>; assigned-clock-rates = <6000000>; /* assigned-clock-parents = <&clk32k>; assigned-clock-rates = <16384>; */ atmel,smc-bus-width = <8>; atmel,smc-read-mode = "nrd"; atmel,smc-write-mode = "nwe"; atmel,smc-exnw-mode = "disabled"; atmel,smc-ncs-rd-setup-ns = <20>; atmel,smc-nrd-setup-ns = <20>; atmel,smc-ncs-wr-setup-ns = <20>; atmel,smc-nwe-setup-ns = <20>; atmel,smc-ncs-rd-pulse-ns = <40>; atmel,smc-nrd-pulse-ns = <40>; atmel,smc-ncs-wr-pulse-ns = <40>; atmel,smc-nwe-pulse-ns = <40>; atmel,smc-nwe-cycle-ns = <110>; atmel,smc-nrd-cycle-ns = <110>; atmel,smc-tdf-ns = <0>; }; }; apb { pinctrl@fc038000 { pinctrl_dummy_fpga_default: dummy_fpga_default { pinmux = <PIN_PD6__PCK1>; bias-disable; }; }; }; }; The chain (desired) is like this: crystal -> mainck -> prog1 -> pck1 -> fpgack You can choose slck, mainck, pllack, upllck, mck, or audiopllck (names from sama5d2 series datasheet) as source clock for each channel of the programmable clock controller. That controller is modelled as prog0, prog1, prog2 and pck0, pck1, pck2 in dts. Now the three puzzling things from the clock-bindings docs: There are in fact three lists, assigned-clocks contains clocks you want to assign parent or rate, those don't need to be the clocks your node uses, but may be any clock higher up in the chain which affects your node. Right? That in itself is puzzling. Beyond that: could I have added those properties to the nodes pck1 or prog1 as well and which one would be the right node to choose? The last hard to understand part from the example code (not the describing text) is the linkage between those three lists. Each entry in assigned-clock- parents and assigned-clock-rates must correspond to the same index list member of assigned-clocks, but you can omit entries in the back. Suggestion to improve the example: diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/ Documentation/devicetree/bindings/clock/clock-bindings.txt index 2ec489eebe72..df098f73e901 100644 --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt @@ -159,7 +159,9 @@ set to 0, or can be omitted if it is not followed by any non-zero entry. }; In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> and -the <&pll 2> clock is assigned a frequency value of 460800 Hz. +the <&pll 2> clock is assigned a frequency value of 460800 Hz. A parent +setting for <&pll 2> is omitted (end of list) and rate setting for <&clkcon 0> +is skipped because set to <0>. Configuring a clock's parent and rate through the device node that consumes the clock can be done only for clocks that have a single user. Specifying Kind regards Alex ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: clk: Make example a bit clearer 2019-08-15 8:37 ` Alexander Dahl @ 2019-08-15 9:50 ` Uwe Kleine-König 2019-08-16 8:53 ` Alexander Dahl 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2019-08-15 9:50 UTC (permalink / raw) To: Alexander Dahl; +Cc: Stephen Boyd, Michael Turquette, linux-clk, kernel On Thu, Aug 15, 2019 at 10:37:54AM +0200, Alexander Dahl wrote: > Hello Uwe, > > thanks for your effort on improving this. See below. > > Am Donnerstag, 15. August 2019, 09:46:04 CEST schrieb Uwe Kleine-König: > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > Previously the example used <&pll 2> in two places which made it harder > > than necessary to understand why this clock gets the parent of > > <&clkcon 0>. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/clock/clock-bindings.txt | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt > > b/Documentation/devicetree/bindings/clock/clock-bindings.txt index > > b646bbcf7f92..30aa63a2eecb 100644 > > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt > > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt > > @@ -154,11 +154,11 @@ set to 0, or can be omitted if it is not followed by > > any non-zero entry. clock-names = "baud", "register"; > > > > assigned-clocks = <&clkcon 0>, <&pll 2>; > > - assigned-clock-parents = <&pll 2>; > > + assigned-clock-parents = <&pll 1>; > > assigned-clock-rates = <0>, <460800>; > > }; > > > > -In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> > > and +In this example the <&pll 1> clock is set as parent of clock <&clkcon > > 0> and the <&pll 2> clock is assigned a frequency value of 460800 Hz. > > > > Configuring a clock's parent and rate through the device node that consumes > > With your change the assigned-clock-parent is a clock also listed with the > clocks a few lines above, don't know if that's less puzzling? > > Let me repeat for the audience here (I initially asked on #armlinux IRC > channel), what I did not understand, maybe someone else has an idea how to > better explain it. > > First: I'm experimenting on a SAMA5D27-SOM1-EK board and want to use one of > the PCKx to output on some easy accessible pin (to see the signal on a scope). > On our real hardware that clock will be input for another IC on the board > connected to the EBI external memory interface. I wrote a dummy driver to > prepare/enable that clock, nothing sophisticated. The dts nodes working now > are these: > > ahb { > ebi: ebi@10000000 { > status = "okay"; > > dummy_fpga: dummy-fpga@2 { > compatible = "thorsis,dummy-fpga"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_dummy_fpga_default>; > status = "okay"; > reg = <0x2 0x0 0x10000>; > clocks = <&pck1>; > assigned-clocks = <&prog1>; > assigned-clock-parents = <&main>; > assigned-clock-rates = <6000000>; > /* > assigned-clock-parents = <&clk32k>; > assigned-clock-rates = <16384>; > */ > > atmel,smc-bus-width = <8>; > atmel,smc-read-mode = "nrd"; > atmel,smc-write-mode = "nwe"; > atmel,smc-exnw-mode = "disabled"; > > atmel,smc-ncs-rd-setup-ns = <20>; > atmel,smc-nrd-setup-ns = <20>; > atmel,smc-ncs-wr-setup-ns = <20>; > atmel,smc-nwe-setup-ns = <20>; > > atmel,smc-ncs-rd-pulse-ns = <40>; > atmel,smc-nrd-pulse-ns = <40>; > atmel,smc-ncs-wr-pulse-ns = <40>; > atmel,smc-nwe-pulse-ns = <40>; > > atmel,smc-nwe-cycle-ns = <110>; > atmel,smc-nrd-cycle-ns = <110>; > > atmel,smc-tdf-ns = <0>; > }; > }; > > apb { > pinctrl@fc038000 { > pinctrl_dummy_fpga_default: dummy_fpga_default { > pinmux = <PIN_PD6__PCK1>; > bias-disable; > }; > }; > }; > }; > > The chain (desired) is like this: crystal -> mainck -> prog1 -> pck1 -> fpgack > > You can choose slck, mainck, pllack, upllck, mck, or audiopllck (names from > sama5d2 series datasheet) as source clock for each channel of the programmable > clock controller. That controller is modelled as prog0, prog1, prog2 and pck0, > pck1, pck2 in dts. > > Now the three puzzling things from the clock-bindings docs: > > There are in fact three lists, assigned-clocks contains clocks you want to > assign parent or rate, those don't need to be the clocks your node uses, but > may be any clock higher up in the chain which affects your node. Right? That > in itself is puzzling. > > Beyond that: could I have added those properties to the nodes pck1 or prog1 as > well and which one would be the right node to choose? > > The last hard to understand part from the example code (not the describing > text) is the linkage between those three lists. Each entry in assigned-clock- > parents and assigned-clock-rates must correspond to the same index list member > of assigned-clocks, but you can omit entries in the back. Suggestion to > improve the example: > > > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/ > Documentation/devicetree/bindings/clock/clock-bindings.txt > index 2ec489eebe72..df098f73e901 100644 > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt > @@ -159,7 +159,9 @@ set to 0, or can be omitted if it is not followed by any > non-zero entry. > }; > > In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> and > -the <&pll 2> clock is assigned a frequency value of 460800 Hz. > +the <&pll 2> clock is assigned a frequency value of 460800 Hz. A parent > +setting for <&pll 2> is omitted (end of list) and rate setting for <&clkcon > 0> > +is skipped because set to <0>. Maybe pick <&pll 3> instead of <&pll 1> in my patch and merge the two changes? Or drop clocks and clock-names from the example because otherwise unrelated clocks are modified which shouldn't be done. The result is below. Best regards Uwe ---->8---- From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de> Date: Thu, 15 Aug 2019 11:48:25 +0200 Subject: [PATCH] dt-bindings: clk: Make example a bit clearer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the example used <&pll 2> in two places which made it harder than necessary to understand why this clock gets the parent of <&clkcon 0>. Also describe why <&pll 2> isn't reparented and <&clkcon 0> gets no rate assigned. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- .../devicetree/bindings/clock/clock-bindings.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt index b646bbcf7f92..1d4942380918 100644 --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt @@ -150,16 +150,18 @@ set to 0, or can be omitted if it is not followed by any non-zero entry. compatible = "fsl,imx-uart"; reg = <0xa000 0x1000>; ... - clocks = <&osc 0>, <&pll 1>; - clock-names = "baud", "register"; + clocks = ... + clock-names = ... assigned-clocks = <&clkcon 0>, <&pll 2>; - assigned-clock-parents = <&pll 2>; + assigned-clock-parents = <&pll 1>; assigned-clock-rates = <0>, <460800>; }; -In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> and -the <&pll 2> clock is assigned a frequency value of 460800 Hz. +In this example the <&pll 1> clock is set as parent of clock <&clkcon 0> and +the <&pll 2> clock is assigned a frequency value of 460800 Hz. A parent +setting for <&pll 2> is omitted (end of list) and rate setting for <&clkcon 0> +is skipped because set to <0>. Configuring a clock's parent and rate through the device node that consumes the clock can be done only for clocks that have a single user. Specifying -- 2.20.1 -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dt-bindings: clk: Make example a bit clearer 2019-08-15 9:50 ` Uwe Kleine-König @ 2019-08-16 8:53 ` Alexander Dahl 0 siblings, 0 replies; 4+ messages in thread From: Alexander Dahl @ 2019-08-16 8:53 UTC (permalink / raw) To: linux-clk; +Cc: Uwe Kleine-König, Stephen Boyd, Michael Turquette, kernel Hello Uwe, Am Donnerstag, 15. August 2019, 11:50:59 CEST schrieb Uwe Kleine-König: > Maybe pick <&pll 3> instead of <&pll 1> in my patch and merge the two > changes? Or drop clocks and clock-names from the example because > otherwise unrelated clocks are modified which shouldn't be done. The > result is below. I like that result below with the dots for the 'clock' properties and the additional explanations. Can we resend that as new patch? Greets Alex > ---->8---- > From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de> > Date: Thu, 15 Aug 2019 11:48:25 +0200 > Subject: [PATCH] dt-bindings: clk: Make example a bit clearer > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Previously the example used <&pll 2> in two places which made it harder > than necessary to understand why this clock gets the parent of > <&clkcon 0>. Also describe why <&pll 2> isn't reparented and <&clkcon 0> > gets no rate assigned. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > .../devicetree/bindings/clock/clock-bindings.txt | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt > b/Documentation/devicetree/bindings/clock/clock-bindings.txt index > b646bbcf7f92..1d4942380918 100644 > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt > @@ -150,16 +150,18 @@ set to 0, or can be omitted if it is not followed by > any non-zero entry. compatible = "fsl,imx-uart"; > reg = <0xa000 0x1000>; > ... > - clocks = <&osc 0>, <&pll 1>; > - clock-names = "baud", "register"; > + clocks = ... > + clock-names = ... > > assigned-clocks = <&clkcon 0>, <&pll 2>; > - assigned-clock-parents = <&pll 2>; > + assigned-clock-parents = <&pll 1>; > assigned-clock-rates = <0>, <460800>; > }; > > -In this example the <&pll 2> clock is set as parent of clock <&clkcon 0> > and -the <&pll 2> clock is assigned a frequency value of 460800 Hz. > +In this example the <&pll 1> clock is set as parent of clock <&clkcon 0> > and +the <&pll 2> clock is assigned a frequency value of 460800 Hz. A > parent +setting for <&pll 2> is omitted (end of list) and rate setting for > <&clkcon 0> +is skipped because set to <0>. > > Configuring a clock's parent and rate through the device node that consumes > the clock can be done only for clocks that have a single user. Specifying ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-16 8:53 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-08-15 7:46 [PATCH] dt-bindings: clk: Make example a bit clearer Uwe Kleine-König 2019-08-15 8:37 ` Alexander Dahl 2019-08-15 9:50 ` Uwe Kleine-König 2019-08-16 8:53 ` Alexander Dahl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox