On 11/03/2021 19:26, Tom Rix wrote:

On 3/11/21 7:25 AM, Krzysztof Kozlowski wrote:

quoted

Hi,

All three Intel arm64 SoCFPGA architectures (Agilex, N5X and Stratix 10)
are basically flavors/platforms of the same architecture.  At least from
the Linux point of view.  Up to a point that N5X and Agilex share DTSI.
Having three top-level architectures for the same one barely makes
sense and complicates driver selection.

Additionally it was pointed out that ARCH_SOCFPGA name is too generic.
There are other vendors making SoC+FPGA designs, so the name should be
changed to have real vendor (currently: Intel).


Dependencies / merging
======================
1. Patch 1 is used as base, so other changes depend on its hunks.
   I put it at beginning as it is something close to a fix, so candidate
   for stable (even though I did not mark it like that).
2. Patch 2: everything depends on it.

3. 64-bit path:
3a. Patches 3-7: depend on patch 2, from 64-bit point of view.
3b. Patch 8: depends on 2-7 as it finally removes 64-bit ARCH_XXX
    symbols.

4. 32-bit path:
4a. Patches 9-14: depend on 2, from 32-bit point of view.
4b. Patch 15: depends on 9-14 as it finally removes 32-bit ARCH_SOCFPGA
    symbol.

If the patches look good, proposed merging is via SoC tree (after
getting acks from everyone). Sharing immutable branches is also a way.


Changes since v2
================
1. Several new patches and changes.
2. Rename ARCH_SOCFPGA to ARCH_INTEL_SOCFPGA on 32-bit and 64-bit.
3. Enable compile testing of 32-bit socfpga clock drivers.
4. Split changes per subsystems for easier review.
5. I already received an ack from Lee Jones, but I did not add it as
   there was big refactoring.  Please kindly ack one more time if it
   looks good.

Changes since v1
================
1. New patch 3: arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64.
2. New patch 4: arm64: intel: merge Agilex and N5X into ARCH_SOCFPGA64.
3. Fix build is.sue reported by kernel test robot (with ARCH_STRATIX10
   and COMPILE_TEST but without selecting some of the clocks).


RFT
===
I tested compile builds on few configurations, so I hope kbuild 0-day
will check more options (please give it few days on the lists).
I compare the generated autoconf.h and found no issues.  Testing on real
hardware would be appreciated.

Best regards,
Krzysztof

Krzysztof Kozlowski (15):
  clk: socfpga: allow building N5X clocks with ARCH_N5X
  ARM: socfpga: introduce common ARCH_INTEL_SOCFPGA
  mfd: altera: merge ARCH_SOCFPGA and ARCH_STRATIX10
  net: stmmac: merge ARCH_SOCFPGA and ARCH_STRATIX10
  clk: socfpga: build together Stratix 10, Agilex and N5X clock drivers
  clk: socfpga: merge ARCH_SOCFPGA and ARCH_STRATIX10
  EDAC: altera: merge ARCH_SOCFPGA and ARCH_STRATIX10
  arm64: socfpga: merge Agilex and N5X into ARCH_INTEL_SOCFPGA
  clk: socfpga: allow compile testing of Stratix 10 / Agilex clocks
  clk: socfpga: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs (and
    compile test)
  dmaengine: socfpga: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs
  fpga: altera: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs
  i2c: altera: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs
  reset: socfpga: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs
  ARM: socfpga: drop ARCH_SOCFPGA

 arch/arm/Kconfig                            |  2 +-
 arch/arm/Kconfig.debug                      |  6 +++---
 arch/arm/Makefile                           |  2 +-
 arch/arm/boot/dts/Makefile                  |  2 +-
 arch/arm/configs/multi_v7_defconfig         |  2 +-
 arch/arm/configs/socfpga_defconfig          |  2 +-
 arch/arm/mach-socfpga/Kconfig               |  4 ++--
 arch/arm64/Kconfig.platforms                | 17 ++++-------------
 arch/arm64/boot/dts/altera/Makefile         |  2 +-
 arch/arm64/boot/dts/intel/Makefile          |  6 +++---
 arch/arm64/configs/defconfig                |  3 +--
 drivers/clk/Kconfig                         |  1 +
 drivers/clk/Makefile                        |  4 +---
 drivers/clk/socfpga/Kconfig                 | 19 +++++++++++++++++++
 drivers/clk/socfpga/Makefile                | 11 +++++------
 drivers/dma/Kconfig                         |  2 +-
 drivers/edac/Kconfig                        |  2 +-
 drivers/edac/altera_edac.c                  | 17 +++++++++++------
 drivers/firmware/Kconfig                    |  2 +-
 drivers/fpga/Kconfig                        |  8 ++++----
 drivers/i2c/busses/Kconfig                  |  2 +-
 drivers/mfd/Kconfig                         |  4 ++--
 drivers/net/ethernet/stmicro/stmmac/Kconfig |  4 ++--
 drivers/reset/Kconfig                       |  6 +++---
 24 files changed, 71 insertions(+), 59 deletions(-)
 create mode 100644 drivers/clk/socfpga/Kconfig

Thanks for changing the config name.

Please review checkpatch --strict on this set, the typical complaint is

clk: socfpga: use ARCH_INTEL_SOCFPGA also for 32-bit ARM SoCs (and compile test)    
WARNING: please write a paragraph that describes the config symbol fully
#35: FILE: drivers/clk/socfpg/Kconfig:11:                       
+config CLK_INTEL_SOCFPGA32

This symbol is not visible to the user, not selectable, so documenting
it more than what is already written in option title (the one going
after "bool") makes little sense. We don't do it for such drivers.
Mostly because it would be duplication of the option title or include
useless information (it's like documenting "int i" with "counter used
for loop"). The checkpatch complains if this is less than three lines,
but it is not possible to write here anything meaningful for more than
one line.

Really, it does not make sense. If you think otherwise, please suggest
the text which is not duplicating option title and does not include
common stuff from clocks.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel