Re: [PATCH v3 1/1] dts: socfpga: Add Mercury+ AA1 devicetree

[Alexandru M Stan]

On Fri, Oct 8, 2021 at 7:08 AM Paweł Anikiel <pan@xxxxxxxxxxxx> wrote:
>
> Add support for the Mercury+ AA1 module for Arria 10 SoC FPGA.
>
> Signed-off-by: Paweł Anikiel <pan@xxxxxxxxxxxx>
> Signed-off-by: Joanna Brozek <jbrozek@xxxxxxxxxxxx>
> Signed-off-by: Mariusz Glebocki <mglebocki@xxxxxxxxxxxx>
> Signed-off-by: Tomasz Gorochowik <tgorochowik@xxxxxxxxxxxx>
> Signed-off-by: Maciej Mikunda <mmikunda@xxxxxxxxxxxx>
> ---
>  arch/arm/boot/dts/Makefile                    |   1 +
>  .../boot/dts/socfpga_arria10_mercury_aa1.dts  | 112 ++++++++++++++++++
>  2 files changed, 113 insertions(+)
>  create mode 100644 arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7e0934180724..0a7809eb3795 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1078,6 +1078,7 @@ dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += \
>         socfpga_arria10_socdk_nand.dtb \
>         socfpga_arria10_socdk_qspi.dtb \
>         socfpga_arria10_socdk_sdmmc.dtb \
> +       socfpga_arria10_mercury_aa1.dtb \
>         socfpga_cyclone5_chameleon96.dtb \
>         socfpga_cyclone5_mcvevk.dtb \
>         socfpga_cyclone5_socdk.dtb \
> diff --git a/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts b/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts
> new file mode 100644
> index 000000000000..2a3364b26361
> --- /dev/null
> +++ b/arch/arm/boot/dts/socfpga_arria10_mercury_aa1.dts
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/dts-v1/;
> +
> +#include "socfpga_arria10.dtsi"
> +
> +/ {
> +
> +       model = "Enclustra Mercury AA1";
> +       compatible = "altr,socfpga-arria10", "altr,socfpga";
> +
> +       aliases {
> +               ethernet0 = &gmac0;
> +               serial1 = &uart1;
> +               i2c0 = &i2c0;
> +               i2c1 = &i2c1;

Yeah, this is fine now. I still would have added this in
"socfpga_arria10.dtsi" instead. I don't think there's ever a case
where these aliases wouldn't be wanted for any user of this chip.

> +       };
> +
> +       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>;
> +       };
> +};
> +
> +&gmac0 {
> +       phy-mode = "rgmii";
> +       phy-addr = <0xffffffff>; /* probe for phy addr */
> +
> +       max-frame-size = <3800>;
> +       status = "okay";
> +
> +       phy-handle = <&phy3>;
> +
> +       mdio {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "snps,dwmac-mdio";
> +               phy3: ethernet-phy@3 {
> +                       txd0-skew-ps = <0>; /* -420ps */
> +                       txd1-skew-ps = <0>; /* -420ps */
> +                       txd2-skew-ps = <0>; /* -420ps */
> +                       txd3-skew-ps = <0>; /* -420ps */
> +                       rxd0-skew-ps = <420>; /* 0ps */
> +                       rxd1-skew-ps = <420>; /* 0ps */
> +                       rxd2-skew-ps = <420>; /* 0ps */
> +                       rxd3-skew-ps = <420>; /* 0ps */
> +                       txen-skew-ps = <0>; /* -420ps */
> +                       txc-skew-ps = <1860>; /* 960ps */
> +                       rxdv-skew-ps = <420>; /* 0ps */
> +                       rxc-skew-ps = <1680>; /* 780ps */
> +                       reg = <3>;
> +               };
> +       };
> +};
> +
> +&gpio0 {
> +       status = "okay";
> +};
> +
> +&gpio1 {
> +       status = "okay";
> +};
> +
> +&gpio2 {
> +       status = "okay";
> +};
> +
> +&i2c1 {
> +       status = "okay";
> +       isl12022: isl12022@6f {
> +               status = "okay";
> +               compatible = "isil,isl12022";
> +               reg = <0x6f>;
> +       };
> +};
> +
> +/* Following mappings are taken from arria10 socdk dts */
> +&mmc {
> +       status = "okay";
> +       cap-sd-highspeed;
> +       broken-cd;
> +       bus-width = <4>;
> +};
> +
> +&osc1 {
> +       clock-frequency = <33330000>;
> +};
> +
> +&uart1 {
> +       status = "okay";
> +};
> +
> +&usb0 {
> +       status = "okay";
> +       dr_mode = "host";
> +};
> --
> 2.25.1
>

Hello Paweł,
Thank you for respinning.

This looks good as a base to make the boards be able to boot. I'd be
happy if it lands (even the v3 patch).
Just a small nit about the location of the aliases.

Reviewed-by: Alexandru M Stan <amstan@xxxxxxxxxxxx>

Thanks,
Alexandru Stan