Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Anup Patel <Anup.Patel@wdc.com>, Palmer Dabbelt <palmer@dabbelt.com>
Cc: "linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	Paul Walmsley <paul.walmsley@sifive.com>
Subject: Re: [PATCH 04/10] riscv: Add BUILTIN_DTB support
Date: Thu, 5 Mar 2020 06:13:45 +0000
Message-ID: <BYAPR04MB5816280119C3A35ACF968DF2E7E20@BYAPR04MB5816.namprd04.prod.outlook.com> (raw)
In-Reply-To: <MN2PR04MB6061B382AF9F875B61B863978DE20@MN2PR04MB6061.namprd04.prod.outlook.com>

On 2020/03/05 14:37, Anup Patel wrote:
> 
> 
>> -----Original Message-----
>> From: Damien Le Moal
>> Sent: 05 March 2020 10:44
>> To: Anup Patel <Anup.Patel@wdc.com>; Palmer Dabbelt
>> <palmer@dabbelt.com>
>> Cc: linux-riscv@lists.infradead.org; Paul Walmsley
>> <paul.walmsley@sifive.com>
>> Subject: Re: [PATCH 04/10] riscv: Add BUILTIN_DTB support
>>
>> On 2020/03/05 13:58, Anup Patel wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Palmer Dabbelt <palmer@dabbelt.com>
>>>> Sent: 05 March 2020 00:59
>>>> To: Damien Le Moal <Damien.LeMoal@wdc.com>
>>>> Cc: linux-riscv@lists.infradead.org; Paul Walmsley
>>>> <paul.walmsley@sifive.com>; Anup Patel <Anup.Patel@wdc.com>
>>>> Subject: Re: [PATCH 04/10] riscv: Add BUILTIN_DTB support
>>>>
>>>> On Wed, 12 Feb 2020 02:34:26 PST (-0800), Damien Le Moal wrote:
>>>>> Enable a kernel builtin dtb for boards not capable of providing a
>>>>> device tree to the kernel.
>>>>
>>>> I'd prefer if we picked a mechanism that allows a single kernel
>>>> binary to be run on multiple systems.  I think there's two use cases here:
>>>
>>> I strongly support "single kernel binary for multiple systems" but
>>> it's for different purpose here.
>>>
>>>>
>>>> * Bootloaders that provide no DTB at all.
>>>> * Bootloaders that provied a DTB that, for some reason, isn't usable.
>>
>> Sure, but as Anup mentions below, the current use case it to not even use
>> any bootloader at all for the K210 since that brings no value at all (in my
>> opinion). More on this below.
>>
>>>>
>>>> Given those constraints, could we do something similar to the early
>> fixups?
>>>> I'm thinking we could build a mapping between a hardware identifier
>>>> and a DTB, then look up the DTB we want to use.  Users that want a
>>>> kernel that only runs on a single device can do so by configuring
>>>> only a single DTB, users that want a more portable kernel can select
>>>> a bunch -- that's essentially the same as how we're treating
>>>> everything else (for example, the
>>>> CONFIG_SOC_* stuff).
>>>
>>> There is no bootloader on Kendryte K210. The Linux RISC-V NOMMU kernel
>>> boots directly. The BUILTIN_DTB is only applicable to cases where
>>> there is no bootloader before kernel.
>>>
>>> The Linux RISC-V NOMMU will tend be used in cases where:
>>> 1. There is no bootloader and kernel boots directly hence we need
>>> builtin DTB feature.
>>> 2. There is very less RAM so we will have to build kernel specific to
>>> a particular platform with bare minimum drivers. Due to this, we will
>>> have separate defconfig for NOMMU platforms.
>>>
>>> I think point1 can be tackled if we enforce having bootloader (such as
>>> U-Boot) for NOMMU systems and drop this patch.
>>
>> But that would go against point 2 as that will use more memory... By "drop
>> this patch", may be you meant to say "not use this config option" ?
> 
> I meant to use U-Boot on Kendryte to launch kernel.
> 
>>
>>> For point2 above, we don't have much alternatives other than reducing
>>> kernel binary size by disabling unwanted drivers.
>>
>> And not using a boot loader. Sean got U-boot working with Kendryte, so it is
>> not that we cannot make it work. It is only that it may be less optimal due to
>> the memory used by the boot loader itself. Unless we can recover it if the
>> kernel relocate itself over it ? Surely doable, but it does sound to me like an
>> overkill for this particular use case, i.e. a tiny, hyper-embedded board where
>> running Linux is probably not the best choice in the first place, at least when
>> looking at real applications. The story is different for "hobbyist" level. My on-
>> going 6 DoF robotic arm project controlled with Linux on K210 is a valid use
>> case after all :)
> 
> Dropping BUILTIN DTB feature will be more like a Linux RISC-V policy thingy.
> My suggestion was more about discouraging Linux S-mode users to use the
> BUILTIN DTB feature.

