Re: [PATCH v10 00/23] RISC-V Kendryte K210 support improvements

[Sean Anderson <seanga2@gmail.com>]

On 1/10/21 8:56 PM, Damien Le Moal wrote:
> On Sat, 2021-01-09 at 09:32 -0800, Palmer Dabbelt wrote:
>> On Sun, 13 Dec 2020 05:50:33 PST (-0800), Damien Le Moal wrote:
>>> This series of patches improves support for boards based on the Canaan
>>> Kendryte K210 RISC-V dual core SoC. Minimal support for this SoC is
>>> already included in the kernel. These patches complete it, enabling
>>> support for most peripherals present on the SoC as well as introducing
>>> device trees for the various K210 boards available on the market today
>>> from SiPeed and Kendryte.
>>
>> Putting everything together like this makes it overly difficult to get things
>> merged: there's some actual fixes, some new arch/riscv stuff, and a handful of
>> drivers.  I know we've been kind of mixing up the SiFive and RISC-V trees, but
>> that's largely because things have been pretty quiet and it's the same people
>> working on everything.  That'll probably change at some point, but it doesn't
>> mean we can just start mixing up everything in here -- even for the SiFive
>> stuff, we usulaly try to do it in the relevant subsystem tree.
> 
> I know that, but for some drivers (e.g. clock), there is overlap that would
> prevent compiling if not all patches go to the same tree. And for people to
> test, if not all drivers are in the same tree, nothing will work (e.g. without
> the pinctrl driver, nothing device will work, even booting will fail). That is
> why I kept sending everything together.
> 
> With what you applied, only the clock driver and the fpioa driver do not really
> belong to the riscv tree. But since you queued the dt-bindings doc patches
> which add the headers for these drivers, it may be necessary to keep them in
> the riscv tree to avoid compilation failures.
> 
> Stephen, Linus, is that OK ?
> 
>>> Pathes 1 to 4 are various fixes for riscv arch code and riscv
>>> dependent devices. Of note here is patch 3 which fix system calls
>>> execution in the no MMU case, and patch 4 which simplifies DTB builtin
>>> handling.
>>
>> The first three are on fixes, but the fourth isn't a fix: it's a fairly
>> significant change to how portable our kernels can be.  The old scheme allows
>> vendors the option of building systems with M-mode compatibility, this new one
>> doesn't.  That said, I don't think anyone is actually going to build systems
>> this way -- we really should have had some sort of mboardid, but that was shot
>> down in favor of some sort of platform thing and it's unlikely we get that far
>> over there.
>>
>> I'm not really sure I'm ready to throw in the towel on binary compatibility
>> between vendors yet, at least in general.  In this specific case it seems fine,
>> though -- accross the board we're just spending way too much time worrying
>> about the small things while we have way bigger problems to deal with.
>> Obviously it would be better if we had some scheme to handle this here, but I'd
>> much rather focus on the basics.
>>
>> I still hope we get to the point where we can have binary compatibility between
>> systems from various vendors, while still having reasonably useful systems.
>> Unfortunately we're quite far away from anything like that, so I'm fine taking
>> this sort of thing as that's as good as we can do for the forseeable future.
> 
> Yes, I agree that working on improving binary portability is very important.
> However, I am not convinced at all that trying to do so using a device-tree
> based environment is viable, or even desired. I think that true portability can
> only be achieved using something like ACPI or equivalent allowing run-time
> device discovery. But that is a discussion for another thread.
> 
>> This is on for-next.
> 
> Thanks.
> 
>>> The following patches 7 to 11 document device tree bindings for the SoC
>>> drivers. The implementation of these drivers is provided in patches 12,
>>> 13 and 14, respectively implementing the SoC clock driver, reset
>>> controller and SOC pin function control.
>>
>> The numbering is off a bit here.  The clock stuff has gone in through that tree
>> and I'm fine  taking the reset controller as that's been reviewed, but I don't
>> see any review on the pinctl driver so I haven't taken that yet.
>>
>> I'm happy to re-send that patch (likely with a more appropriate subject line,
>> as it's a pinctl driver not a riscv patch).
> 
> I rebased the series on the riscv tree fixes+for-next branches and changed the
> subject line of these 2 patches. I am testing that now and will resend soon.
> But so far so good. All is working fine.
> 
>>> Patches 15 to 20 update the existing K210 device tree and add new
>>> device tree files for several K210 based boards: MAIX Bit, MAIXDUINO,
>>> MAIX Dock and MAIX Go boards from SiPeed and the KD233 development
>>> board from Canaan.
>>
>> There are tons of checkpatch warnings in these, mostly related to compat
>> strings that don't have binding definitions.  It looks like there's just a lot
>> of stuff in there that doesn't have any support on the Linux side, my guess
>> would be that the best thing to do is to drop those until they're defined.
> 
> Yes, I am aware of these warnings. Despite that, I kept the undocumented and
> unsupported DT nodes as having the complete device-trees (soc k210.dtsi part
> and boards dts) constitute the best documentation ever for the SoC and the
> boards. Most of this work come from Sean (with some corrections from me) and
> extracted all this information from the almost non-existent documentation
> (basically only board layout doc is available) using mostly only the Kendryte
> SDK is really hard. So despite the warning, I would really prefer that we keep
> the DTs as complete as they are now. This would also allow us to point to
> specific nodes that need support for new developers that want to get involved
> with riscv (mentoring program of the foundation). These boards being extremely
> cheap are the perfect platform for students and hobbyist to get involved.
> 
> So unless you insist, I am going to resend the DTs as-is.

Just a note, everything which isn't supported has been left disabled
(usually implicitly). It could be possible to remove unsupported nodes,
but I would like to keep the Linux and U-Boot device trees in-sync.

--Sean

> 
>>
>>> Finally the last two patches updates the k210 nommu defconfig to include
>>> the newly implemented drivers and provide a new default configuration
>>> file enabling SD card support.
>>
>> I'm also going to just leave these out for now, until we sort out the above
>> issues.  Let me know if you're going to send another patch set, or
> 
> Thanks.
>