Re: [PATCH v6 08/12] arm64: dts: nuvoton: Add initial ma35d1 device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Stephen,


On 2023/3/29 上午 10:19, Stephen Boyd wrote:
Quoting Jacky Huang (2023-03-28 19:03:24)
On 2023/3/29 上午 01:57, Stephen Boyd wrote:
Quoting Jacky Huang (2023-03-27 19:19:08)
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
new file mode 100644
index 000000000000..0740b0b218a7
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
@@ -0,0 +1,231 @@
[...]
+
+               L2_0: l2-cache0 {
Just l2-cache for the node name. Doesn't it go under the cpu0 node as
well?
This describes the level-2 cache which is external to and shared by cpu0
& cpu1.
And only level-1 cache is inside of CPU core.
L2_0 is must, because both cpu0 and cpu1 has a next-level-cache =
<&L2_0> property.
Ok. The name should just be l2-cache then, not l2-cache0.

OK, I will fix it.

Many identical example of l2-cache node can be found in arm64 dts, such
as k3-arm642.dtsi,
rk3328.dtsi, mt8195.dtsi, etc. Here is just a copy of similar arm64
multi-core SoCs.

So we would like to keep this unchanged. Is it OK for you? Thanks.

Mostly ok, yes.

Thank you for the agreement.


+
+       sys: system-management@40460000 {
+               compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
+               reg = <0x0 0x40460000 0x0 0x200>;
+
+               reset: reset-controller {
+                       compatible = "nuvoton,ma35d1-reset";
+                       #reset-cells = <1>;
+               };
+       };
+
+       clk: clock-controller@40460200 {
+               compatible = "nuvoton,ma35d1-clk", "syscon";
+               reg = <0x00000000 0x40460200 0x0 0x100>;
+               #clock-cells = <1>;
+               clocks = <&clk_hxt>;
+               nuvoton,sys = <&sys>;
+       };
It looks like the device at 40460000 is a reset and clock controller.
Just make it one node and register the clk or reset device as an
auxiliary device.
40460000 is for system control registers, including power contrl,
multifunction pin control,
usb phy control, IP reset control, power-on setting information, and
many other miscellaneous controls.
The registers of reset controller is only a subset of system control
registers.

40460200 is for clock controller which is independent of the system
control integration
The register base of clock controller is very close to system
controller, but in fact the two are independent.
What do you use the syscon for then? The clock driver must want to use
the syscon for something, implying that they are the same device.

The register lock mechanism is applied to protect many critical registers from false written.
The register lock control register is one register in system controller.
Some registers of the clock controller are lock protected. Not only clock controller, but other IP such as RTC, PWM, ADC, etc, also have lock protected registers. All these IP requires
syscon to access the lock/unlock control register in the system controller.
That's why we add a <&sys> to the clock controller.

Should we implement a ma35d1-sysctl driver to protect register_lock() and register_unlock() and export to those drivers?  If yes, we can remove the <&sys> from clock controller.


Best regards,
Jacky Huang











[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux