From: Robert Marko <robert.marko@sartura.hr>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: "Rob Herring" <robh+dt@kernel.org>,
krzysztof.kozlowski+dt@linaro.org, "Andrew Lunn" <andrew@lunn.ch>,
gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com,
shawnguo@kernel.org, "Linus Walleij" <linus.walleij@linaro.org>,
kostap@marvell.com, devicetree <devicetree@vger.kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Linux ARM" <linux-arm-kernel@lists.infradead.org>,
"Pali Rohár" <pali@kernel.org>,
"Marek Behún" <marek.behun@nic.cz>
Subject: Re: [PATCH 2/2] arm64: dts: marvell: add support for Methode eDPU
Date: Tue, 10 May 2022 14:43:38 +0200 [thread overview]
Message-ID: <CA+HBbNGWbGu73JtCb68QMhF6o9KrcfZH2AtOL6jUAnxrmCBcsQ@mail.gmail.com> (raw)
In-Reply-To: <fde74400-34aa-df80-5af5-cb4ee89c8e6f@linaro.org>
On Tue, May 10, 2022 at 1:46 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 10/05/2022 13:41, Robert Marko wrote:
> > On Tue, May 10, 2022 at 12:20 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 09/05/2022 13:00, Robert Marko wrote:
> >>> Methode eDPU is an Armada 3720 powered board based on the Methode uDPU.
> >>>
> >>> They feature the same CPU, RAM, and storage as well as the form factor.
> >>>
> >>> However, eDPU only has one SFP slot plus a copper G.hn port.
> >>>
> >>> In order to reduce duplication, split the uDPU DTS into a common one.
> >>>
> >>> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> >>> ---
> >>> arch/arm64/boot/dts/marvell/Makefile | 1 +
> >>> .../boot/dts/marvell/armada-3720-eDPU.dts | 14 ++
> >>> .../boot/dts/marvell/armada-3720-uDPU.dts | 148 +---------------
> >>> .../boot/dts/marvell/armada-3720-uDPU.dtsi | 163 ++++++++++++++++++
> >>> 4 files changed, 179 insertions(+), 147 deletions(-)
> >>> create mode 100644 arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
> >>> create mode 100644 arch/arm64/boot/dts/marvell/armada-3720-uDPU.dtsi
> >>>
> >>> diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
> >>> index 1c794cdcb8e6..104d7d7e8215 100644
> >>> --- a/arch/arm64/boot/dts/marvell/Makefile
> >>> +++ b/arch/arm64/boot/dts/marvell/Makefile
> >>> @@ -1,6 +1,7 @@
> >>> # SPDX-License-Identifier: GPL-2.0
> >>> # Mvebu SoC Family
> >>> dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-db.dtb
> >>> +dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-eDPU.dtb
> >>> dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin.dtb
> >>> dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-emmc.dtb
> >>> dtb-$(CONFIG_ARCH_MVEBU) += armada-3720-espressobin-ultra.dtb
> >>> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
> >>> new file mode 100644
> >>> index 000000000000..6b573a6854cc
> >>> --- /dev/null
> >>> +++ b/arch/arm64/boot/dts/marvell/armada-3720-eDPU.dts
> >>> @@ -0,0 +1,14 @@
> >>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >>> +
> >>> +/dts-v1/;
> >>> +
> >>> +#include "armada-3720-uDPU.dtsi"
> >>> +
> >>> +/ {
> >>> + model = "Methode eDPU Board";
> >>> + compatible = "methode,edpu", "marvell,armada3720";
> >>
> >> You need also bindings for the board compatible. Someone should convert
> >> the Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt to YAML.
> >
> > Ok, I can convert the SoC compatibles at least for now.
> > Any advice you can give me on how the handle the Espressobin boards
> > having multiple board-specific compatibles?
> > For example, Espressobin V7 has:
> > "globalscale,espressobin-v7", "globalscale,espressobin"
> >
>
> Documentation/devicetree/bindings/arm/fsl.yaml
Thanks, now it makes sense.
>
> >>
> >>> +};
> >>> +> + sfp_eth1: sfp-eth1 {
> >>
> >> Generic node names, please.
> >
> > Can you give me an example of what would be appropriate here because the SFP
> > bindings example utilizes the same naming scheme as used here?
>
> "sfp" if you have only one sfp node.
There are 2 SFP nodes in total, that is why they are named according
to the ethernet controller
to which they are connected.
uDPU has 2 SFP slots while eDPU has 1, so one was moved to uDPU DTS.
>
> >
> >>
> >>> + compatible = "sff,sfp";
> >>> + i2c-bus = <&i2c1>;
> >>> + los-gpio = <&gpiosb 7 GPIO_ACTIVE_HIGH>;
> >>> + mod-def0-gpio = <&gpiosb 8 GPIO_ACTIVE_LOW>;
> >>> + tx-disable-gpio = <&gpiosb 9 GPIO_ACTIVE_HIGH>;
> >>> + tx-fault-gpio = <&gpiosb 10 GPIO_ACTIVE_HIGH>;
> >>> + maximum-power-milliwatt = <3000>;
> >>> + };
> >>> +};
> >>> +
> >>> +&sdhci0 {
> >>> + status = "okay";
> >>> + bus-width = <8>;
> >>> + mmc-ddr-1_8v;
> >>> + mmc-hs400-1_8v;
> >>> + marvell,pad-type = "fixed-1-8v";
> >>> + non-removable;
> >>> + no-sd;
> >>> + no-sdio;
> >>> +};
> >>> +
> >>> +&spi0 {
> >>> + status = "okay";
> >>> + pinctrl-names = "default";
> >>> + pinctrl-0 = <&spi_quad_pins>;
> >>> +
> >>> + spi-flash@0 {
> >>
> >> Run dtbs_check and fix the errors.
> >
> > Ok, will split the DTSI and eDPU commits and fixup nodes in between.
> >>
> >>> + compatible = "jedec,spi-nor";
> >>> + reg = <0>;
> >>> + spi-max-frequency = <54000000>;
> >>> +
> >>> + partitions {
> >>> + compatible = "fixed-partitions";
> >>> + #address-cells = <1>;
> >>> + #size-cells = <1>;
> >>> + /* only bootloader is located on the SPI */
> >>> + partition@0 {
> >>> + label = "firmware";
> >>> + reg = <0x0 0x180000>;
> >>> + };
> >>> +
> >>> + partition@180000 {
> >>> + label = "u-boot-env";
> >>> + reg = <0x180000 0x10000>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&pinctrl_nb {
> >>> + i2c2_recovery_pins: i2c2-recovery-pins {
> >>> + groups = "i2c2";
> >>> + function = "gpio";
> >>> + };
> >>> +};
> >>> +
> >>> +&i2c1 {
> >>> + status = "okay";
> >>> + pinctrl-names = "default", "recovery";
> >>> + pinctrl-0 = <&i2c2_pins>;
> >>> + pinctrl-1 = <&i2c2_recovery_pins>;
> >>> + /delete-property/mrvl,i2c-fast-mode;
> >>> + scl-gpios = <&gpionb 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >>> + sda-gpios = <&gpionb 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >>> +
> >>> + nct375@48 {
> >>
> >> Generic node names, representing class of a device.
> > Ok, will rename in v2.
> >>
> >>> + status = "okay";
> >>
> >> OK status is by default, why do you need it? Also, it goes as last property.
> >
> > It's not needed, I have not changed any nodes, they are just
> > copy/paste during the DTS split.
> > Will drop it in v2.
> >
>
> Hm, but the node names were different in original DTS, so this is not a
> simple split. In such case better to correct coding styles in one patch
> (node names, status etc) and then perform the split. The split should
> create the same output DTB, which is not the case here.
Understood, I did all of the fixups now before the split to make it clear.
Regards,
Robert
>
> Best regards,
> Krzysztof
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
next prev parent reply other threads:[~2022-05-10 12:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-09 11:00 [PATCH 1/2] dt-bindings: vendor-prefixes: add Methode Electronics Robert Marko
2022-05-09 11:00 ` [PATCH 2/2] arm64: dts: marvell: add support for Methode eDPU Robert Marko
2022-05-10 10:20 ` Krzysztof Kozlowski
2022-05-10 11:41 ` Robert Marko
2022-05-10 11:46 ` Krzysztof Kozlowski
2022-05-10 12:43 ` Robert Marko [this message]
2022-05-10 12:50 ` Krzysztof Kozlowski
2022-05-10 10:20 ` [PATCH 1/2] dt-bindings: vendor-prefixes: add Methode Electronics Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+HBbNGWbGu73JtCb68QMhF6o9KrcfZH2AtOL6jUAnxrmCBcsQ@mail.gmail.com \
--to=robert.marko@sartura.hr \
--cc=andrew@lunn.ch \
--cc=devicetree@vger.kernel.org \
--cc=gregory.clement@bootlin.com \
--cc=kostap@marvell.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marek.behun@nic.cz \
--cc=pali@kernel.org \
--cc=robh+dt@kernel.org \
--cc=sebastian.hesselbarth@gmail.com \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).