From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C9685C433F5 for ; Sun, 19 Dec 2021 00:04:01 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7EBBA82FE0; Sun, 19 Dec 2021 01:03:58 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 74CE48306F; Sun, 19 Dec 2021 01:03:56 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id CA30E82FE0 for ; Sun, 19 Dec 2021 01:03:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C5F8411B3; Sat, 18 Dec 2021 16:03:44 -0800 (PST) Received: from slackpad.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A03593F5A1; Sat, 18 Dec 2021 16:03:42 -0800 (PST) Date: Sun, 19 Dec 2021 00:03:33 +0000 From: Andre Przywara To: Tony Dinh Cc: U-Boot Mailing List , Stefan Roese , David Purdy , Tom Rini , Marek Beh_n , Pali Roh_r , Christian Hewitt , Fabio Estevam , Jagan Teki , Lokesh Vutla , Neil Armstrong , Peter Robinson , Tim Harvey Subject: Re: [PATCH 1/8] arm: kirkwood: Pogoplug-V4 : Add DTS files Message-ID: <20211219000333.395f3ad2@slackpad.fritz.box> In-Reply-To: References: <20211218042335.5865-1-mibodhi@gmail.com> <20211218042335.5865-2-mibodhi@gmail.com> <20211218163832.5eaaa996@slackpad.fritz.box> Organization: Arm Ltd. X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Sat, 18 Dec 2021 14:05:39 -0800 Tony Dinh wrote: Hi Tony, > On Sat, Dec 18, 2021 at 8:38 AM Andre Przywara wrote: > > > > On Fri, 17 Dec 2021 20:23:27 -0800 > > Tony Dinh wrote: > > > > Hi, > > > > > Add DTS files for Pogoplug V4 board > > > > is there any reason you didn't take the kirkwood-pogoplug-series-4.dts > > file from the Linux tree? That is the same device, isn't it? > > > > Tom will ask you about the upstream status anyway... > > There were some differences that I dislike about the Linux tree > version. Such as the NAND mtd0 is read-only, but we store the envs in > mtd0 (a few blocks after the u-boot image). It seems we both created > this DTS about the same time back then in early 2010's using the same > source as a base. Both versions are identical in substance, but the > node placements are different. > > I'm not familiar with the thinking here. If we take the Linux upstream > version, are we allowed to modify it? please advise. In general the rule is to only take verbatim copies from the Linux tree. The DT describes the hardware, so there should be no change needed between U-Boot and Linux. The kernel has more reviewers for DT changes, so their DTs are of better quality. So if you think the DT in the kernel tree is wrong or missing out, you should try to send a patch to Linux to get this fixed, then sync the change into the U-Boot tree. There is a way to make small changes to the DT that U-Boot sees: You create a file with the same stub name, but add "-u-boot.dtsi", and put your changes in there. Look into arch/arm/dts, there are plenty of examples. However you should have a good reason for doing that, and ideally would not need that at all. Your case is somewhat interesting: the partition isn't technically write-protected, I guess, it's probably just some Linux safe guarding measure to prevent accidental overwrites of the firmware. So read-only in there is strictly speaking not correct, so you should try to get this fixed in Linux. Ideally U-Boot would add the read-only property, after it consumed the node, so that the kernel does not stomp over it (unless you want U-Boot environment access from Linux). So in your case you should copy the existing kernel DT, and then put: "/delete-property/ read-only;" in *-u-boot.dtsi. What DT do you typically give to the kernel? Something loaded along with the kernel? Or just the U-Boot DT, using $fdtcontroladdr? Cheers, Andre > > > > > > Signed-off-by: Tony Dinh > > > --- > > > > > > arch/arm/dts/Makefile | 1 + > > > arch/arm/dts/kirkwood-pogoplug_v4.dts | 145 ++++++++++++++++++++++++++ > > > 2 files changed, 146 insertions(+) > > > create mode 100644 arch/arm/dts/kirkwood-pogoplug_v4.dts > > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > > > index 7f622fedbd..716c6c0ed2 100644 > > > --- a/arch/arm/dts/Makefile > > > +++ b/arch/arm/dts/Makefile > > > @@ -68,6 +68,7 @@ dtb-$(CONFIG_ARCH_KIRKWOOD) += \ > > > kirkwood-openrd-client.dtb \ > > > kirkwood-openrd-ultimate.dtb \ > > > kirkwood-pogo_e02.dtb \ > > > + kirkwood-pogoplug_v4.dtb \ > > > kirkwood-sheevaplug.dtb > > > > > > dtb-$(CONFIG_MACH_S900) += \ > > > diff --git a/arch/arm/dts/kirkwood-pogoplug_v4.dts b/arch/arm/dts/kirkwood-pogoplug_v4.dts > > > new file mode 100644 > > > index 0000000000..b4754f600e > > > --- /dev/null > > > +++ b/arch/arm/dts/kirkwood-pogoplug_v4.dts > > > @@ -0,0 +1,145 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (C) 2014-2021 Tony Dinh > > > + * > > > + * Device tree file for the Pogoplug V4/Mobile > > > + * > > > + */ > > > + > > > +/dts-v1/; > > > + > > > +#include "kirkwood.dtsi" > > > +#include "kirkwood-6192.dtsi" > > > + > > > +/ { > > > + model = "Pogoplug v4"; > > > + compatible = "cloudengines,pogoplug-v4", "cloudengines,pogoplug-mobile", > > > + "marvell,kirkwood-88f6192", "marvell,kirkwood"; > > > + > > > + memory { > > > + device_type = "memory"; > > > + reg = <0x00000000 0x8000000>; > > > + }; > > > + > > > + chosen { > > > + bootargs = "console=ttyS0,115200"; > > > + stdout-path = &uart0; > > > + }; > > > + > > > + ocp@f1000000 { > > > + pinctrl: pin-controller@10000 { > > > + pmx_led_green: pmx-led-green { > > > + marvell,pins = "mpp22"; > > > + marvell,function = "gpio"; > > > + }; > > > + pmx_led_red: pmx-led-red { > > > + marvell,pins = "mpp24"; > > > + marvell,function = "gpio"; > > > + }; > > > + pmx_button_eject: pmx-button-eject { > > > + marvell,pins = "mpp29"; > > > + marvell,function = "gpio"; > > > + }; > > > + }; > > > + > > > + serial@12000 { > > > + status = "ok"; > > > + }; > > > + > > > + sata@80000 { > > > + status = "okay"; > > > + nr-ports = <2>; > > > + }; > > > + > > > + mvsdio@90000 { > > > + pinctrl-0 = <&pmx_sdio>; > > > + pinctrl-names = "default"; > > > + status = "okay"; > > > + cd-gpios = <&gpio0 27 GPIO_ACTIVE_HIGH>; > > > + /* No WP GPIO */ > > > + }; > > > + }; > > > + > > > + gpio-leds { > > > + compatible = "gpio-leds"; > > > + pinctrl-0 = <&pmx_led_red &pmx_led_green>; > > > + pinctrl-names = "default"; > > > + > > > + health { > > > + label = "status:green:health"; > > > + gpios = <&gpio0 22 GPIO_ACTIVE_LOW>; > > > + default-state = "keep"; > > > + }; > > > + fault { > > > + label = "status:red:fault"; > > > + gpios = <&gpio0 24 GPIO_ACTIVE_LOW>; > > > + }; > > > + }; > > > + > > > + gpio_keys { > > > + compatible = "gpio-keys"; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + pinctrl-0 = <&pmx_button_eject>; > > > + pinctrl-names = "default"; > > > + > > > + button@1 { > > > + label = "Eject Button"; > > > + linux,code = ; > > > + gpios = <&gpio0 29 GPIO_ACTIVE_LOW>; > > > + }; > > > + }; > > > + > > > +}; > > > + > > > +&nand { > > > + status = "okay"; > > > + > > > + partition@0 { > > > + label = "u-boot"; > > > + reg = <0x0000000 0x200000>; > > > + }; > > > + > > > + partition@200000 { > > > + label = "uImage"; > > > + reg = <0x200000 0x300000>; > > > + }; > > > + > > > + partition@500000 { > > > + label = "uImage2"; > > > + reg = <0x500000 0x300000>; > > > + }; > > > + > > > + partition@800000 { > > > + label = "failsafe"; > > > + reg = <0x800000 0x800000>; > > > + }; > > > + > > > + partition@1000000 { > > > + label = "root"; > > > + reg = <0x1000000 0x7000000>; > > > + }; > > > +}; > > > + > > > +&mdio { > > > + status = "okay"; > > > + > > > + ethphy0: ethernet-phy@0 { > > > + reg = <0>; > > > + }; > > > +}; > > > + > > > +ð0 { > > > + status = "okay"; > > > + ethernet0-port@0 { > > > + phy-handle = <ðphy0>; > > > + }; > > > +}; > > > + > > > +&pciec { > > > + status = "okay"; > > > +}; > > > + > > > +&pcie0 { > > > + status = "okay"; > > > +}; > >