Re: [PATCH v11 03/10] riscv: Update Canaan Kendryte K210 device tree

From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: "linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"seanga2@gmail.com" <seanga2@gmail.com>
Subject: Re: [PATCH v11 03/10] riscv: Update Canaan Kendryte K210 device tree
Date: Fri, 15 Jan 2021 00:33:15 +0000	[thread overview]
Message-ID: <BL0PR04MB651404813AAFE779EB2972A6E7A70@BL0PR04MB6514.namprd04.prod.outlook.com> (raw)
In-Reply-To: mhng-b30be63a-5293-4d18-85cb-fb2c62b3188e@palmerdabbelt-glaptop

On 2021/01/15 8:32, Palmer Dabbelt wrote:
> On Mon, 11 Jan 2021 16:58:41 PST (-0800), Damien Le Moal wrote:
>> Update the Canaan Kendryte K210 base device tree k210.dtsi to define
>> all peripherals of the SoC, their clocks and reset lines. The device
>> tree file k210.dts is renamed to k210_generic.dts and becomes the
>> default value selection of the SOC_CANAAN_K210_DTB_BUILTIN_SOURCE
>> configuration option. No device beside the serial console is defined by
>> this device tree. This makes this generic device tree suitable for use
>> with a builtin initramfs with all known K210 based boards.
>>
>> These changes result in the K210_CLK_ACLK clock ID to be unused and
>> removed from the dt-bindings k210-clk.h header file.
>>
>> Most updates to the k210.dtsi file come from Sean Anderson's work on
>> U-Boot support for the K210.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> Reviewed-by: Anup Patel <anup@brainfault.org>
>> ---
>>  arch/riscv/Kconfig.socs                     |   2 +-
>>  arch/riscv/boot/dts/canaan/k210.dts         |  23 -
>>  arch/riscv/boot/dts/canaan/k210.dtsi        | 551 +++++++++++++++++++-
>>  arch/riscv/boot/dts/canaan/k210_generic.dts |  46 ++
>>  include/dt-bindings/clock/k210-clk.h        |   1 -
>>  5 files changed, 573 insertions(+), 50 deletions(-)
>>  delete mode 100644 arch/riscv/boot/dts/canaan/k210.dts
>>  create mode 100644 arch/riscv/boot/dts/canaan/k210_generic.dts
> 
> [Snipping this to the relevant bits, in case you missed it before.]
> 
>> @@ -81,40 +107,515 @@ in0: oscillator {
>>  	soc {
>>  		#address-cells = <1>;
>>  		#size-cells = <1>;
>> -		compatible = "kendryte,k210-soc", "simple-bus";
>> +		compatible = "canaan,k210-soc", "simple-bus";
>>  		ranges;
>>  		interrupt-parent = <&plic0>;
>>
>> -		sysctl: sysctl@50440000 {
>> -			compatible = "kendryte,k210-sysctl", "simple-mfd";
>> -			reg = <0x50440000 0x1000>;
>> -			#clock-cells = <1>;
>> +		debug0: debug@0 {
>> +			compatible = "canaan,k210-debug", "riscv,debug";
> 
> I'm still getting lots of warnings about undocumented DT compatible strings
> from checpatch.  Some of them might be in flight, but I don't see many of them
> (including both of these debug ones) having been defined anywhere.  We went
> through a whole process to sort out the SiFive DT naming conventions, I don't
> want to just circumvent that for the Canaan stuff by merging it as-is.
> 

As mentioned in my previous reply about this, I am aware of the warnings and I
am ignoring them. The reason is that I really would like to keep the DTs as-is,
describing all pieces of the SoC, even the nodes that do not have Linux support
yet. Doing so, we end up with what amounts to the best public documentation ever
 for this SoC and the boards as what is available publicly from the SoC and
boards vendors is not great (that is an understatement). All this information
was extracted from the Kendryte SDK and from board design CAD prints (available
from https://dl.sipeed.com/MAIX). Sean did most of that for the MAIX Bit board
and I added other boards and some corrections. I really do not wish anybody to
have to do this again to figure out the memory mapping, clocks, reset lines, and
pinctrl mapping. That was hard and painful.

Having these unsupported DT nodes around will also allow us to point to them for
people who want to start getting involved with risc-v & Linux and want a project
to start with. I see these boards as cheap hardware for hobbyist and educational
tools rather than production hardware. I am having a blast building a biped
walking robot with a MAIX Bit as the controller right now...

So unless you keep insisting, I would like to keep the DTs as they are. The
naming convention of all nodes in the DT is fine and follows the
"vendor,soc-function" pattern. All unsupported nodes are also marked with
"status = disabled;" so they do not cause any problem.

Thoughts ?

-- 
Damien Le Moal
Western Digital Research

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv