[PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Krzysztof Kozlowski]

On 10/06/2022 00:29, Tomer Maimon wrote:
>>> +     clk_refclk: clk-refclk {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <25000000>;
>>
>> Ignored comment.
> Could we use it as a default clock-frequency?

In DTS? If my assumption, that this clock is not on SoC itself, is
correct, then the answer is no, you cannot. The clock physically sits on
the board, so it is defined by board DTS. Feel free to embed in SoC DTSI
most of the clock properties, but the core property - frequency - must
be outside.

>>
>>> +             clock-output-names = "refclk";
>>> +     };
>>> +
>>> +     /* external reference clock for cpu. float in normal operation */
>>> +     clk_sysbypck: clk-sysbypck {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <1000000000>;
>>
>> Ignored comment.
> same as above
>>
>>> +             clock-output-names = "sysbypck";
>>> +     };
>>> +
>>> +     /* external reference clock for MC. float in normal operation */
>>> +     clk_mcbypck: clk-mcbypck {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <1050000000>;
> same as above
>>> +             clock-output-names = "mcbypck";
>>> +     };
>>> +
>>> +     soc {
>>> +             #address-cells = <2>;
>>> +             #size-cells = <2>;
>>> +             compatible = "simple-bus";
>>> +             interrupt-parent = <&gic>;
>>> +             ranges;
>>> +
>>> +             gcr: gcr at f0800000 {
> I understand it sounds generic but I try to be as much compatible with NPCM7XX
> https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L91

Then fix NPCM7XX. Please do not multiple bad choices because it looks
similar. Fix the other wrong one.

>>
>> Ignored comment.
>>
>>> +                     compatible = "nuvoton,npcm845-gcr", "syscon",
>>> +                             "simple-mfd";
>>
>> This is not a simple-mfd... I see original bindings defined it that way,
>> but why? I think they should be corrected - remove simple-mfd from the
>> bindings and DTS.
> will remove in both places in V3
>>
>>
>>> +                     reg = <0x0 0xf0800000 0x0 0x1000>;
>>> +             };
>>> +
>>> +             gic: interrupt-controller at dfff9000 {
>>> +                     compatible = "arm,gic-400";
>>> +                     reg = <0x0 0xdfff9000 0x0 0x1000>,
>>> +                           <0x0 0xdfffa000 0x0 0x2000>,
>>> +                           <0x0 0xdfffc000 0x0 0x2000>,
>>> +                           <0x0 0xdfffe000 0x0 0x2000>;
>>> +                     interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>>> +                     #interrupt-cells = <3>;
>>> +                     interrupt-controller;
>>> +                     #address-cells = <0>;
>>> +                     ppi-partitions {
>>> +                             ppi_cluster0: interrupt-partition-0 {
>>> +                                     affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
>>> +                             };
>>> +                     };
>>> +             };
>>> +     };
>>> +
>>> +     ahb {
>>> +             #address-cells = <2>;
>>> +             #size-cells = <2>;
>>> +             compatible = "simple-bus";
>>> +             interrupt-parent = <&gic>;
> 
>>> +             ranges;
>>> +
>>> +             rstc: rstc at f0801000 {
>>
>> Ignored comment.
>>
> I understand it sounds generic but I try to be as much compatible with NPCM7XX
> https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L109

Fix 7xx.


Best regards,
Krzysztof