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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 1 Nov 2022 18:02:34 +0900
From:   Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Arnd Bergmann <arnd@...db.de>, Olof Johansson <olof@...om.net>,
        Masami Hiramatsu <mhiramat@...nel.org>
Cc:     soc@...nel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] arm64: dts: uniphier: Add NX1 SoC and boards support

Hi Krzysztof,

Sorry for late and thank you for reviewing.

On 2022/10/27 22:36, Krzysztof Kozlowski wrote:
> On 27/10/2022 00:51, Kunihiko Hayashi wrote:
>> Initial version of devicetree sources for NX1 SoC and boards.
>>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> diff --git a/arch/arm64/boot/dts/socionext/uniphier-nx1.dtsi
>> b/arch/arm64/boot/dts/socionext/uniphier-nx1.dtsi
>> new file mode 100644
>> index 000000000000..e401763de86e
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/socionext/uniphier-nx1.dtsi

(snip)

>> +	clocks {
>> +		refclk: ref {
> 
> Generic node name, so at least "clock" prefix or suffix.

I see. I'll change the node name with "clock".

>> +			compatible = "fixed-clock";
>> +			#clock-cells = <0>;
>> +			clock-frequency = <25000000>;
> 
> Are you sure the clock is a property of a SoC? IOW, it is physically on
> the SoC? If not, define entire clock or its frequency by boards.

Indeed. This shows an external oscillator outside of a SoC.
I'll consider to define the node or the property by board devicetree.

>> +		};
>> +	};
>> +
>> +	emmc_pwrseq: emmc-pwrseq {
>> +		compatible = "mmc-pwrseq-emmc";
>> +		reset-gpios = <&gpio UNIPHIER_GPIO_PORT(3, 0) GPIO_ACTIVE_LOW>;
>> +	};
>> +
>> +	timer {
>> +		compatible = "arm,armv8-timer";
>> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_HIGH>;
>> +	};
>> +
>> +	thermal-zones {
>> +		cpu-thermal {
>> +			polling-delay-passive = <250>;	/* 250ms */
>> +			polling-delay = <1000>;		/* 1000ms */
>> +			thermal-sensors = <&pvtctl>;
>> +
>> +			trips {
>> +				cpu_crit: cpu-crit {
>> +					temperature = <120000>;	/* 120C */
>> +					hysteresis = <2000>;
>> +					type = "critical";
>> +				};
>> +				cpu_alert: cpu-alert {
>> +					temperature = <110000>;	/* 110C */
>> +					hysteresis = <2000>;
>> +					type = "passive";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&cpu_alert>;
>> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		secure-memory@...00000 {
>> +			reg = <0x0 0x21000000 0x0 0x01000000>;
>> +			no-map;
>> +		};
>> +	};
>> +
>> +	soc@0 {
>> +		compatible = "simple-bus";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges = <0 0 0 0xffffffff>;
>> +
>> +		sysctrl: sysctrl@...40000 {
> 
> Node name: system-controller or syscon

I see. I'll rename it to a generic name.
And also other system controller nodes in the same way.

> 
>> +			compatible = "socionext,uniphier-nx1-sysctrl",
> 
> This looks undocumented.
> 
> Did you run `make dtbs_check`?

I've done it, however, I missed the dt-binding document for the system
controller. I'll need to add a system controller's document before
this patch.

>> +				     "simple-mfd", "syscon";
>> +			reg = <0x11840000 0x10000>;
>> +
>> +			sys_clk: clock {
> 
> Node name: clock-controller

Need to rename to a generic name in the same way. After this as well.

>> +				compatible = "socionext,uniphier-nx1-clock";
>> +				#clock-cells = <1>;
>> +			};
>> +
>> +			sys_rst: reset {
> 
> reset-controller
> 
>> +				compatible = "socionext,uniphier-nx1-reset";
>> +				#reset-cells = <1>;
>> +			};
>> +
>> +			watchdog {
>> +				compatible = "socionext,uniphier-wdt";
>> +			};
>> +
>> +			pvtctl: thermal-sensor {
>> +				compatible = "socionext,uniphier-nx1-thermal";
>> +				interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
>> +				#thermal-sensor-cells = <0>;
>> +				socionext,tmod-calibration = <0x0f22 0x68ee>;
>> +			};
>> +		};
>> +
>> +		spi0: spi@...06000 {
>> +			compatible = "socionext,uniphier-scssi";
>> +			status = "disabled";
>> +			reg = <0x14006000 0x100>;
> 
> Reg is second property. Status goes last. The same in other nodes.

Hmm, I've put "status" here according to the existing (uniphier's) DT policy
and this should rewrite the policy. Is there documentation somewhere that
recommends the order? Or, should I refer to previous comments?

>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_spi0>;
>> +			clocks = <&peri_clk 11>;
>> +			resets = <&peri_rst 11>;
>> +		};
>> +
>> +		spi1: spi@...06100 {
>> +			compatible = "socionext,uniphier-scssi";
>> +			status = "disabled";
>> +			reg = <0x14006100 0x100>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_spi1>;
>> +			clocks = <&peri_clk 12>;
>> +			resets = <&peri_rst 12>;
>> +		};
>> +
>> +		serial0: serial@...06800 {
>> +			compatible = "socionext,uniphier-uart";
>> +			status = "disabled";
>> +			reg = <0x14006800 0x40>;
>> +			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_uart0>;
>> +			clocks = <&peri_clk 0>;
>> +			resets = <&peri_rst 0>;
>> +		};
>> +
>> +		serial1: serial@...06900 {
>> +			compatible = "socionext,uniphier-uart";
>> +			status = "disabled";
>> +			reg = <0x14006900 0x40>;
>> +			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_uart1>;
>> +			clocks = <&peri_clk 1>;
>> +			resets = <&peri_rst 1>;
>> +		};
>> +
>> +		serial2: serial@...06a00 {
>> +			compatible = "socionext,uniphier-uart";
>> +			status = "disabled";
>> +			reg = <0x14006a00 0x40>;
>> +			interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_uart2>;
>> +			clocks = <&peri_clk 2>;
>> +			resets = <&peri_rst 2>;
>> +		};
>> +
>> +		serial3: serial@...06b00 {
>> +			compatible = "socionext,uniphier-uart";
>> +			status = "disabled";
>> +			reg = <0x14006b00 0x40>;
>> +			interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_uart3>;
>> +			clocks = <&peri_clk 3>;
>> +			resets = <&peri_rst 3>;
>> +		};
>> +
>> +		gpio: gpio@...07000 {
>> +			compatible = "socionext,uniphier-gpio";
>> +			reg = <0x14007000 0x200>;
>> +			interrupt-parent = <&aidet>;
>> +			interrupt-controller;
>> +			#interrupt-cells = <2>;
>> +			gpio-controller;
>> +			#gpio-cells = <2>;
>> +			gpio-ranges = <&pinctrl 0 0 0>,
>> +				      <&pinctrl 49 0 0>,
>> +				      <&pinctrl 53 0 0>,
>> +				      <&pinctrl 96 0 0>,
>> +				      <&pinctrl 120 0 0>;
>> +			gpio-ranges-group-names = "gpio_range0",
>> +						  "gpio_range1",
>> +						  "gpio_range2",
>> +						  "gpio_range3",
>> +						  "gpio_range4";
>> +			ngpios = <126>;
>> +			socionext,interrupt-ranges = <0 48 6>;
>> +		};
>> +
>> +		eth: ethernet@...00000 {
>> +			compatible = "socionext,uniphier-nx1-ave4";
>> +			status = "disabled";
>> +			reg = <0x15000000 0x8500>;
>> +			interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_ether_rgmii>;
>> +			clock-names = "ether";
>> +			clocks = <&sys_clk 6>;
>> +			reset-names = "ether";
>> +			resets = <&sys_rst 6>;
>> +			phy-mode = "rgmii-id";
>> +			local-mac-address = [00 00 00 00 00 00];
>> +			socionext,syscon-phy-mode = <&soc_glue 0>;
>> +
>> +			mdio: mdio {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +			};
>> +		};
>> +
>> +		usb: usb@...00000 {
>> +			compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>> +			status = "disabled";
>> +			reg = <0x15a00000 0xcd00>;
>> +			interrupt-names = "host";
>> +			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_usb0>, <&pinctrl_usb1>;
>> +			clock-names = "ref", "bus_early", "suspend";
>> +			clocks = <&sys_clk 12>, <&sys_clk 12>, <&sys_clk 12>;
>> +			resets = <&usb_rst 15>;
>> +			phys = <&usb_hsphy0>, <&usb_hsphy1>,
>> +			       <&usb_ssphy0>, <&usb_ssphy1>;
>> +			dr_mode = "host";
>> +		};
>> +
>> +		usb-controller@...00000 {
>> +			compatible = "socionext,uniphier-nx1-dwc3-glue",
> 
> Undocumented compatible.
> 
>> +				     "simple-mfd";
> 
> 
> Missing reg. Did you run `make dtbs_check`? Did you actually check it
> with dtc?

I missed it and will add it.

>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0 0x15b00000 0x400>;
>> +
>> +			usb_rst: reset@0 {
> 
> reset-controller
> 
>> +				compatible = "socionext,uniphier-nx1-usb3-reset";
>> +				reg = <0x0 0x4>;
>> +				#reset-cells = <1>;
>> +				clock-names = "link";
>> +				clocks = <&sys_clk 12>;
>> +				reset-names = "link";
>> +				resets = <&sys_rst 12>;
>> +			};
>> +
>> +			usb_vbus0: regulator@100 {
>> +				compatible = "socionext,uniphier-nx1-usb3-regulator";
>> +				reg = <0x100 0x10>;
>> +				clock-names = "link";
>> +				clocks = <&sys_clk 12>;
>> +				reset-names = "link";
>> +				resets = <&sys_rst 12>;
>> +			};
>> +
>> +			usb_vbus1: regulator@110 {
>> +				compatible = "socionext,uniphier-nx1-usb3-regulator";
>> +				reg = <0x110 0x10>;
>> +				clock-names = "link";
>> +				clocks = <&sys_clk 12>;
>> +				reset-names = "link";
>> +				resets = <&sys_rst 12>;
>> +			};
>> +
>> +			usb_hsphy0: hs-phy@200 {
> 
> Node name: phy
> 
> The same in all other places. It can be also usb-phy and few others
> mentioned iin DT spec:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

I see. I'll replace the node name with "phy".

>> +				compatible = "socionext,uniphier-nx1-usb3-hsphy";
>> +				reg = <0x200 0x10>;
>> +				#phy-cells = <0>;
>> +				clock-names = "link", "phy", "phy-ext";
>> +				clocks = <&sys_clk 12>, <&sys_clk 16>,
>> +					 <&sys_clk 13>;
>> +				reset-names = "link", "phy";
>> +				resets = <&sys_rst 12>, <&sys_rst 16>;
>> +				vbus-supply = <&usb_vbus0>;
>> +			};
>> +
>> +			usb_hsphy1: hs-phy@210 {
>> +				compatible = "socionext,uniphier-nx1-usb3-hsphy";
>> +				reg = <0x210 0x10>;
>> +				#phy-cells = <0>;
>> +				clock-names = "link", "phy", "phy-ext";
>> +				clocks = <&sys_clk 12>, <&sys_clk 16>,
>> +					 <&sys_clk 13>;
>> +				reset-names = "link", "phy";
>> +				resets = <&sys_rst 12>, <&sys_rst 16>;
>> +				vbus-supply = <&usb_vbus1>;
>> +			};
>> +
>> +			usb_ssphy0: ss-phy@300 {
>> +				compatible = "socionext,uniphier-nx1-usb3-ssphy";
>> +				reg = <0x300 0x10>;
>> +				#phy-cells = <0>;
>> +				clock-names = "link", "phy", "phy-ext";
>> +				clocks = <&sys_clk 12>, <&sys_clk 17>,
>> +					 <&sys_clk 13>;
>> +				reset-names = "link", "phy";
>> +				resets = <&sys_rst 12>, <&sys_rst 17>;
>> +				vbus-supply = <&usb_vbus0>;
>> +			};
>> +
>> +			usb_ssphy1: ss-phy@310 {
>> +				compatible = "socionext,uniphier-nx1-usb3-ssphy";
>> +				reg = <0x310 0x10>;
>> +				#phy-cells = <0>;
>> +				clock-names = "link", "phy", "phy-ext";
>> +				clocks = <&sys_clk 12>, <&sys_clk 18>,
>> +					 <&sys_clk 13>;
>> +				reset-names = "link", "phy";
>> +				resets = <&sys_rst 12>, <&sys_rst 18>;
>> +				vbus-supply = <&usb_vbus1>;
>> +			};
>> +		};
>> +
>> +		pcie: pcie@...00000 {
>> +			compatible = "socionext,uniphier-pcie";
>> +			status = "disabled";
>> +			reg-names = "dbi", "link", "config", "atu";
>> +			reg = <0x16000000 0x1000>, <0x179a0000 0x10000>,
>> +			      <0x0fff0000 0x10000>, <0x16300000 0x1000>;
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +			clocks = <&sys_clk 24>;
>> +			resets = <&sys_rst 24>;
>> +			num-lanes = <2>;
>> +			num-viewport = <1>;
>> +			bus-range = <0x0 0xff>;
>> +			device_type = "pci";
>> +			ranges =
>> +			/* downstream I/O */
>> +				<0x81000000 0 0x00000000 0x0ffe0000 0 0x00010000>,
>> +			/* non-prefetchable memory */
>> +				<0x82000000 0 0x20000000 0x04200000 0 0x0bde0000>;
>> +			#interrupt-cells = <1>;
>> +			interrupt-names = "dma", "msi";
>> +			interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-map-mask = <0 0 0 7>;
>> +			interrupt-map = <0 0 0 1 &pcie_intc 0>,	/* INTA */
>> +					<0 0 0 2 &pcie_intc 1>,	/* INTB */
>> +					<0 0 0 3 &pcie_intc 2>,	/* INTC */
>> +					<0 0 0 4 &pcie_intc 3>;	/* INTD */
>> +			phy-names = "pcie-phy";
>> +			phys = <&pcie_phy>;
>> +
>> +			pcie_intc: legacy-interrupt-controller {
>> +				interrupt-controller;
>> +				#interrupt-cells = <1>;
>> +				interrupt-parent = <&gic>;
>> +				interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
>> +			};
>> +		};
>> +
>> +		pcie_ep: pcie-ep@...00000 {
>> +			compatible = "socionext,uniphier-nx1-pcie-ep";
>> +			status = "disabled";
>> +			reg-names = "dbi", "dbi2", "link", "addr_space", "atu";
>> +			reg = <0x16000000 0x1000>, <0x16100000 0x1000>,
>> +			      <0x179a0000 0x10000>, <0x04200000 0xbe00000>,
>> +			      <0x16300000 0x1000>;
>> +			clock-names = "link";
>> +			clocks = <&sys_clk 24>;
>> +			reset-names = "link";
>> +			resets = <&sys_rst 24>;
>> +			num-ib-windows = <16>;
>> +			num-ob-windows = <16>;
>> +			num-lanes = <2>;
>> +			phy-names = "pcie-phy";
>> +			phys = <&pcie_phy>;
>> +		};
>> +
>> +		pcie_phy: phy@...b8000 {
>> +			compatible = "socionext,uniphier-nx1-pcie-phy";
>> +			reg = <0x179b8000 0x4000>;
>> +			#phy-cells = <0>;
>> +			clock-names = "link";
>> +			clocks = <&sys_clk 24>;
>> +			reset-names = "link";
>> +			resets = <&sys_rst 24>;
>> +			socionext,syscon = <&sysctrl>;
>> +		};
>> +
>> +		i2c0: i2c@...80000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18780000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c0>;
>> +			clocks = <&peri_clk 4>;
>> +			resets = <&peri_rst 4>;
>> +			clock-frequency = <400000>;
>> +		};
>> +
>> +		i2c1: i2c@...81000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18781000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c1>;
>> +			clocks = <&peri_clk 5>;
>> +			resets = <&peri_rst 5>;
>> +			clock-frequency = <400000>;
>> +		};
>> +
>> +		i2c2: i2c@...82000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18782000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c2>;
>> +			clocks = <&peri_clk 6>;
>> +			resets = <&peri_rst 6>;
>> +			clock-frequency = <400000>;
>> +		};
>> +
>> +		i2c3: i2c@...83000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18783000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c3>;
>> +			clocks = <&peri_clk 7>;
>> +			resets = <&peri_rst 7>;
>> +			clock-frequency = <400000>;
>> +		};
>> +
>> +		i2c4: i2c@...84000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18784000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c4>;
>> +			clocks = <&peri_clk 8>;
>> +			resets = <&peri_rst 8>;
>> +			clock-frequency = <100000>;
>> +		};
>> +
>> +		i2c5: i2c@...85000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18785000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c5>;
>> +			clocks = <&peri_clk 9>;
>> +			resets = <&peri_rst 9>;
>> +			clock-frequency = <100000>;
>> +		};
>> +
>> +		i2c6: i2c@...86000 {
>> +			compatible = "socionext,uniphier-fi2c";
>> +			status = "disabled";
>> +			reg = <0x18786000 0x80>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_i2c6>;
>> +			clocks = <&peri_clk 10>;
>> +			resets = <&peri_rst 10>;
>> +			clock-frequency = <100000>;
>> +		};
>> +
>> +		sdctrl@...10000 {
> 
> Same comment as before...
> 
>> +			compatible = "socionext,uniphier-nx1-sdctrl",
> 
> Same problem as before...
> 
>> +				     "simple-mfd", "syscon";
>> +			reg = <0x19810000 0x400>;
>> +
>> +			sd_clk: clock {
>> +				compatible = "socionext,uniphier-nx1-sd-clock";
>> +				#clock-cells = <1>;
>> +			};
>> +
>> +			sd_rst: reset {
>> +				compatible = "socionext,uniphier-nx1-sd-reset";
>> +				#reset-cells = <1>;
>> +			};
>> +		};
>> +
>> +		perictrl@...20000 {
>> +			compatible = "socionext,uniphier-nx1-perictrl",
> 
> Also undocumented.
> 
>> +				     "simple-mfd", "syscon";
>> +			reg = <0x19820000 0x200>;
>> +
>> +			peri_clk: clock {
> 
> clock-controller
> 
>> +				compatible = "socionext,uniphier-nx1-peri-clock";
>> +				#clock-cells = <1>;
>> +			};
>> +
>> +			peri_rst: reset {
> 
> reset-controller
> 
>> +				compatible = "socionext,uniphier-nx1-peri-reset";
>> +				#reset-cells = <1>;
>> +			};
>> +		};
>> +
>> +		emmc: mmc@...00000 {
>> +			compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
>> +			reg = <0x1a000000 0x400>;
>> +			interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_emmc>;
>> +			clocks = <&sys_clk 4>;
>> +			resets = <&sys_rst 4>;
>> +			bus-width = <8>;
>> +			mmc-ddr-1_8v;
>> +			mmc-hs200-1_8v;
>> +			mmc-pwrseq = <&emmc_pwrseq>;
>> +			cdns,phy-input-delay-legacy = <9>;
>> +			cdns,phy-input-delay-mmc-highspeed = <2>;
>> +			cdns,phy-input-delay-mmc-ddr = <3>;
>> +			cdns,phy-dll-delay-sdclk = <21>;
>> +			cdns,phy-dll-delay-sdclk-hsmmc = <21>;
>> +		};
>> +
>> +		sd: mmc@...00000 {
>> +			compatible = "socionext,uniphier-sd-v3.1.1";
>> +			status = "disabled";
>> +			reg = <0x1a400000 0x800>;
>> +			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_sd>;
>> +			clocks = <&sd_clk 0>;
>> +			reset-names = "host";
>> +			resets = <&sd_rst 0>;
>> +			bus-width = <4>;
>> +			cap-sd-highspeed;
>> +		};
>> +
>> +		soc_glue: soc-glue@...00000 {
>> +			compatible = "socionext,uniphier-nx1-soc-glue",
> 
> I am not going to check...
> 
>> +				     "simple-mfd", "syscon";
>> +			reg = <0x1f800000 0x2000>;
>> +
>> +			pinctrl: pinctrl {
>> +				compatible = "socionext,uniphier-nx1-pinctrl";
>> +			};
>> +		};
>> +
>> +		soc-glue@...00000 {
>> +			compatible = "socionext,uniphier-nx1-soc-glue-debug",
> 
> Hm...
> 
>> +				     "simple-mfd";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			ranges = <0 0x1f900000 0x2000>;
>> +		};
>> +
>> +		xdmac: dma-controller@...10000 {
>> +			compatible = "socionext,uniphier-xdmac";
>> +			reg = <0x1fa10000 0x5300>;
>> +			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
>> +			dma-channels = <16>;
>> +			#dma-cells = <2>;
>> +		};
>> +
>> +		aidet: aidet@...20000 {
> 
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

I forgot to rename it and will rename to "interrupt-controller".

> 
>> +			compatible = "socionext,uniphier-nx1-aidet";
>> +			reg = <0x1fb20000 0x200>;
>> +			interrupt-controller;
>> +			#interrupt-cells = <2>;
>> +		};

Thank you,

---
Best Regards
Kunihiko Hayashi

Powered by blists - more mailing lists