[PATCH 0/4] Update Chameleon v3 configuration

[PATCH 0/4] Update Chameleon v3 configuration

[Paweł Anikiel]

These changes add the third chameleon variation and make it easier to
deploy images to different boards.

Paweł Anikiel (4):
  socfpga: chameleonv3: Enable ext4 in SPL
  socfpga: chameleonv3: Move environment to a text file
  arm: dts: chameleonv3: Override chameleonv3 bitstream names
  arm: dts: chameleonv3: Add 270-2 variant

 arch/arm/dts/Makefile                               |  1 +
 .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi   | 12 ++++++++++++
 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts  |  5 +++++
 .../socfpga_arria10_chameleonv3_270_3-u-boot.dtsi   |  4 ++++
 .../socfpga_arria10_chameleonv3_480_2-u-boot.dtsi   |  4 ++++
 board/google/chameleonv3/environment.txt            | 13 +++++++++++++
 configs/socfpga_chameleonv3_defconfig               |  1 +
 include/configs/socfpga_chameleonv3.h               |  9 ++++-----
 8 files changed, 44 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
 create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
 create mode 100644 board/google/chameleonv3/environment.txt

-- 
2.37.2.672.g94769d06f0-goog

[PATCH 1/4] socfpga: chameleonv3: Enable ext4 in SPL

[Paweł Anikiel]

Allow SPL to boot from an ext4 filesystem.

Signed-off-by: Paweł Anikiel <pan@semihalf.com>
---
 configs/socfpga_chameleonv3_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/socfpga_chameleonv3_defconfig b/configs/socfpga_chameleonv3_defconfig
index e78d3b51de..a6901980b5 100644
--- a/configs/socfpga_chameleonv3_defconfig
+++ b/configs/socfpga_chameleonv3_defconfig
@@ -6,6 +6,7 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_chameleonv3_480_2"
 CONFIG_SPL_TEXT_BASE=0xFFE00000
 CONFIG_SPL_DRIVERS_MISC=y
 CONFIG_TARGET_SOCFPGA_CHAMELEONV3=y
+CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_FS_FAT=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 2/4] socfpga: chameleonv3: Move environment to a text file

[Paweł Anikiel]

Move the environment to an easily editable text file in the boot
partition

Signed-off-by: Paweł Anikiel <pan@semihalf.com>
---
 board/google/chameleonv3/environment.txt | 13 +++++++++++++
 include/configs/socfpga_chameleonv3.h    |  9 ++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)
 create mode 100644 board/google/chameleonv3/environment.txt

diff --git a/board/google/chameleonv3/environment.txt b/board/google/chameleonv3/environment.txt
new file mode 100644
index 0000000000..52aedbb90a
--- /dev/null
+++ b/board/google/chameleonv3/environment.txt
@@ -0,0 +1,13 @@
+# MMC boot command
+bootcmd_mmc=load mmc 0:1 ${loadaddr} kernel.itb; bootm
+
+# Network boot command and vars
+bootcmd_net=dhcp; tftpboot ${loadaddr} kernel.itb; bootm
+autoload=no
+serverip=192.168.0.1
+
+# U-Boot will run this after loading this file
+bootcmd_txt=run bootcmd_mmc
+
+# Kernel cmdline
+bootargs=cma=256M console=ttyS1,115200 root=/dev/mmcblk0p3 rootflags=subvol=root rw rootwait
diff --git a/include/configs/socfpga_chameleonv3.h b/include/configs/socfpga_chameleonv3.h
index 75d2081fac..4c43d89a20 100644
--- a/include/configs/socfpga_chameleonv3.h
+++ b/include/configs/socfpga_chameleonv3.h
@@ -21,11 +21,10 @@
 #define CONFIG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600, 115200}
 
 #define CONFIG_EXTRA_ENV_SETTINGS \
-	"autoload=no\0" \
-	"bootargs=cma=256M console=ttyS1,115200 root=/dev/mmcblk0p3 rw rootwait\0" \
-	"distro_bootcmd=bridge enable; run bootcmd_mmc\0" \
-	"bootcmd_mmc=load mmc 0:1 ${loadaddr} kernel.itb; bootm\0" \
-	"bootcmd_net=dhcp; tftpboot ${loadaddr} kernel.itb; bootm\0"
+	"distro_bootcmd=bridge enable; " \
+		"load mmc 0:1 ${loadaddr} u-boot.txt; " \
+		"env import -t ${loadaddr}; " \
+		"run bootcmd_txt\0"
 
 /*
  * L4 OSC1 Timer 0
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 3/4] arm: dts: chameleonv3: Override chameleonv3 bitstream names

[Paweł Anikiel]

Set the bitstream name per Chameleon variant. This allows the same
boot filesystem with all bitstream variants to be used on different
boards.

Signed-off-by: Paweł Anikiel <pan@semihalf.com>
---
 arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi | 4 ++++
 arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
index e789d49657..a7aa17b220 100644
--- a/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
@@ -6,3 +6,7 @@
 #include "socfpga_arria10-handoff.dtsi"
 #include "socfpga_arria10_handoff_u-boot.dtsi"
 #include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
+
+&fpga_mgr {
+	altr,bitstream = "fpga-270-3.itb";
+};
diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
index 7bbcc471c5..82a94894ea 100644
--- a/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
@@ -6,3 +6,7 @@
 #include "socfpga_arria10-handoff.dtsi"
 #include "socfpga_arria10_handoff_u-boot.dtsi"
 #include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
+
+&fpga_mgr {
+	altr,bitstream = "fpga-480-2.itb";
+};
-- 
2.37.2.672.g94769d06f0-goog

[PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Paweł Anikiel]

Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
Mercury+ AA1 module

Signed-off-by: Paweł Anikiel <pan@semihalf.com>
---
 arch/arm/dts/Makefile                                |  1 +
 .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
 3 files changed, 18 insertions(+)
 create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
 create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 7330121dba..36d5d65595 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=				\
 	socfpga_agilex_socdk.dtb			\
 	socfpga_arria5_secu1.dtb			\
 	socfpga_arria5_socdk.dtb			\
+	socfpga_arria10_chameleonv3_270_2.dtb		\
 	socfpga_arria10_chameleonv3_270_3.dtb		\
 	socfpga_arria10_chameleonv3_480_2.dtb		\
 	socfpga_arria10_socdk_sdmmc.dtb			\
diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
new file mode 100644
index 0000000000..05b4485cf3
--- /dev/null
+++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2022 Google LLC
+ */
+#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
+#include "socfpga_arria10-handoff.dtsi"
+#include "socfpga_arria10_handoff_u-boot.dtsi"
+#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
+
+&fpga_mgr {
+	altr,bitstream = "fpga-270-2.itb";
+};
diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
new file mode 100644
index 0000000000..5f40af6eb9
--- /dev/null
+++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2022 Google LLC
+ */
+#include "socfpga_arria10_chameleonv3.dts"
-- 
2.37.2.672.g94769d06f0-goog

Re: [PATCH 1/4] socfpga: chameleonv3: Enable ext4 in SPL

[Alexandru M Stan]

On Fri, Aug 26, 2022 at 12:54 AM Paweł Anikiel <pan@semihalf.com> wrote:
>
> Allow SPL to boot from an ext4 filesystem.
>
> Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> ---
>  configs/socfpga_chameleonv3_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/socfpga_chameleonv3_defconfig b/configs/socfpga_chameleonv3_defconfig
> index e78d3b51de..a6901980b5 100644
> --- a/configs/socfpga_chameleonv3_defconfig
> +++ b/configs/socfpga_chameleonv3_defconfig
> @@ -6,6 +6,7 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_arria10_chameleonv3_480_2"
>  CONFIG_SPL_TEXT_BASE=0xFFE00000
>  CONFIG_SPL_DRIVERS_MISC=y
>  CONFIG_TARGET_SOCFPGA_CHAMELEONV3=y
> +CONFIG_SPL_FS_EXT4=y
>  CONFIG_SPL_FS_FAT=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_FIT=y
> --
> 2.37.2.672.g94769d06f0-goog
>

Works great for using symlinks on /boot! Thanks!

Tested-by: Alexandru Stan <amstan@chromium.org>
Reviewed-by: Alexandru Stan <amstan@chromium.org>

Re: [PATCH 2/4] socfpga: chameleonv3: Move environment to a text file

[Alexandru M Stan]

