lkml.org 
[lkml]   [2018]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 16/19] csky: Device tree
    Hi Arnd,

    On Mon, Mar 19, 2018 at 11:28:03PM +0800, Arnd Bergmann wrote:
    > Please add a changelog text to each patch, and send patches that add
    > .dts files or
    > binding documents to the devicetree mailing list.
    Ok

    > It is usually better for an SoC based board to split the SoC specific into a
    > separate .dtsi file that gets included by the board .dts file.
    Ok

    > > +/ {
    > > + model = "Nationalchip gx6605s ck610";
    > > + compatible = "nationalchip,gx6605s,ck610";
    >
    > Is ck610 the name of the CPU core? The general convention is to have the
    > top-level "compatible" property list first the name of the board, then the
    > name of the soc, but not the name of the CPU core.
    Yes, ck610 is a cpu core of abiv1. and I will change it like this:
    model = "Zhuxian-sword gx6605s Development Board";
    compatible = "zhuxian-sword,gx6605s";
    ref: https://c-sky.github.io/docs/gx6605s.html

    > Here you should list the specific type of CPU in the compatible property and
    > document the binding for that string in the Documentations/devicetree/bindings
    > hierarchy. Without a binding, the 'ccr' and 'hint' properties make no sense.
    Sorry, I forget remove the cpu region. It's no use now.

    > If there is any chance that you could have SMP systems in the future, it
    > would be better to start with #address-cells=<1>, with appropriate reg
    > properties.
    Ok

    > Each node with a register property also needs the address in the
    > node name, e.g. "interrupt-controller@500000"
    Ok

    > Try building the dtb file with 'make W=1' to get warnings about
    > when you got that wrong.
    Thx

    > > + timer0 {
    > > + compatible = "nationalchip,timer-v1";
    >
    > This should be "timer@400"
    Ok

    > Also, each device node should have a binding documentation
    > to explain the binding associated with that "compatible"
    > string.
    Ok, I will add them.

    > > + ohci0: ohci-hcd0 {
    > The names here should be "usb@...", not "ehci-hcd"
    Ok

    > > + chosen {
    > > + bootargs = "console=ttyS0,115200 rdinit=/sbin/init root=/dev/ram0";
    > > + };
    >
    > The bootargs should not be in the dts file normally, they should come from the
    > boot loader.
    I want to keep bootargs in dts, because the bootloader only pass the dtb to kernel.

    > For the console, use the "stdout-path" property.
    Ok

    Best Regards
    Guo Ren

    \
     
     \ /
      Last update: 2018-03-20 14:56    [W:2.559 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site