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

From: Sean Anderson <seanga2@gmail.com>
To: Palmer Dabbelt <palmer@dabbelt.com>,
	Damien Le Moal <Damien.LeMoal@wdc.com>
Cc: linux-riscv@lists.infradead.org
Subject: Re: [PATCH v11 03/10] riscv: Update Canaan Kendryte K210 device tree
Date: Thu, 14 Jan 2021 19:06:49 -0500	[thread overview]
Message-ID: <484787f2-558c-1eb4-7ab0-23f0ac5e6cd5@gmail.com> (raw)
In-Reply-To: <9d32abd1-ffb4-a887-a40d-fc173a371d23@gmail.com>

On 1/14/21 7:01 PM, Sean Anderson wrote:
> 
> On 1/14/21 6:32 PM, 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 far as I'm aware, it's recommended practice to add device-specific compatible
> 

Here it's because "riscv,debug" doesn't exist. This is the "debug"
device as described in the debug spec. AFAIK Linux never needs to
configure this device. It could probably be removed.

I am going to try and go through the list of nonexistant compatibles and
see if there are any other devices like this (nothing else like it in
Linux).

--Sean

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