On 01/07/2024 15:25, Andrew Lunn wrote: > 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. They cannot be here because the binding does not allow it. They are redundant though, in a way that kernel will work. > >> 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. One can just read the binding. Is there address/size cells? That's true that you need cleaned up platform to make efficient use of the tools, but one can test one particular schema which would print just limited amount of bindings. > >>> + 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. That's ok. > Best regards, Krzysztof
