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 [thread overview]
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
________________________________________
next prev parent reply other threads:[~2020-02-18 14:12 UTC|newest]
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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).