Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
From: Carlos Eduardo de Paula <me@carlosedp.com>
To: Damien Le Moal <Damien.LeMoal@wdc.com>
Cc: orionwl@x0f.org, Anup Patel <Anup.Patel@wdc.com>,
	Sean Anderson <seanga2@gmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>
Subject: Re: [PATCH 08/10] riscv: Add Kendryte K210 device tree
Date: Tue, 18 Feb 2020 11:12:14 -0300
Message-ID: <CADnnUqe3AbTStJg9LS4qupH-OnBDGjuEFbnX8EXW8MUr4kwoGQ@mail.gmail.com> (raw)
In-Reply-To: <BYAPR04MB5816ED294439828E562EB085E7140@BYAPR04MB5816.namprd04.prod.outlook.com>

On Sat, Feb 15, 2020 at 12:00 AM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>
> On 2020/02/15 11:48, Sean Anderson wrote:
> >>>> +          cpu0: cpu@0 {
> >>>> +                  device_type = "cpu";
> >>>> +                  reg = <0>;
> >>>> +                  compatible = "riscv";
> >>>> +                  riscv,isa = "rv64imafdc";
> >>>> +                  mmu-type = "none";
> >>>
> >>> This should be "sv36".
> >>
> >> If we want to run the MMU, yes. For a nommu kernel, I would rather stick
> >> with "none". Not that it really matters since the nommu kernel will not
> >> look at this entry anyway. No strong opinion either way in the end.
> >> I have not checked the specs yet, but does sv36 necessarily implies older
> >> specs 1.9 too ? If not, then we may want something else in there for this
> >> soc special case.
> >
> > Ah, this should be "sv39", sorry. Ideally we would put something like
> > the priv spec version in the isa string, or perhaps as a separate
> > property. From reading the dt docs, it seems like one should try to
> > describe the hardware as best as possible to allow for
> > foward-compatibility.
>
> OK. But I guess we can keep the "none" here until we get to work on the MMU
> support. That definitely sounds safer to me considering the specs difference.
>
> >>>> +                  d-cache-size = <0x8000>;
> >>>> +                  d-cache-block-size = <64>; /* bogus */
> >>>> +                  clocks = <&sysctl 0>;
> >>>
> >>> This is correct only by coincidence. The clock structure is
> >>>
> >>> in0 -> pll0 -> aclk -> cpu
> >>>
> >>> aclk divides by two by default, so it runs at 390 MHz, which is also
> >>> what you set pll1 to. However, if someone else (such as the bootloader)
> >>> changes the pll0 frequency then this will be completely off.
> >>
> >> Yes... The clock management needs more work as mentioned in the cover
> >> letter. All of this works for now with direct m-mode boot (no boot loader)
> >> and relies on the hardware defaults which are coded here. The sysctl driver
> >> also relies on those defaults. A more solid implementation will need the
> >> soc_early_init() code to discover and set things up correctly.
> >>
> >> As mentioned in the cover letter, this is all a base. It works, but
> >> definitely is not complete.
> >
> > At the very least, I would different identifiers for each clock. That
> > way you can ignore them now and add support later. There isn't a
> > "natural" ordering (since the clocks are in a different order in every
> > register), so I am using this arbitrary numbering scheme [1].
> >
> > [1] https://github.com/Forty-Bot/u-boot/blob/maix_v6/include/dt-bindings/clock/k210-sysctl.h
>
> Good idea. I had a look at this when I used your device tree but decided on
> not using it for simplicity since using the default HW setup led to that
> single clock 0. But this is a good point. I will use the identifiers and
> for now have all the IDs used defined to "0". As the sysctl driver is
> changed and improved, the DT can remain the same and more devices added easily.
>
> >>>> +          ranges;
> >>>> +          interrupt-parent = <&plic0>;
> >>>> +
> >>>> +          sysctl: sysctl@50440000 {
> >>>> +                  compatible = "kendryte,k210-sysctl", "syscon";
> >>>> +                  reg = <0x50440000 0x1000>;
> >>>> +                  #clock-cells = <1>;
> >>>> +          };
> >>>
> >>> Would it be possible to model this as an MFD? There are a lot of
> >>> different registers in here, many of which are unrelated to clocks. For
> >>> example, there are also reset registers, a reboot register, and DMA
> >>> handshake controls. I think modeling this as a clock controller only
> >>> does not correctly reflect the hardware, and will be awkward in the
> >>> future.
> >>
> >> Absolutely. It is far from complete. And seeing your complete device tree,
> >> there are likely a lot of peripherals for which Linux already has drivers
> >> and that could be used if the clocks/sysctl are improved. As mentioned in
> >> the cover letter, this is left as an exercise for interested people :)
> >> Note that I am indeed interested in working on this a little more. I simply
> >> lack the time to do it :)
> >
> > My next project after u-boot support was going to be Linux, so I can
> > lend a hand after I get everything merged on that end.
>
> That would be great !
>
> >>>> +          plic0: interrupt-controller@c000000 {
> >>>> +                  #interrupt-cells = <1>;
> >>>> +                  interrupt-controller;
> >>>> +                  compatible = "kendryte,k210-plic0", "riscv,plic0";
> >>>> +                  reg = <0xC000000 0x3FFF008>;
> >>>
> >>> With regard to the size of registers, I had the following exchange on
> >>> the U-Boot mailing list.
> >>>
> >>> On Tue, Feb 4, 2020 at 10:23 PM Sean Anderson <seanga2@gmail.com> wrote:
> >>>>
> >>>> On 2/4/20 6:32 AM, Bin Meng wrote:
> >>>>> Hi Sean,
> >>>>>
> >>>>> On Mon, Feb 3, 2020 at 4:10 AM Sean Anderson <seanga2@gmail.com> wrote:
> >>>>>> Should the size of a reg be the size of the documented registers, or the size
> >>>>>> of the address space which will be routed to that device?
> >>>>>
> >>>>> Perhaps we need use the size of the address space routed to that
> >>>>> device, in case there is some undocumented registers we need handle.
> >>>>
> >>>> Ok, I'll go with the whole address space then.
> >>>
> >>> You may want to make similar changes for Linux; I didn't see any
> >>> documentation about what the preferred size was.
> >>
> >> I wondered about it too. Not really sure what to do about it.
> >
> > The sizes in my device tree are based on reading device memory and
> > seeing where it repeats. For example, the memory at 50210000 and
> > 50210100 is the same, so I set the uart1 reg to <50210000 0x100>.
>
> OK. Will make changes and retest.
>
> >
> >>>> +                  interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 0xffffffff>,
> >>>> +                                        <&cpu1_intc 11>, <&cpu1_intc 0xffffffff>;
> >>>> +                  riscv,ndev = <65>;
> >>>> +                  riscv,max-priority = <0x07>;
> >>>> +          };
> >>>> +
> >>>> +          uarths0: serial@38000000 {
> >>>> +                  compatible = "kendryte,k210-uart0", "sifive,uart0";
> >>>
> >>> I would change the first compatible string to "kendryte,k210-uarths",
> >>> since that is how this uart is described in their documentation.
> >>
> >> OK. It makes sense.
> >>
> >>>
> >>>> +                  reg = <0x38000000 0x20>;
> >>>
> >>> Same thing as the size comments above.
> >>>
> >>>> +                  interrupts = <33>;
> >>>> +                  clocks = <&sysctl 0>;
> >>>
> >>> Same clock comments.
> >>>
> >>>> +          };
> >>>> +  };
> >>>> +};
> >>>>
> >>>
> >>> --Sean
> >>>
> >>
> >>
> >
> >
>
>
> --
> Damien Le Moal
> Western Digital Research
>

Maybe it's a known thing but I found an MMU implementation here:
https://gist.github.com/44670/0d8c152df7c5b59d17d469aba4dda0e5

Comes from as fork of the sdk here https://github.com/44670/libk9
implementing also the LCD and other peripheral support.

Might help out adding support to it.

-- 
________________________________________
Carlos Eduardo de Paula
me@carlosedp.com
http://carlosedp.com
http://twitter.com/carlosedp
Linkedin
________________________________________


  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
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 [this message]
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=CADnnUqe3AbTStJg9LS4qupH-OnBDGjuEFbnX8EXW8MUr4kwoGQ@mail.gmail.com \
    --to=me@carlosedp.com \
    --cc=Anup.Patel@wdc.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=linux-riscv@lists.infradead.org \
    --cc=orionwl@x0f.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=seanga2@gmail.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