Re: [PATCH v16 09/16] riscv: Update Canaan Kendryte K210 device tree

On Fri, 2021-02-05 at 14:25 -0600, Rob Herring wrote:
[...]

quoted

+			otp0: nvmem@50420000 {
+				#address-cells = <1>;
+				#size-cells = <1>;
+				compatible = "canaan,k210-otp";
+				reg = <0x50420000 0x100>,
+				      <0x88000000 0x20000>;
+				reg-names = "reg", "mem";
+				clocks = <&sysclk K210_CLK_ROM>;
+				resets = <&sysrst K210_RST_ROM>;
+				read-only;
+				status = "disabled";

Your disabled nodes seem a bit excessive. A device should really only be 
disabled if it's a board level decision to use or not. I'd assume the 
OTP is always there and usable.

Please see below.

quoted

+
+				/* Bootloader */
+				firmware@00000 {

Drop leading 0s.

Is this memory mapped? If so, you are missing 'ranges' in the parent to 
make it translateable.

quoted

+					reg = <0x00000 0xC200>;
+				};
+
+				/*
+				 * config string as described in RISC-V
+				 * privileged spec 1.9
+				 */
+				config-1-9@1c000 {
+					reg = <0x1C000 0x1000>;
+				};
+
+				/*
+				 * Device tree containing only registers,
+				 * interrupts, and cpus
+				 */
+				fdt@1d000 {
+					reg = <0x1D000 0x2000>;
+				};
+
+				/* CPU/ROM credits */
+				credits@1f000 {
+					reg = <0x1F000 0x1000>;
+				};
+			};
+
+			dvp0: camera@50430000 {
+				compatible = "canaan,k210-dvp";

No documented. Seems to be several of them.

There are no Linux drivers for these undocumented nodes. That is why I did not
add any documentation. make dtbs_check does not complain about that as long as
the nodes are marked disabled. I kept these nodes to have the DTS in sync with
U-Boot which has them. Keeping them also creates documentation for the SoC
since this device tree is more detailed than the SoC specsheet...

I removed "status = disabled;" from all nodes that have a Linux driver and kept
it for all nodes that don't have one.



-- 
Damien Le Moal
Western Digital Research