On Fri, Aug 26, 2022 at 12:54 AM Paweł Anikiel <pan@semihalf.com> wrote:
>
> Move the environment to an easily editable text file in the boot
> partition
>
> Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> ---
>  board/google/chameleonv3/environment.txt | 13 +++++++++++++
>  include/configs/socfpga_chameleonv3.h    |  9 ++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)
>  create mode 100644 board/google/chameleonv3/environment.txt
>
> diff --git a/board/google/chameleonv3/environment.txt b/board/google/chameleonv3/environment.txt
> new file mode 100644
> index 0000000000..52aedbb90a
> --- /dev/null
> +++ b/board/google/chameleonv3/environment.txt
> @@ -0,0 +1,13 @@
> +# MMC boot command
> +bootcmd_mmc=load mmc 0:1 ${loadaddr} kernel.itb; bootm
> +
> +# Network boot command and vars
> +bootcmd_net=dhcp; tftpboot ${loadaddr} kernel.itb; bootm
> +autoload=no
> +serverip=192.168.0.1
> +
> +# U-Boot will run this after loading this file
> +bootcmd_txt=run bootcmd_mmc
> +
> +# Kernel cmdline
> +bootargs=cma=256M console=ttyS1,115200 root=/dev/mmcblk0p3 rootflags=subvol=root rw rootwait
> diff --git a/include/configs/socfpga_chameleonv3.h b/include/configs/socfpga_chameleonv3.h
> index 75d2081fac..4c43d89a20 100644
> --- a/include/configs/socfpga_chameleonv3.h
> +++ b/include/configs/socfpga_chameleonv3.h
> @@ -21,11 +21,10 @@
>  #define CONFIG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600, 115200}
>
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> -       "autoload=no\0" \
> -       "bootargs=cma=256M console=ttyS1,115200 root=/dev/mmcblk0p3 rw rootwait\0" \
> -       "distro_bootcmd=bridge enable; run bootcmd_mmc\0" \
> -       "bootcmd_mmc=load mmc 0:1 ${loadaddr} kernel.itb; bootm\0" \
> -       "bootcmd_net=dhcp; tftpboot ${loadaddr} kernel.itb; bootm\0"
> +       "distro_bootcmd=bridge enable; " \
> +               "load mmc 0:1 ${loadaddr} u-boot.txt; " \
> +               "env import -t ${loadaddr}; " \
> +               "run bootcmd_txt\0"
>
>  /*
>   * L4 OSC1 Timer 0
> --
> 2.37.2.672.g94769d06f0-goog
>

This has been my request. I really want to be able to change kernel
cmdline by editing a text file in /boot instead of recompiling u-boot.

Tested-by: Alexandru Stan <amstan@chromium.org>

I've been getting the following warning:
> Loading Environment from MMC... *** Warning - bad CRC, using default environment

Is this expected since it's a bare, non CRCd, txt file? Is there any
way we can silence the warning?

Alexandru Stan (amstan)

Re: [PATCH 3/4] arm: dts: chameleonv3: Override chameleonv3 bitstream names

[Alexandru M Stan]

On Fri, Aug 26, 2022 at 12:54 AM Paweł Anikiel <pan@semihalf.com> wrote:
>
> Set the bitstream name per Chameleon variant. This allows the same
> boot filesystem with all bitstream variants to be used on different
> boards.
>
> Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> ---
>  arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi | 4 ++++
>  arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
> index e789d49657..a7aa17b220 100644
> --- a/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
> +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi
> @@ -6,3 +6,7 @@
>  #include "socfpga_arria10-handoff.dtsi"
>  #include "socfpga_arria10_handoff_u-boot.dtsi"
>  #include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> +
> +&fpga_mgr {
> +       altr,bitstream = "fpga-270-3.itb";
> +};
> diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
> index 7bbcc471c5..82a94894ea 100644
> --- a/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
> +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi
> @@ -6,3 +6,7 @@
>  #include "socfpga_arria10-handoff.dtsi"
>  #include "socfpga_arria10_handoff_u-boot.dtsi"
>  #include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> +
> +&fpga_mgr {
> +       altr,bitstream = "fpga-480-2.itb";
> +};
> --
> 2.37.2.672.g94769d06f0-goog
>

Thank you!

Tested-by: Alexandru Stan <amstan@chromium.org>
Reviewed-by: Alexandru Stan <amstan@chromium.org>

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Simon Glass]

Hi Paweł,

On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
>
> Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> Mercury+ AA1 module
>
> Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> ---
>  arch/arm/dts/Makefile                                |  1 +
>  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
>  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
>  3 files changed, 18 insertions(+)
>  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
>  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 7330121dba..36d5d65595 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
>         socfpga_agilex_socdk.dtb                        \
>         socfpga_arria5_secu1.dtb                        \
>         socfpga_arria5_socdk.dtb                        \
> +       socfpga_arria10_chameleonv3_270_2.dtb           \
>         socfpga_arria10_chameleonv3_270_3.dtb           \
>         socfpga_arria10_chameleonv3_480_2.dtb           \
>         socfpga_arria10_socdk_sdmmc.dtb                 \
> diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> new file mode 100644
> index 0000000000..05b4485cf3
> --- /dev/null
> +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2022 Google LLC
> + */
> +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> +#include "socfpga_arria10-handoff.dtsi"
> +#include "socfpga_arria10_handoff_u-boot.dtsi"
> +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> +
> +&fpga_mgr {
> +       altr,bitstream = "fpga-270-2.itb";
> +};
> diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> new file mode 100644
> index 0000000000..5f40af6eb9
> --- /dev/null
> +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2022 Google LLC
> + */
> +#include "socfpga_arria10_chameleonv3.dts"

Can you create a common .dtsi file instead? We should not be including
a .dts file into another file.

> --
> 2.37.2.672.g94769d06f0-goog
>

Regards,
Simon

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Paweł Anikiel]