Got it. For now, we could tie BUILTIN DTB config to !MMU case only. If there is
a valid use case that pops up for regular MMU/S-mode Linux, it is easy to change.

> I agree that it is difficult to fit a proper boot-flow (having proper bootloader)
> due to limited RAM. If we don't link DTB to Linux RISC-V NOMMU then some
> thing else need to provide it hence bootloader suggestion.

OK. Understood and I agree.

> 
> Apart from the Linux RISC-V NOMMU use-case, the BUILTIN DTB feature can
> be useful in environments such as FPGAs/Palladium where proper bootloader
> is not available.

OK. But we do not have to enable it for this right away, no ? So should I just
not allow BUILTIN DTB for MMU==true case for now ?

> 
> Regards,
> Anup
> 
>>
>>>
>>>>
>>>> For the hardware ID, could we do something like:
>>>>
>>>> * Check for the top-level DT compatible string, on systems where we
>> have a
>>>>   provided DTB.
>>>> * Check for a matching mimpid/marchid/mvendorid tuple, maybe with
>>>> some sort of
>>>>   masking functionality if we later need one.  These are availiable via SBI
>>>>   calls, but I'd be inclined to restrict them to M-mode boot and just say the
>>>>   SBI must provide a device tree with at least a suitable compatible string.
>>>>
>>>> While I suppose we could put together a tool for generating these
>>>> tables, for now we could probably just stick the mappings in a table
>>>> for now given that there's only one of them.
>>>>
>>>> That said, I'm not sure what to do about the different Kendryte
>>>> boards -- is there any way to poke the hardware to see which is which?
>>>
>>> I am sure there are two three different boards out there. Don't know
>>> exact differences between these boards.
>>
>> As far as I can tell, all the boards use the exact same SoC. No differences that
>> I can detect nor aware of. What differs between the different flavors of
>> boards are the perypherals attached: some have WiFi, different LCDs and
>> different cameras. The device tree is able to describe that of course, but the
>> core dtsi part for the SoC itself seem to be OK at least for the 4 different
>> boards I have (Kendryte KD233, Sipeed MAIXDUINO, MAIX Go and Dan
>> Dock).
>>
>>>
>>> Regards,
>>> Anup
>>>
>>>>
>>>>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>>>>> ---
>>>>>  arch/riscv/Kbuild            |  1 +
>>>>>  arch/riscv/Kconfig           | 18 ++++++++++++++++++
>>>>>  arch/riscv/boot/dts/Makefile |  4 ++++
>>>>>  arch/riscv/kernel/setup.c    |  6 ++++++
>>>>>  arch/riscv/mm/init.c         |  4 ++++
>>>>>  5 files changed, 33 insertions(+)
>>>>>
>>>>> diff --git a/arch/riscv/Kbuild b/arch/riscv/Kbuild index
>>>>> d1d0aa70fdf1..988804e430e4 100644
>>>>> --- a/arch/riscv/Kbuild
>>>>> +++ b/arch/riscv/Kbuild
>>>>> @@ -1,3 +1,4 @@
>>>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>>>
>>>>>  obj-y += kernel/ mm/ net/
>>>>> +obj-$(CONFIG_USE_BUILTIN_DTB)	+= boot/dts/
>>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index
>>>>> 1a3b5a5276be..28899e15f548 100644
>>>>> --- a/arch/riscv/Kconfig
>>>>> +++ b/arch/riscv/Kconfig
>>>>> @@ -355,6 +355,24 @@ config CMDLINE_FORCE
>>>>>
>>>>>  endchoice
>>>>>
>>>>> +config USE_BUILTIN_DTB
>>>>> +	bool "Use builtin DTB"
>>>>> +	help
>>>>> +	  Link a device tree blob for particular hardware into the kernel,
>>>>> +	  suppressing use of the DTB pointer provided by the bootloader.
>>>>> +	  This option should only be used with hardware or bootloaders that
>>>>> +	  are not capable of providing a DTB to the kernel, or for
>>>>> +	  experimental hardware without stable device tree bindings.
>>>>> +
>>>>> +config BUILTIN_DTB_SOURCE
>>>>> +	string "Source file for builtin DTB"
>>>>> +	default ""
>>>>> +	depends on USE_BUILTIN_DTB
>>>>> +	help
>>>>> +	  Base name (without suffix, relative to arch/riscv/boot/dts) for
>>>>> +	  the a DTS file that will be used to produce the DTB linked into
>>>>> +	  the kernel.
>>>>> +
>>>>>  endmenu
>>>>>
>>>>>  menu "Power management options"
>>>>> diff --git a/arch/riscv/boot/dts/Makefile
>>>>> b/arch/riscv/boot/dts/Makefile index dcc3ada78455..0bf2669aa12d
>>>>> 100644
>>>>> --- a/arch/riscv/boot/dts/Makefile
>>>>> +++ b/arch/riscv/boot/dts/Makefile
>>>>> @@ -1,2 +1,6 @@
>>>>>  # SPDX-License-Identifier: GPL-2.0
>>>>> +ifneq ($(CONFIG_BUILTIN_DTB_SOURCE),"")
>>>>> +obj-$(CONFIG_USE_BUILTIN_DTB) += $(patsubst
>>>>> +"%",%,$(CONFIG_BUILTIN_DTB_SOURCE)).dtb.o
>>>>> +else
>>>>>  subdir-y += sifive
>>>>> +endif
>>>>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>>>>> index 0a6d415b0a5a..3e89be9d888c 100644
>>>>> --- a/arch/riscv/kernel/setup.c
>>>>> +++ b/arch/riscv/kernel/setup.c
>>>>> @@ -68,7 +68,13 @@ void __init setup_arch(char **cmdline_p)
>>>>>
>>>>>  	setup_bootmem();
>>>>>  	paging_init();
>>>>> +
>>>>> +#if IS_ENABLED(CONFIG_USE_BUILTIN_DTB)
>>>>> +	unflatten_and_copy_device_tree();
>>>>> +#else
>>>>>  	unflatten_device_tree();
>>>>> +#endif
>>>>> +
>>>>>  	clint_init_boot_cpu();
>>>>>
>>>>>  #ifdef CONFIG_SWIOTLB
>>>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index
>>>>> 965a8cf4829c..1274e889d008 100644
>>>>> --- a/arch/riscv/mm/init.c
>>>>> +++ b/arch/riscv/mm/init.c
>>>>> @@ -480,7 +480,11 @@ static void __init setup_vm_final(void)  #else
>>>>> asmlinkage void __init setup_vm(uintptr_t dtb_pa)  {
>>>>> +#if IS_ENABLED(CONFIG_USE_BUILTIN_DTB)
>>>>> +	dtb_early_va = __dtb_start;
>>>>> +#else
>>>>>  	dtb_early_va = (void *)dtb_pa;
>>>>> +#endif
>>>>>  }
>>>>>
>>>>>  static inline void setup_vm_final(void)
>>>
>>
>>
>> --
>> Damien Le Moal
>> Western Digital Research
> 


