* Stephen Boyd <sboyd@xxxxxxxxxx> [190918 23:29]:
> Quoting Tony Lindgren (2019-09-18 13:53:44)
> > Hi,
> >
> > * Stephen Boyd <sboyd@xxxxxxxxxx> [190918 18:08]:
> > > Quoting Tony Lindgren (2019-09-08 12:42:41)
> > > > Or do you have some better ideas on how to name a clock controller
> > > > in the device tree?
> > > >
> > >
> > > Why does the name of the clock controller or clkdm_name matter? Using a
> > > string from reg-names smells like a workaround to describe some sort of
> > > linkage between things that isn't being described in DT today.
> >
> > Correct. This problem will eventually disappear with genpd
> > handling the clockdomains.
> >
> > But currently the clockdomain name is parsed from the dt node
> > name, which is not standard practise. Using reg-names
> > is a standard binding, and it's usage follows the standand
> > here to describe the reg range.
> >
> > Then eventually with genpd, the reg-names will just become
> > optinoal. But until that happens the $subject patch fixes
> > issues as described in the patch.
> >
>
> Is anything broken? It looks like the hidden dependency on the node name
> is being changed to be a slightly less hidden dependency on reg-names.
Yes I agree. This is still way better than relying on dts node names :)
> reg-names is supposed to be an optional property, so we're trading one
> thing for another. I still don't understand the reasoning here, but if
> Tero is happy to ack/review this change then I'm not too worried about
> it assuming the reg-names property eventually becomes optional. Just
> seems like more work and DT churn for no to little gain?
Yeah with genpd the reg-names becomes optional.
What is currently broken is we can get a wrong clockdomain or no
clockdomain for a clock if the clock controller has multiple
domains.
This happens at least with rng for omap4 and 5 where l4_per has two
clock domains (l4_per and l4_sec). I'll be posting rng dts patches
for all the SoCs after -rc1, but below is what's needed for rng on
omap4 for example after the $subject patch for reference. See the
addition of "l4_secure_clkctrl: clock@1a0" node below.
The other option would be to search and replace the "clk@" domains
with "foo@" type domains with more churn. See for example the
changes from "clk@" to "foo@" for dra7 done in b5f8ffbb6fad:
$ git show b5f8ffbb6fad | grep -C1 "clk@"
- mpu_clkctrl: clk@20 {
+ mpu_clkctrl: mpu-clkctrl@20 {
--
- ipu_clkctrl: clk@40 {
+ ipu1_clkctrl: ipu1-clkctrl@20 {
...
And we cannot mix "clk@" naming and "foo@" naming as a flag for all
instances is set by "clk@" naming during clkctrl clock init. So all
the clock domain nodes would need to be renamed just to add l4_sec
domain for omap4 and 5.
Anyways, if acceptable, an immutable branch against v5.3 or v5.4-rc1
with just the $subject patch would be great so I can merge it in too
for the related rng changes for v5.5.
I'm not yet sure if the $subject patch is needed for some SoC as a
fix together with a dts change to add the reg-names, but it is
possible.
Regards,
Tony
8< -----------------------
diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -653,6 +653,11 @@
};
};
+/* RNG is used by secure mode and not accessible */
+&rng_target {
+ status = "disabled";
+};
+
/* Configure pwm clock source for timers 8 & 9 */
&timer8 {
assigned-clocks = <&abe_clkctrl OMAP4_TIMER8_CLKCTRL 24>;
diff --git a/arch/arm/boot/dts/omap4-l4.dtsi b/arch/arm/boot/dts/omap4-l4.dtsi
--- a/arch/arm/boot/dts/omap4-l4.dtsi
+++ b/arch/arm/boot/dts/omap4-l4.dtsi
@@ -2010,12 +2010,26 @@
};
};
- target-module@90000 { /* 0x48090000, ap 57 2a.0 */
- compatible = "ti,sysc";
- status = "disabled";
+ rng_target: target-module@90000 { /* 0x48090000, ap 57 2a.0 */
+ compatible = "ti,sysc-omap2", "ti,sysc";
+ reg = <0x91fe0 0x4>,
+ <0x91fe4 0x4>;
+ reg-names = "rev", "sysc";
+ ti,sysc-mask = <(SYSC_OMAP2_AUTOIDLE)>;
+ ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+ <SYSC_IDLE_NO>;
+ /* Domains (P, C): l4per_pwrdm, l4_secure_clkdm */
+ clocks = <&l4_secure_clkctrl OMAP4_RNG_CLKCTRL 0>;
+ clock-names = "fck";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x90000 0x2000>;
+
+ rng: rng@0 {
+ compatible = "ti,omap4-rng";
+ reg = <0x0 0x2000>;
+ interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+ };
};
target-module@96000 { /* 0x48096000, ap 37 26.0 */
diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi b/arch/arm/boot/dts/omap44xx-clocks.dtsi
--- a/arch/arm/boot/dts/omap44xx-clocks.dtsi
+++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi
@@ -1284,8 +1284,14 @@
reg = <0x20 0x144>;
#clock-cells = <2>;
};
- };
+ l4_secure_clkctrl: clock@1a0 {
+ compatible = "ti,clkctrl";
+ reg = <0x1a0 0x28>;
+ reg-names = "l4_secure";
+ #clock-cells = <2>;
+ };
+ };
};
&prm {
diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
--- a/drivers/clk/ti/clk-44xx.c
+++ b/drivers/clk/ti/clk-44xx.c
@@ -604,6 +604,12 @@ static const struct omap_clkctrl_reg_data omap4_l4_per_clkctrl_regs[] __initcons
{ 0 },
};
+static const struct
+omap_clkctrl_reg_data omap4_l4_secure_clkctrl_regs[] __initconst = {
+ { OMAP4_RNG_CLKCTRL, NULL, CLKF_HW_SUP | CLKF_SOC_NONSEC, "" },
+ { 0 },
+};
+
static const struct omap_clkctrl_bit_data omap4_gpio1_bit_data[] __initconst = {
{ 8, TI_CLK_GATE, omap4_gpio2_dbclk_parents, NULL },
{ 0 },
@@ -691,6 +697,7 @@ const struct omap_clkctrl_data omap4_clkctrl_data[] __initconst = {
{ 0x4a009220, omap4_l3_gfx_clkctrl_regs },
{ 0x4a009320, omap4_l3_init_clkctrl_regs },
{ 0x4a009420, omap4_l4_per_clkctrl_regs },
+ { 0x4a0095a0, omap4_l4_secure_clkctrl_regs },
{ 0x4a307820, omap4_l4_wkup_clkctrl_regs },
{ 0x4a307a20, omap4_emu_sys_clkctrl_regs },
{ 0 },
diff --git a/include/dt-bindings/clock/omap4.h b/include/dt-bindings/clock/omap4.h
--- a/include/dt-bindings/clock/omap4.h
+++ b/include/dt-bindings/clock/omap4.h
@@ -124,6 +124,11 @@
#define OMAP4_UART4_CLKCTRL OMAP4_CLKCTRL_INDEX(0x158)
#define OMAP4_MMC5_CLKCTRL OMAP4_CLKCTRL_INDEX(0x160)
+/* l4_secure clocks */
+#define OMAP4_L4_SECURE_CLKCTRL_OFFSET 0x1a0
+#define OMAP4_L4_SECURE_CLKCTRL_INDEX(offset) ((offset) - OMAP4_L4_SECURE_CLKCTRL_OFFSET)
+#define OMAP4_RNG_CLKCTRL OMAP4_L4_SECURE_CLKCTRL_INDEX(0x1c0)
+
/* l4_wkup clocks */
#define OMAP4_L4_WKUP_CLKCTRL OMAP4_CLKCTRL_INDEX(0x20)
#define OMAP4_WD_TIMER2_CLKCTRL OMAP4_CLKCTRL_INDEX(0x30)
--
2.23.0