On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Paweł,
>
> On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> >
> > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > Mercury+ AA1 module
> >
> > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > ---
> >  arch/arm/dts/Makefile                                |  1 +
> >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> >  3 files changed, 18 insertions(+)
> >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index 7330121dba..36d5d65595 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> >         socfpga_agilex_socdk.dtb                        \
> >         socfpga_arria5_secu1.dtb                        \
> >         socfpga_arria5_socdk.dtb                        \
> > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> >         socfpga_arria10_chameleonv3_270_3.dtb           \
> >         socfpga_arria10_chameleonv3_480_2.dtb           \
> >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > new file mode 100644
> > index 0000000000..05b4485cf3
> > --- /dev/null
> > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2022 Google LLC
> > + */
> > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > +#include "socfpga_arria10-handoff.dtsi"
> > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > +
> > +&fpga_mgr {
> > +       altr,bitstream = "fpga-270-2.itb";
> > +};
> > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > new file mode 100644
> > index 0000000000..5f40af6eb9
> > --- /dev/null
> > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > @@ -0,0 +1,5 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2022 Google LLC
> > + */
> > +#include "socfpga_arria10_chameleonv3.dts"
>
> Can you create a common .dtsi file instead? We should not be including
> a .dts file into another file.
>
Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
because nothing includes it (no handoff headers are needed). Is it
fine to have the names differ across U-Boot and Linux?

Regards,
Paweł

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Simon Glass]

Hi Paweł,

On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
>
> On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Paweł,
> >
> > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > >
> > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > Mercury+ AA1 module
> > >
> > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > ---
> > >  arch/arm/dts/Makefile                                |  1 +
> > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > >  3 files changed, 18 insertions(+)
> > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > >


> > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > index 7330121dba..36d5d65595 100644
> > > --- a/arch/arm/dts/Makefile
> > > +++ b/arch/arm/dts/Makefile
> > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > >         socfpga_agilex_socdk.dtb                        \
> > >         socfpga_arria5_secu1.dtb                        \
> > >         socfpga_arria5_socdk.dtb                        \
> > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > new file mode 100644
> > > index 0000000000..05b4485cf3
> > > --- /dev/null
> > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > @@ -0,0 +1,12 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright 2022 Google LLC
> > > + */
> > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > +#include "socfpga_arria10-handoff.dtsi"
> > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > +
> > > +&fpga_mgr {
> > > +       altr,bitstream = "fpga-270-2.itb";
> > > +};
> > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > new file mode 100644
> > > index 0000000000..5f40af6eb9
> > > --- /dev/null
> > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > @@ -0,0 +1,5 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright 2022 Google LLC
> > > + */
> > > +#include "socfpga_arria10_chameleonv3.dts"
> >
> > Can you create a common .dtsi file instead? We should not be including
> > a .dts file into another file.
> >
> Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> because nothing includes it (no handoff headers are needed). Is it
> fine to have the names differ across U-Boot and Linux?

Ideally not, but we should not include a .dts file in another one and
it is probably more important to follow that rule. But why is Linux
not getting this variant?

Regards,
Simon

Re: [PATCH 3/4] arm: dts: chameleonv3: Override chameleonv3 bitstream names

[Simon Glass]

On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
>
> Set the bitstream name per Chameleon variant. This allows the same
> boot filesystem with all bitstream variants to be used on different
> boards.
>
> Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> ---
>  arch/arm/dts/socfpga_arria10_chameleonv3_270_3-u-boot.dtsi | 4 ++++
>  arch/arm/dts/socfpga_arria10_chameleonv3_480_2-u-boot.dtsi | 4 ++++
>  2 files changed, 8 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Alexandru M Stan]

Hey Simon,

On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Paweł,
>
> On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> >
> > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Paweł,
> > >
> > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > >
> > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > Mercury+ AA1 module
> > > >
> > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > ---
> > > >  arch/arm/dts/Makefile                                |  1 +
> > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > >  3 files changed, 18 insertions(+)
> > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > >
>
>
> > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > index 7330121dba..36d5d65595 100644
> > > > --- a/arch/arm/dts/Makefile
> > > > +++ b/arch/arm/dts/Makefile
> > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > >         socfpga_agilex_socdk.dtb                        \
> > > >         socfpga_arria5_secu1.dtb                        \
> > > >         socfpga_arria5_socdk.dtb                        \
> > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > new file mode 100644
> > > > index 0000000000..05b4485cf3
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > @@ -0,0 +1,12 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright 2022 Google LLC
> > > > + */
> > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > +
> > > > +&fpga_mgr {
> > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > +};
> > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > new file mode 100644
> > > > index 0000000000..5f40af6eb9
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > @@ -0,0 +1,5 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright 2022 Google LLC
> > > > + */
> > > > +#include "socfpga_arria10_chameleonv3.dts"
> > >
> > > Can you create a common .dtsi file instead? We should not be including
> > > a .dts file into another file.
> > >
> > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > because nothing includes it (no handoff headers are needed). Is it
> > fine to have the names differ across U-Boot and Linux?
>
> Ideally not, but we should not include a .dts file in another one and
> it is probably more important to follow that rule. But why is Linux
> not getting this variant?
>
> Regards,
> Simon

