On Mon, Jul 17, 2017 at 9:54 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Sat, Jul 15, 2017 at 7:05 PM, Linus Walleij <linus.wall...@linaro.org> > wrote: >> This adds a device tree file for the Gemini-based D-Link DIR-685 >> router, supporting all devices that are currently supported in >> the main DTSI SoC file. >> >> Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
>> + chosen { >> + bootargs = "console=ttyS0,19200n8"; > > I think you can drop bootargs, as stdout-path is present. > >> + stdout-path = &uart0; > > stdout-path = "uart0:115200n8"; OK this works fine! Thanks. >> + button@8 { > > button-esc { ... } ? >> + button@13 { > > button-eject { ... } ? OK fixed this. >> + led@7 { > > led-wps? > >> + led@11 { > > led-blue? (does it have a label on the box? HD1?) OK fixed this. >> + label = "dir685:blue:HD"; > > Looks like a legacy platform device name, not a DT label. That is coincidental. I was told in the past (by other reviewers) to name LEDs as "platform:color:function". c.f. armada-370-dlink-dns327l.dts: gpio-leds { sata-l-amber-pin { label = "dns327l:amber:sata-l"; (...) >> + gpio-i2c { >> + compatible = "i2c-gpio"; >> + gpios = <&gpio0 5 0>, /* SDA */ >> + <&gpio0 6 0>; /* SCL */ > > The i2c-gpio DT bindings really should be amended to support (optional) > gpio-names. It's not needed I think, I can name the lines with gpio-line-names and the labels added by the subsystem in Linux looks really nice in lsgpio: GPIO chip: gpiochip0, "FTGPIO010", 32 GPIO lines line 0: unnamed unused line 1: unnamed unused line 2: unnamed unused line 3: unnamed unused line 4: unnamed unused line 5: unnamed "sda" [kernel] line 6: unnamed "scl" [kernel] line 7: unnamed "dir685:blue:WPS" [kernel output active-low] line 8: unnamed "reset" [kernel active-low] Cool eh? :) >> + /* >> + * This "RedBoot" is the Storlink derivative. >> + */ >> + partition@0 { > > Shouldn't partitions be in a subnode named "partitions"? Hm yeah that is the new style I guess. That requires a separate patch to patch all DT[I|S] files though. >> + sata: sata@46000000 { > > "&sata {", and move outside hierarchy. > > Add "pci" label to gemini.dtsi, "&pci {", and move outside hiearchy. > > Add "ata" label to gemini.dtsi, "&pci {", and move outside hiearchy. I am under the impression that whether to use the &node style or overlay style (use the same node names) is a matter of taste. All other DTS files for this platform use this style, so I prefer to keep to it. Older DTSes and qcom DTs do this too... but others such as Marvell use this &node style. If some DT maintainers step out and say they want all to be done this way for everyone I guess I can make a patch changing them all and the base DTSI as well, but on top of this patch. Yours, Linus Walleij _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel