On Mon, Jul 01, 2024 at 08:01:46AM +0200, Krzysztof Kozlowski wrote: > On 29/06/2024 13:34, Zoltan HERPAI wrote: > > +++ b/arch/arm/boot/dts/marvell/kirkwood-dns320l.dts > > @@ -0,0 +1,197 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Device Tree file for D-Link DNS-320L > > + * > > + * Copyright (C) 2024, Zoltan HERPAI <wigyori@xxxxxxx> > > + * Copyright (C) 2015, Sunke Schluters <sunke-dev@xxxxxxxxxxxxx> > > + * > > + * This file is based on the works of: > > + * - Sunke Schluters <sunke-dev@xxxxxxxxxxxxx> > > + * - https://github.com/scus1/dns320l/blob/master/kernel/dts/kirkwood-dns320l.dts > > + * - Andreas Bohler <dev@xxxxxxxxxxx>: > > + * - http://www.aboehler.at/doku/doku.php/projects:dns320l > > + * - http://www.aboehler.at/hg/linux-dns320l/file/ba7a60ad7687/linux-3.12/kirkwood-dns320l.dts > > + */ > > + > > +/dts-v1/; > > + > > +#include "kirkwood.dtsi" > > +#include "kirkwood-6281.dtsi" > > + > > +/ { > > + model = "D-Link DNS-320L"; > > + compatible = "dlink,dns320l", "marvell,kirkwood-88f6702", "marvell,kirkwood"; > > + > > + memory { > > + device_type = "memory"; > > + reg = <0x00000000 0x10000000>; > > + }; > > + > > + chosen { > > + bootargs = "console=ttyS0,115200n8 earlyprintk"; > > + stdout-path = &uart0; > > + }; > > + > > + gpio-keys { > > + compatible = "gpio-keys"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > Nope, these cannot be there. Depends. The kernel, which is what really matters, is happy with them there. Have a look at all the other kirkwood dts files. > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check W=1` (see > Documentation/devicetree/bindings/writing-schema.rst or > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > for instructions). I suspect that is not going to be easy to interpret. kirkwood is very old, much older than the YAML descriptions. DT descriptions of this age were considered correct if the kernel understood them, and the kernel is much more flexible than the YAML bindings. As a result, there are going to be a huge number of warnings, and it will take a lot of skill to pick out real warning which can be fixed from the noise. Also, nobody really cares, because these devices have been out of production for a decade. Nobody is going to clean up the DT files. > > + pinctrl-0 = <&pmx_buttons>; > > + pinctrl-names = "default"; > > + > > + button@1 { > > + label = "Reset push button"; > > + linux,code = <KEY_RESTART>; > > + gpios = <&gpio0 28 1>; > > + }; > > + > > + button@2 { > > + label = "USB unmount button"; > > + linux,code = <KEY_EJECTCD>; > > + gpios = <&gpio0 27 1>; > > + }; > > + }; > > + > > + gpio-leds { > > + compatible = "gpio-leds"; > > + pinctrl-0 = <&pmx_leds>; > > + pinctrl-names = "default"; > > + > > + blue-usb { > > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check W=1` (see > Documentation/devicetree/bindings/writing-schema.rst or > https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > for instructions). > > + ocp@f1000000 { > > Why you are not overriding by label/phandle? Look at the old .dts files. That is the way it was done 10 years ago. This is uniform with other kirkwood .dts files. There is something to be said for being uniform with other files of the same sort. I think we need to find a balance here. I agree with some of your comments, removing the regulator container, moving gpio-keys and gpio-leds. But i think ocp@f1000000 and not using labels can stay. Andrew