Linux (at least for the near future) does not care about which variant
it is. The big differences between 270, 480, -2, -3 are mostly about
the number of FPGA logic gates and speed grades. Such things affect
the FPGA bitstream greatly, and might even affect clock presets that
u-boot cares about, but by the time linux loads it doesn't matter
anymore.

Alexandru Stan (amstan)

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Paweł Anikiel]

On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan@chromium.org> wrote:
>
> Hey Simon,
>
> On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Paweł,
> >
> > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> > >
> > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Paweł,
> > > >
> > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > >
> > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > Mercury+ AA1 module
> > > > >
> > > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > > ---
> > > > >  arch/arm/dts/Makefile                                |  1 +
> > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > > >  3 files changed, 18 insertions(+)
> > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > >
> >
> >
> > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > index 7330121dba..36d5d65595 100644
> > > > > --- a/arch/arm/dts/Makefile
> > > > > +++ b/arch/arm/dts/Makefile
> > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > > >         socfpga_agilex_socdk.dtb                        \
> > > > >         socfpga_arria5_secu1.dtb                        \
> > > > >         socfpga_arria5_socdk.dtb                        \
> > > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > new file mode 100644
> > > > > index 0000000000..05b4485cf3
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > @@ -0,0 +1,12 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright 2022 Google LLC
> > > > > + */
> > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > +
> > > > > +&fpga_mgr {
> > > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > > +};
> > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > new file mode 100644
> > > > > index 0000000000..5f40af6eb9
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > @@ -0,0 +1,5 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright 2022 Google LLC
> > > > > + */
> > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > >
> > > > Can you create a common .dtsi file instead? We should not be including
> > > > a .dts file into another file.
> > > >
> > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > because nothing includes it (no handoff headers are needed). Is it
> > > fine to have the names differ across U-Boot and Linux?
> >
> > Ideally not, but we should not include a .dts file in another one and
> > it is probably more important to follow that rule. But why is Linux
> > not getting this variant?
> >
> > Regards,
> > Simon
>
> Linux (at least for the near future) does not care about which variant
> it is. The big differences between 270, 480, -2, -3 are mostly about
> the number of FPGA logic gates and speed grades. Such things affect
> the FPGA bitstream greatly, and might even affect clock presets that
> u-boot cares about, but by the time linux loads it doesn't matter
> anymore.

Perhaps a more detailed explanation:

The Main and Peripheral PLLs (as well as some other clocks) are
configured by U-Boot. On the other hand, Linux expects them to be
configured when it boots, and does not touch them.

The clock configuration depends mainly on the speed grade of the Arria
10 SoC (marked by us as -2 and -3), but also on the fpga hardware
design (e.g. user-defined clocks for the fpga), and is included in the
u-boot devicetree:
> +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> +#include "socfpga_arria10-handoff.dtsi"
> +#include "socfpga_arria10_handoff_u-boot.dtsi"

Linux, on the other hand, doesn't need such information, and there is
no distinction between the different chameleon variants.

Regards,
Paweł

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Simon Glass]

Hi Paweł,

