On 30/05/2022 15:08, Paweł Anikiel wrote:
> The Mercury+ AA1 is not a standalone board, rather it's a module
> with an Arria 10 SoC and some peripherals on it. Remove everything that
> is not strictly related to the module.
Subject has several issues:
1. Use prefix matching subsystem (git log --oneline)
2. You are not changing here anything to header. Header files have
different format and end with .h. This is a DTSI file.
>
> Signed-off-by: Paweł Anikiel <pan@xxxxxxxxxxxx>
Thank you for your patch. There is something to discuss/improve.
> ---
> arch/arm/boot/dts/Makefile | 1 -
> ...1.dts => socfpga_arria10_mercury_aa1.dtsi} | 68 ++++---------------
> 2 files changed, 14 insertions(+), 55 deletions(-)
> rename arch/arm/boot/dts/{socfpga_arria10_mercury_aa1.dts => socfpga_arria10_mercury_aa1.dtsi} (58%)
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index edfbedaa6168..023c8b4ba45c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1146,7 +1146,6 @@ dtb-$(CONFIG_ARCH_S5PV210) += \
> s5pv210-torbreck.dtb
> dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += \
> socfpga_arria5_socdk.dtb \
> - socfpga_arria10_mercury_aa1.dtb \
> socfpga_arria10_socdk_nand.dtb \
> socfpga_arria10_socdk_qspi.dtb \
> socfpga_arria10_socdk_sdmmc.dtb \
> diff --git a/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts b/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dtsi
> similarity index 58%
> rename from arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts
> rename to arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dtsi
> index a75c059b6727..fee1fc39bb2b 100644
> --- a/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts
> +++ b/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dtsi
> @@ -1,57 +1,38 @@
> // SPDX-License-Identifier: GPL-2.0
> -/dts-v1/;
> -
> +/*
> + * Copyright 2022 Google LLC
> + */
How is this related to the patch?
> #include "socfpga_arria10.dtsi"
>
> / {
> -
> - model = "Enclustra Mercury AA1";
> - compatible = "enclustra,mercury-aa1", "altr,socfpga-arria10", "altr,socfpga";
> -
> aliases {
> ethernet0 = &gmac0;
> serial1 = &uart1;
> - i2c0 = &i2c0;
> - i2c1 = &i2c1;
> - };
> -
> - memory@0 {
> - name = "memory";
> - device_type = "memory";
> - reg = <0x0 0x80000000>; /* 2GB */
> };
>
> chosen {
> stdout-path = "serial1:115200n8";
> };
> -};
>
> -&eccmgr {
> - sdmmca-ecc@ff8c2c00 {
> - compatible = "altr,socfpga-sdmmc-ecc";
> - reg = <0xff8c2c00 0x400>;
> - altr,ecc-parent = <&mmc>;
> - interrupts = <15 IRQ_TYPE_LEVEL_HIGH>,
> - <47 IRQ_TYPE_LEVEL_HIGH>,
> - <16 IRQ_TYPE_LEVEL_HIGH>,
> - <48 IRQ_TYPE_LEVEL_HIGH>;
> + memory@0 {
> + name = "memory";
> + device_type = "memory";
> + reg = <0x0 0x80000000>; /* 2GB */
> };
> };
>
> &gmac0 {
> phy-mode = "rgmii";
> - phy-addr = <0xffffffff>; /* probe for phy addr */
> + phy-handle = <&phy3>;
>
> max-frame-size = <3800>;
> - status = "okay";
> -
> - phy-handle = <&phy3>;
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "snps,dwmac-mdio";
> phy3: ethernet-phy@3 {
> + reg = <3>;
> txd0-skew-ps = <0>; /* -420ps */
> txd1-skew-ps = <0>; /* -420ps */
> txd2-skew-ps = <0>; /* -420ps */
> @@ -64,35 +45,23 @@ phy3: ethernet-phy@3 {
> txc-skew-ps = <1860>; /* 960ps */
> rxdv-skew-ps = <420>; /* 0ps */
> rxc-skew-ps = <1680>; /* 780ps */
> - reg = <3>;
This and few other changes (like memory) are not related to the commit.
Do not mix different cleanups together.
> };
> };
> };
>
> -&gpio0 {
> - status = "okay";
> -};
> -
> -&gpio1 {
> - status = "okay";
> -};
> -
> -&gpio2 {
> - status = "okay";
> -};
Why removing all these? Aren't they available on the SoM? The same
question applies to several other pieces, for example UART and USB -
aren't these part of SoM?
> -
> &i2c1 {
> - status = "okay";
> + atsha204a: atsha204a@64 {
How is this change related at all to what you described in commit? There
was no atsha204a before.
> + compatible = "atmel,atsha204a";
> + reg = <0x64>;
> + };
> +
> isl12022: isl12022@6f {
> - status = "okay";
> compatible = "isil,isl12022";
> reg = <0x6f>;
> };
> };
>
Best regards,
Krzysztof
