Messages in this thread | | | Date | Tue, 20 Mar 2018 21:55:25 +0800 | From | Guo Ren <> | Subject | Re: [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
| |