On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel <pan@semihalf.com> wrote:
>
> On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan@chromium.org> wrote:
> >
> > Hey Simon,
> >
> > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Paweł,
> > >
> > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> > > >
> > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Paweł,
> > > > >
> > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > >
> > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > Mercury+ AA1 module
> > > > > >
> > > > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > > > ---
> > > > > >  arch/arm/dts/Makefile                                |  1 +
> > > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > > > >  3 files changed, 18 insertions(+)
> > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > >
> > >
> > >
> > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > > index 7330121dba..36d5d65595 100644
> > > > > > --- a/arch/arm/dts/Makefile
> > > > > > +++ b/arch/arm/dts/Makefile
> > > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > > > >         socfpga_agilex_socdk.dtb                        \
> > > > > >         socfpga_arria5_secu1.dtb                        \
> > > > > >         socfpga_arria5_socdk.dtb                        \
> > > > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > new file mode 100644
> > > > > > index 0000000000..05b4485cf3
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > @@ -0,0 +1,12 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright 2022 Google LLC
> > > > > > + */
> > > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > > +
> > > > > > +&fpga_mgr {
> > > > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > > > +};
> > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > new file mode 100644
> > > > > > index 0000000000..5f40af6eb9
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > @@ -0,0 +1,5 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright 2022 Google LLC
> > > > > > + */
> > > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > > >
> > > > > Can you create a common .dtsi file instead? We should not be including
> > > > > a .dts file into another file.
> > > > >
> > > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > > because nothing includes it (no handoff headers are needed). Is it
> > > > fine to have the names differ across U-Boot and Linux?
> > >
> > > Ideally not, but we should not include a .dts file in another one and
> > > it is probably more important to follow that rule. But why is Linux
> > > not getting this variant?
> > >
> > > Regards,
> > > Simon
> >
> > Linux (at least for the near future) does not care about which variant
> > it is. The big differences between 270, 480, -2, -3 are mostly about
> > the number of FPGA logic gates and speed grades. Such things affect
> > the FPGA bitstream greatly, and might even affect clock presets that
> > u-boot cares about, but by the time linux loads it doesn't matter
> > anymore.
>
> Perhaps a more detailed explanation:
>
> The Main and Peripheral PLLs (as well as some other clocks) are
> configured by U-Boot. On the other hand, Linux expects them to be
> configured when it boots, and does not touch them.
>
> The clock configuration depends mainly on the speed grade of the Arria
> 10 SoC (marked by us as -2 and -3), but also on the fpga hardware
> design (e.g. user-defined clocks for the fpga), and is included in the
> u-boot devicetree:
> > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > +#include "socfpga_arria10-handoff.dtsi"
> > +#include "socfpga_arria10_handoff_u-boot.dtsi"
>
> Linux, on the other hand, doesn't need such information, and there is
> no distinction between the different chameleon variants.

One option would be to put everything in a .dtsi in linux with a
single top-level dts that just includes it. Then U-Boot is not that
different.

Regards,
Simon

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Paweł Anikiel]

On Tue, Aug 30, 2022 at 5:57 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Paweł,
>
> On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel <pan@semihalf.com> wrote:
> >
> > On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan@chromium.org> wrote:
> > >
> > > Hey Simon,
> > >
> > > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Paweł,
> > > >
> > > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > >
> > > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Paweł,
> > > > > >
> > > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > > >
> > > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > > Mercury+ AA1 module
> > > > > > >
> > > > > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > > > > ---
> > > > > > >  arch/arm/dts/Makefile                                |  1 +
> > > > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > > > > >  3 files changed, 18 insertions(+)
> > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > >
> > > >
> > > >
> > > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > > > index 7330121dba..36d5d65595 100644
> > > > > > > --- a/arch/arm/dts/Makefile
> > > > > > > +++ b/arch/arm/dts/Makefile
> > > > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > > > > >         socfpga_agilex_socdk.dtb                        \
> > > > > > >         socfpga_arria5_secu1.dtb                        \
> > > > > > >         socfpga_arria5_socdk.dtb                        \
> > > > > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > > > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > > > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > > > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > new file mode 100644
> > > > > > > index 0000000000..05b4485cf3
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > @@ -0,0 +1,12 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > +/*
> > > > > > > + * Copyright 2022 Google LLC
> > > > > > > + */
> > > > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > > > +
> > > > > > > +&fpga_mgr {
> > > > > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > > > > +};
> > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > new file mode 100644
> > > > > > > index 0000000000..5f40af6eb9
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > @@ -0,0 +1,5 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > +/*
> > > > > > > + * Copyright 2022 Google LLC
> > > > > > > + */
> > > > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > > > >
> > > > > > Can you create a common .dtsi file instead? We should not be including
> > > > > > a .dts file into another file.
> > > > > >
> > > > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > > > because nothing includes it (no handoff headers are needed). Is it
> > > > > fine to have the names differ across U-Boot and Linux?
> > > >
> > > > Ideally not, but we should not include a .dts file in another one and
> > > > it is probably more important to follow that rule. But why is Linux
> > > > not getting this variant?
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > Linux (at least for the near future) does not care about which variant
> > > it is. The big differences between 270, 480, -2, -3 are mostly about
> > > the number of FPGA logic gates and speed grades. Such things affect
> > > the FPGA bitstream greatly, and might even affect clock presets that
> > > u-boot cares about, but by the time linux loads it doesn't matter
> > > anymore.
> >
> > Perhaps a more detailed explanation:
> >
> > The Main and Peripheral PLLs (as well as some other clocks) are
> > configured by U-Boot. On the other hand, Linux expects them to be
> > configured when it boots, and does not touch them.
> >
> > The clock configuration depends mainly on the speed grade of the Arria
> > 10 SoC (marked by us as -2 and -3), but also on the fpga hardware
> > design (e.g. user-defined clocks for the fpga), and is included in the
> > u-boot devicetree:
> > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > +#include "socfpga_arria10-handoff.dtsi"
> > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> >
> > Linux, on the other hand, doesn't need such information, and there is
> > no distinction between the different chameleon variants.
>
> One option would be to put everything in a .dtsi in linux with a
> single top-level dts that just includes it. Then U-Boot is not that
> different.

