From: Alexander Dahl <ada@thorsis.com>
To: "Uwe Kleine-König" <uwe@kleine-koenig.org>
Cc: "Michael Turquette" <mturquette@baylibre.com>,
"Stephen Boyd" <sboyd@kernel.org>,
kernel@pengutronix.de, linux-clk@vger.kernel.org,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH] dt-bindings: clk: Make example a bit clearer
Date: Thu, 15 Aug 2019 10:37:54 +0200 [thread overview]
Message-ID: <1870872.EFtpEp3zHr@ada> (raw)
In-Reply-To: <20190815074604.5416-1-uwe@kleine-koenig.org>
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
next prev parent reply other threads:[~2019-08-15 8:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2019-08-15 9:50 ` Uwe Kleine-König
2019-08-16 8:53 ` Alexander Dahl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1870872.EFtpEp3zHr@ada \
--to=ada@thorsis.com \
--cc=kernel@pengutronix.de \
--cc=linux-clk@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=sboyd@kernel.org \
--cc=u.kleine-koenig@pengutronix.de \
--cc=uwe@kleine-koenig.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).