| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 29 Mar 2023 10:03:24 +0800
From: Jacky Huang <ychuang570808@...il.com>
To: Stephen Boyd <sboyd@...nel.org>, gregkh@...uxfoundation.org,
jirislaby@...nel.org, krzysztof.kozlowski+dt@...aro.org,
lee@...nel.org, mturquette@...libre.com, p.zabel@...gutronix.de,
robh+dt@...nel.org
Cc: devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
arnd@...db.de, schung@...oton.com, mjchen@...oton.com,
Jacky Huang <ychuang3@...oton.com>
Subject: Re: [PATCH v6 08/12] arm64: dts: nuvoton: Add initial ma35d1 device
tree
Dear Stephen,
Thanks for your advice.
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 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2023 Nuvoton Technology Corp.
>> + * Author: Shan-Chun Hung <schung@...oton.com>
>> + * Jacky huang <ychuang3@...oton.com>
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
>> +#include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
>> +
>> +/ {
>> + compatible = "nuvoton,ma35d1";
>> + interrupt-parent = <&gic>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + cpus {
>> + #address-cells = <2>;
>> + #size-cells = <0>;
>> +
>> + cpu0: cpu@0 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a35";
>> + reg = <0x0 0x0>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_0>;
>> + };
>> +
>> + cpu1: cpu@1 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a35";
>> + reg = <0x0 0x1>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_0>;
>> + };
>> +
>> + 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.
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.
>> + compatible = "cache";
>> + cache-level = <2>;
>> + };
>> + };
>> +
>> + psci {
>> + compatible = "arm,psci-0.2";
>> + method = "smc";
>> + };
>> +
>> + timer {
>> + compatible = "arm,armv8-timer";
>> + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
>> + IRQ_TYPE_LEVEL_LOW)>, /* Physical Secure */
>> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
>> + IRQ_TYPE_LEVEL_LOW)>, /* Physical Non-Secure */
>> + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
>> + IRQ_TYPE_LEVEL_LOW)>, /* Virtual */
>> + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
>> + IRQ_TYPE_LEVEL_LOW)>; /* Hypervisor */
>> + clock-frequency = <12000000>;
> Remove this property. The frequency should be read by the driver.
I will remove it.
>> + interrupt-parent = <&gic>;
>> + };
> Please create an 'soc' node for the SoC to hold all the nodes that have
> a reg property.
OK, we will use soc node in the next version.
>> +
>> + sys: system-management@...60000 {
>> + 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@...60200 {
>> + 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.
>> +
>> + gic: interrupt-controller@...01000 {
>> + compatible = "arm,gic-400";
>> + reg = <0x0 0x50801000 0 0x1000>, /* GICD */
>> + <0x0 0x50802000 0 0x2000>, /* GICC */
>> + <0x0 0x50804000 0 0x2000>, /* GICH */
>> + <0x0 0x50806000 0 0x2000>; /* GICV */
>> + #interrupt-cells = <3>;
>> + interrupt-parent = <&gic>;
>> + interrupt-controller;
>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_RAW(0x13) |
>> + IRQ_TYPE_LEVEL_HIGH)>;
>> + };
>> +
>> + uart0:serial@...00000 {
> Add a space after :
I will fix it. Thank you.
>> + compatible = "nuvoton,ma35d1-uart";
>> + reg = <0x0 0x40700000 0x0 0x100>;
>> + interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk UART0_GATE>;
>> + status = "disabled";
>> + };
Best regards,
Jacky Huang
Powered by blists - more mailing lists