[PATCH 3/3] ARM: dts: marvell: add support for D-Link DNS-320L

[Zoltan Herpai]

Hi Andrew,

On 06/29/2024 16:34, Andrew Lunn wrote:
> It has been a long time since i reviewed a kirkwood DT
> description. Also, best practices have changed, so the example you
> copied is probably doing things which today would be classed as wrong.
>
>> +// SPDX-License-Identifier: GPL-2.0-or-later
> It is typical to use a dual license now:
>
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>
> However, if this is mostly Sunke Schluters work, you probably cannot
> change the license without his agreement.
That's correct, a significant portion of the DTS was taken from Sunke, 
and there is at least one other DTS with this license, so I didn't want 
to change this.

>> +/*
>> + * Device Tree file for D-Link DNS-320L
>> + *
>> + * Copyright (C) 2024, Zoltan HERPAI<wigyori at uid0.hu>
>> + * Copyright (C) 2015, Sunke Schluters<sunke-dev at schlueters.de>
>> + *
>> + * This file is based on the works of:
>> + * - Sunke Schluters<sunke-dev at schlueters.de>
>> + *   -https://github.com/scus1/dns320l/blob/master/kernel/dts/kirkwood-dns320l.dts
>> + * - Andreas Bohler<dev at aboehler.at>:
>> + *   -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;
> I _think_ current best practice is to make the serial port speed part
> of the stdout-path
>
> 	stdout-path = &uart0:115200n8;
>
> Also, earlyprintk is a debug flag, it should not be needed for a
> production DT file.
Indeed, I'll update these.

>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +		pinctrl-0 = <&pmx_buttons>;
>> +		pinctrl-names = "default";
>> +
>> +		button at 1 {
>> +			label = "Reset push button";
>> +			linux,code = <KEY_RESTART>;
>> +			gpios = <&gpio0 28 1>;
> Please use GPIO_ACTIVE_LOW rather than 1. Also for other places gpios
> are used.
Ack, will send a v2 with these updates - thanks for reviewing.

Regards,
Zoltan Herpai