-- 
Damien Le Moal
Western Digital Research


  reply index

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12 10:34 [PATCH 00/10] Kendryte k210 SoC boards support Damien Le Moal
2020-02-12 10:34 ` [PATCH 01/10] riscv: Fix gitignore Damien Le Moal
2020-02-20  0:15   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 02/10] riscv: Force flat memory model with no-mmu Damien Le Moal
2020-02-14 20:18   ` Sean Anderson
2020-02-15  2:15     ` Damien Le Moal
2020-02-15  2:26       ` Sean Anderson
2020-02-15  2:40         ` Damien Le Moal
2020-03-02  3:48   ` Anup Patel
2020-03-04 18:38   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 03/10] riscv: Unaligned load/store handling for M_MODE Damien Le Moal
2020-03-02  3:57   ` Anup Patel
2020-03-04 19:28   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 04/10] riscv: Add BUILTIN_DTB support Damien Le Moal
2020-03-02  3:58   ` Anup Patel
2020-03-04 19:28   ` Palmer Dabbelt
2020-03-05  4:58     ` Anup Patel
2020-03-05  5:14       ` Damien Le Moal
2020-03-05  5:37         ` Anup Patel
2020-03-05  6:13           ` Damien Le Moal [this message]
2020-03-08  6:10             ` Anup Patel
2020-03-05  8:18         ` Atish Patra
2020-03-07  0:02           ` Sean Anderson
2020-03-07  1:51             ` Atish Patra
2020-03-07  2:08               ` Sean Anderson
2020-03-06 23:56         ` Sean Anderson
2020-02-12 10:34 ` [PATCH 05/10] riscv: Add SOC early init support Damien Le Moal
2020-03-04 19:28   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 06/10] riscv: Add Kendryte K210 SoC support Damien Le Moal
2020-02-14 20:31   ` Sean Anderson
2020-03-04 19:38   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 07/10] riscv: Select required drivers for Kendryte SOC Damien Le Moal
2020-03-02  3:59   ` Anup Patel
2020-03-04 19:44   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 08/10] riscv: Add Kendryte K210 device tree Damien Le Moal
2020-02-14 20:51   ` Sean Anderson
2020-02-15  2:34     ` Damien Le Moal
2020-02-15  2:48       ` Sean Anderson
2020-02-15  3:00         ` Damien Le Moal
2020-02-18 14:12           ` Carlos Eduardo de Paula
2020-02-18 14:18             ` Sean Anderson
2020-02-18 14:30               ` Carlos Eduardo de Paula
2020-02-18 17:48                 ` Sean Anderson
2020-02-18 19:26                   ` Carlos Eduardo de Paula
2020-02-19  9:06                     ` Wladimir J. van der Laan
2020-02-19 22:28                       ` Sean Anderson
2020-02-20 10:48                         ` Wladimir J. van der Laan
2020-02-22 19:07                       ` Wladimir J. van der Laan
2020-04-01 17:55                         ` Drew Fustini
2020-04-02  2:24                           ` Damien Le Moal
2020-02-19  8:50                   ` Wladimir J. van der Laan
2020-02-27 19:43       ` Sean Anderson
2020-03-02  4:06   ` Anup Patel
2020-03-02  4:15     ` Damien Le Moal
2020-03-02  4:22       ` Anup Patel
2020-03-02  4:51         ` Damien Le Moal
2020-03-02  5:05           ` Anup Patel
2020-03-02  5:08             ` Damien Le Moal
2020-03-07  0:18               ` Sean Anderson
2020-03-07  4:11                 ` Anup Patel
2020-03-04 19:44   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 09/10] riscv: Kendryte K210 default config Damien Le Moal
2020-03-02  4:07   ` Anup Patel
2020-03-04 19:44   ` Palmer Dabbelt
2020-02-12 10:34 ` [PATCH 10/10] riscv: create a loader.bin for the kendryte kflash.py tool Damien Le Moal
2020-03-02  4:08   ` Anup Patel
2020-03-04 19:44   ` Palmer Dabbelt
2020-02-14 15:05 ` [PATCH 00/10] Kendryte k210 SoC boards support Carlos Eduardo de Paula
2020-02-15  2:02   ` Damien Le Moal
2020-02-17 13:28     ` Carlos Eduardo de Paula
2020-02-26 21:31       ` Carlos Eduardo de Paula
2020-02-27  2:18         ` Damien Le Moal
2020-02-28 20:32 ` Sean Anderson
2020-03-02  3:01   ` Damien Le Moal
2020-03-02  3:53     ` Sean Anderson
2020-03-02  4:11       ` Damien Le Moal
2020-03-02  4:18         ` Sean Anderson
2020-03-02  4:54           ` Damien Le Moal
2020-03-02  4:56             ` Sean Anderson
2020-03-02  5:03               ` Damien Le Moal
2020-03-02  4:17       ` Anup Patel
2020-03-02  4:21         ` Sean Anderson
2020-03-02  4:48         ` Damien Le Moal
2020-03-02  4:51           ` Damien Le Moal
2020-03-02  5:02           ` Sean Anderson
2020-03-02  5:11             ` Damien Le Moal
2020-03-02  5:25               ` Sean Anderson
2020-03-02  5:43                 ` Damien Le Moal
2020-03-04 19:44 ` Palmer Dabbelt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR04MB5816280119C3A35ACF968DF2E7E20@BYAPR04MB5816.namprd04.prod.outlook.com \
    --to=damien.lemoal@wdc.com \
    --cc=Anup.Patel@wdc.com \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git