[PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant
Simon Glass
sjg at chromium.org
Fri Sep 2 21:59:10 CEST 2022
Hi Paweł,
On Fri, 2 Sept 2022 at 07:16, Paweł Anikiel <pan at semihalf.com> wrote:
>
> On Tue, Aug 30, 2022 at 5:57 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Paweł,
> >
> > On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel <pan at semihalf.com> wrote:
> > >
> > > On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan <amstan at chromium.org> wrote:
> > > >
> > > > Hey Simon,
> > > >
> > > > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Paweł,
> > > > >
> > > > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel <pan at semihalf.com> wrote:
> > > > > >
> > > > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Paweł,
> > > > > > >
> > > > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel <pan at semihalf.com> wrote:
> > > > > > > >
> > > > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > > > Mercury+ AA1 module
> > > > > > > >
> > > > > > > > Signed-off-by: Paweł Anikiel <pan at 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
More information about the U-Boot
mailing list