I assume you mean a .dts file with a single #include "... .dtsi". Do
you think the maintainers will be fine with such change? From the
perspective of Linux it seems strange IMO, especially when there's
nothing else including that .dtsi.

Regards,
Paweł

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Simon Glass]

Hi Paweł,

On Fri, 2 Sept 2022 at 07:16, Paweł Anikiel <pan@semihalf.com> wrote:
>
> On Tue, Aug 30, 2022 at 5:57 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Paweł,
> >
> > On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel <pan@semihalf.com> wrote:
> > >
> > > On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan@chromium.org> wrote:
> > > >
> > > > Hey Simon,
> > > >
> > > > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Paweł,
> > > > >
> > > > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > >
> > > > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Paweł,
> > > > > > >
> > > > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > > > >
> > > > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > > > Mercury+ AA1 module
> > > > > > > >
> > > > > > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > > > > > ---
> > > > > > > >  arch/arm/dts/Makefile                                |  1 +
> > > > > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > > > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > > > > > >  3 files changed, 18 insertions(+)
> > > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > >
> > > > >
> > > > >
> > > > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > > > > index 7330121dba..36d5d65595 100644
> > > > > > > > --- a/arch/arm/dts/Makefile
> > > > > > > > +++ b/arch/arm/dts/Makefile
> > > > > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > > > > > >         socfpga_agilex_socdk.dtb                        \
> > > > > > > >         socfpga_arria5_secu1.dtb                        \
> > > > > > > >         socfpga_arria5_socdk.dtb                        \
> > > > > > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > > > > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > > > > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > > > > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000000..05b4485cf3
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > > @@ -0,0 +1,12 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > > +/*
> > > > > > > > + * Copyright 2022 Google LLC
> > > > > > > > + */
> > > > > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > > > > +
> > > > > > > > +&fpga_mgr {
> > > > > > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > > > > > +};
> > > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000000..5f40af6eb9
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > > @@ -0,0 +1,5 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > > +/*
> > > > > > > > + * Copyright 2022 Google LLC
> > > > > > > > + */
> > > > > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > > > > >
> > > > > > > Can you create a common .dtsi file instead? We should not be including
> > > > > > > a .dts file into another file.
> > > > > > >
> > > > > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > > > > because nothing includes it (no handoff headers are needed). Is it
> > > > > > fine to have the names differ across U-Boot and Linux?
> > > > >
> > > > > Ideally not, but we should not include a .dts file in another one and
> > > > > it is probably more important to follow that rule. But why is Linux
> > > > > not getting this variant?
> > > > >
> > > > > Regards,
> > > > > Simon
> > > >
> > > > Linux (at least for the near future) does not care about which variant
> > > > it is. The big differences between 270, 480, -2, -3 are mostly about
> > > > the number of FPGA logic gates and speed grades. Such things affect
> > > > the FPGA bitstream greatly, and might even affect clock presets that
> > > > u-boot cares about, but by the time linux loads it doesn't matter
> > > > anymore.
> > >
> > > Perhaps a more detailed explanation:
> > >
> > > The Main and Peripheral PLLs (as well as some other clocks) are
> > > configured by U-Boot. On the other hand, Linux expects them to be
> > > configured when it boots, and does not touch them.
> > >
> > > The clock configuration depends mainly on the speed grade of the Arria
> > > 10 SoC (marked by us as -2 and -3), but also on the fpga hardware
> > > design (e.g. user-defined clocks for the fpga), and is included in the
> > > u-boot devicetree:
> > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > >
> > > Linux, on the other hand, doesn't need such information, and there is
> > > no distinction between the different chameleon variants.
> >
> > One option would be to put everything in a .dtsi in linux with a
> > single top-level dts that just includes it. Then U-Boot is not that
> > different.
>
> I assume you mean a .dts file with a single #include "... .dtsi". Do
> you think the maintainers will be fine with such change? From the
> perspective of Linux it seems strange IMO, especially when there's
> nothing else including that .dtsi.

Sure, but the alternative is to upstream one of the other .dts files
from U-Boot, if the maintainer would prefer that.

Have you thought about upstreaming the FPGA binding?

Regards,
Simon

Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

[Paweł Anikiel]

On Fri, Sep 2, 2022 at 9:59 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Paweł,
>
> On Fri, 2 Sept 2022 at 07:16, Paweł Anikiel <pan@semihalf.com> wrote:
> >
> > On Tue, Aug 30, 2022 at 5:57 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Paweł,
> > >
> > > On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel <pan@semihalf.com> wrote:
> > > >
> > > > On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan@chromium.org> wrote:
> > > > >
> > > > > Hey Simon,
> > > > >
> > > > > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Paweł,
> > > > > >
> > > > > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > > >
> > > > > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Paweł,
> > > > > > > >
> > > > > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan@semihalf.com> wrote:
> > > > > > > > >
> > > > > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > > > > Mercury+ AA1 module
> > > > > > > > >
> > > > > > > > > Signed-off-by: Paweł Anikiel <pan@semihalf.com>
> > > > > > > > > ---
> > > > > > > > >  arch/arm/dts/Makefile                                |  1 +
> > > > > > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi    | 12 ++++++++++++
> > > > > > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +++++
> > > > > > > > >  3 files changed, 18 insertions(+)
> > > > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > > >
> > > > > >
> > > > > >
> > > > > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > > > > > index 7330121dba..36d5d65595 100644
> > > > > > > > > --- a/arch/arm/dts/Makefile
> > > > > > > > > +++ b/arch/arm/dts/Makefile
> > > > > > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=                               \
> > > > > > > > >         socfpga_agilex_socdk.dtb                        \
> > > > > > > > >         socfpga_arria5_secu1.dtb                        \
> > > > > > > > >         socfpga_arria5_socdk.dtb                        \
> > > > > > > > > +       socfpga_arria10_chameleonv3_270_2.dtb           \
> > > > > > > > >         socfpga_arria10_chameleonv3_270_3.dtb           \
> > > > > > > > >         socfpga_arria10_chameleonv3_480_2.dtb           \
> > > > > > > > >         socfpga_arria10_socdk_sdmmc.dtb                 \
> > > > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > > > new file mode 100644
> > > > > > > > > index 0000000000..05b4485cf3
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > > > > @@ -0,0 +1,12 @@
> > > > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > > > +/*
> > > > > > > > > + * Copyright 2022 Google LLC
> > > > > > > > > + */
> > > > > > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > > > > > +
> > > > > > > > > +&fpga_mgr {
> > > > > > > > > +       altr,bitstream = "fpga-270-2.itb";
> > > > > > > > > +};
> > > > > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > > > new file mode 100644
> > > > > > > > > index 0000000000..5f40af6eb9
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > > > > @@ -0,0 +1,5 @@
> > > > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > > > +/*
> > > > > > > > > + * Copyright 2022 Google LLC
> > > > > > > > > + */
> > > > > > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > > > > > >
> > > > > > > > Can you create a common .dtsi file instead? We should not be including
> > > > > > > > a .dts file into another file.
> > > > > > > >
> > > > > > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > > > > > because nothing includes it (no handoff headers are needed). Is it
> > > > > > > fine to have the names differ across U-Boot and Linux?
> > > > > >
> > > > > > Ideally not, but we should not include a .dts file in another one and
> > > > > > it is probably more important to follow that rule. But why is Linux
> > > > > > not getting this variant?
> > > > > >
> > > > > > Regards,
> > > > > > Simon
> > > > >
> > > > > Linux (at least for the near future) does not care about which variant
> > > > > it is. The big differences between 270, 480, -2, -3 are mostly about
> > > > > the number of FPGA logic gates and speed grades. Such things affect
> > > > > the FPGA bitstream greatly, and might even affect clock presets that
> > > > > u-boot cares about, but by the time linux loads it doesn't matter
> > > > > anymore.
> > > >
> > > > Perhaps a more detailed explanation:
> > > >
> > > > The Main and Peripheral PLLs (as well as some other clocks) are
> > > > configured by U-Boot. On the other hand, Linux expects them to be
> > > > configured when it boots, and does not touch them.
> > > >
> > > > The clock configuration depends mainly on the speed grade of the Arria
> > > > 10 SoC (marked by us as -2 and -3), but also on the fpga hardware
> > > > design (e.g. user-defined clocks for the fpga), and is included in the
> > > > u-boot devicetree:
> > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > >
> > > > Linux, on the other hand, doesn't need such information, and there is
> > > > no distinction between the different chameleon variants.
> > >
> > > One option would be to put everything in a .dtsi in linux with a
> > > single top-level dts that just includes it. Then U-Boot is not that
> > > different.
> >
> > I assume you mean a .dts file with a single #include "... .dtsi". Do
> > you think the maintainers will be fine with such change? From the
> > perspective of Linux it seems strange IMO, especially when there's
> > nothing else including that .dtsi.
>
> Sure, but the alternative is to upstream one of the other .dts files
> from U-Boot, if the maintainer would prefer that.
>
> Have you thought about upstreaming the FPGA binding?

I just don't think having the different chameleon variants in Linux
makes sense (or FPGA bindings for the same reason), since the only
difference is with bitstream names and the clock settings which Linux
doesn't do.

I think it would be simpler to rename chameleonv3.dts to .dtsi in
u-boot. If that’s fine for you, I’ll prepare a v2 right away.

Regards,
Paweł