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ł