Re: [PATCH V4 3/5] arm64: dts: nuvoton: Add initial support for MA35D1
From: Krzysztof Kozlowski <hidden>
Date: 2022-05-13 06:58:07
Also in:
linux-clk, linux-devicetree, lkml
On 13/05/2022 08:48, Jacky Huang wrote:
quoted
quoted
+ + hxt_24m: hxt_24mhz {No underscores in node name. Generic node names, so "clock-X" or "clock-some-suffix"OK, I will modify it as hxt-24m: hxt-24mhz
No, it is not a generic node name. Please read my reply again.
quoted
quoted
+ compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>;This does not look like property of SoC. Where is this clock defined? In the SoC or on the board?It's an external crystal on the board. I add this node, because it's the clock source of clock controller. It always present on all ma35d1 boards. clk: clock-controller@40460200 { compatible = "nuvoton,ma35d1-clk"; reg = <0x0 0x40460200 0x0 0x100>; #clock-cells = <1>; clocks = <&hxt_24m>; clock-names = "HXT_24MHz"; ...quoted
quoted
+ clock-output-names = "HXT_24MHz"; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | + IRQ_TYPE_LEVEL_LOW)>; + clock-frequency = <12000000>; + }; + + sys: system-controller@40460000 { + compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";Why is this a simple-mfd if there are no children here? What do you want to instantiate here?It's not a device, but a set of registers for system level control. I want to provide a register base mapping for other devices to access system control registers.
This does not explain why you need simple-mfd. simple-mfd is not for providing a register base mapping for other devices.
quoted
Where is the nuvoton,ma35d1-sys compatible documented?OK, I will add the compatible document in next version.quoted
quoted
+ reg = <0x0 0x40460000 0x0 0x400>; + }; + + reset: reset-controller { + compatible = "nuvoton,ma35d1-reset";Also not documented.I will also add the document for it.
All of these should fail on checkpatch which points that you either did not run it or ignored the result. Please run checkpatch on all your submissions to Linux kernel and be sure that there is no warning or error. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel