Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 11/03/2021 10:14, Arnd Bergmann wrote: > On Thu, Mar 11, 2021 at 8:08 AM Krzysztof Kozlowski > wrote: >> On 10/03/2021 17:42, Arnd Bergmann wrote: >>> On Wed, Mar 10, 2021 at 4:54 PM Krzysztof Kozlowski >>> wrote: On 10/03/2021 16:47, Krzysztof Kozlowski wrote: > This edac Altera driver is very weird... it uses the same compatible > differently depending whether this is 32-bit or 64-bit (e.g. Stratix > 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we > have two. It's quite a new code (2019 from Intel), not some ancient > legacy, so it should never have been accepted... Oh, it's not that horrible as it sounds. They actually have different compatibles for edac driver with these differences (e.g. in interrupts). They just do not use them and instead check for the basic (common?) compatible and architecture... Anyway without testing I am not the person to fix the edac driver. >>> >>> Ok, This should be fixed properly as you describe, but as a quick hack >>> it wouldn't be hard to just change the #ifdef to check for CONFIG_64BIT >>> instead of CONFIG_ARCH_STRATIX10 during the rename of the config >>> symbol. >> >> This would work. The trouble with renaming ARCH_SOCFPGA into >> ARCH_INTEL_SOCFPGA is that still SOCFPGA will appear in many other >> Kconfig symbols or even directory paths. >> >> Let me use ARCH_INTEL_SOCFPGA for 64bit here and renaming of 32bit a >> little bit later. > > Maybe you can introduce a hidden 'ARCH_INTEL_SOCFPGA' option first > and select that from both the 32-bit and the 64-bit platforms in the first > step. > > That should decouple the cleanups, so you can change the drivers to > (only) 'depends on ARCH_INTEL_SOCFPGA' before removing the other > names. Sure, let me try that. I have a v3 almost ready. Best regards, Krzysztof
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On Thu, Mar 11, 2021 at 8:08 AM Krzysztof Kozlowski wrote: > On 10/03/2021 17:42, Arnd Bergmann wrote: > > On Wed, Mar 10, 2021 at 4:54 PM Krzysztof Kozlowski > > wrote: > >> On 10/03/2021 16:47, Krzysztof Kozlowski wrote: > >>> This edac Altera driver is very weird... it uses the same compatible > >>> differently depending whether this is 32-bit or 64-bit (e.g. Stratix > >>> 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we > >>> have two. It's quite a new code (2019 from Intel), not some ancient > >>> legacy, so it should never have been accepted... > >> > >> Oh, it's not that horrible as it sounds. They actually have different > >> compatibles for edac driver with these differences (e.g. in interrupts). > >> They just do not use them and instead check for the basic (common?) > >> compatible and architecture... Anyway without testing I am not the > >> person to fix the edac driver. > > > > Ok, This should be fixed properly as you describe, but as a quick hack > > it wouldn't be hard to just change the #ifdef to check for CONFIG_64BIT > > instead of CONFIG_ARCH_STRATIX10 during the rename of the config > > symbol. > > This would work. The trouble with renaming ARCH_SOCFPGA into > ARCH_INTEL_SOCFPGA is that still SOCFPGA will appear in many other > Kconfig symbols or even directory paths. > > Let me use ARCH_INTEL_SOCFPGA for 64bit here and renaming of 32bit a > little bit later. Maybe you can introduce a hidden 'ARCH_INTEL_SOCFPGA' option first and select that from both the 32-bit and the 64-bit platforms in the first step. That should decouple the cleanups, so you can change the drivers to (only) 'depends on ARCH_INTEL_SOCFPGA' before removing the other names. Arnd
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 10/03/2021 17:42, Arnd Bergmann wrote: > On Wed, Mar 10, 2021 at 4:54 PM Krzysztof Kozlowski > wrote: >> On 10/03/2021 16:47, Krzysztof Kozlowski wrote: >>> This edac Altera driver is very weird... it uses the same compatible >>> differently depending whether this is 32-bit or 64-bit (e.g. Stratix >>> 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we >>> have two. It's quite a new code (2019 from Intel), not some ancient >>> legacy, so it should never have been accepted... >> >> Oh, it's not that horrible as it sounds. They actually have different >> compatibles for edac driver with these differences (e.g. in interrupts). >> They just do not use them and instead check for the basic (common?) >> compatible and architecture... Anyway without testing I am not the >> person to fix the edac driver. > > Ok, This should be fixed properly as you describe, but as a quick hack > it wouldn't be hard to just change the #ifdef to check for CONFIG_64BIT > instead of CONFIG_ARCH_STRATIX10 during the rename of the config > symbol. This would work. The trouble with renaming ARCH_SOCFPGA into ARCH_INTEL_SOCFPGA is that still SOCFPGA will appear in many other Kconfig symbols or even directory paths. Let me use ARCH_INTEL_SOCFPGA for 64bit here and renaming of 32bit a little bit later. Best regards, Krzysztof
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On Wed, Mar 10, 2021 at 4:54 PM Krzysztof Kozlowski wrote: > On 10/03/2021 16:47, Krzysztof Kozlowski wrote: > > This edac Altera driver is very weird... it uses the same compatible > > differently depending whether this is 32-bit or 64-bit (e.g. Stratix > > 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we > > have two. It's quite a new code (2019 from Intel), not some ancient > > legacy, so it should never have been accepted... > > Oh, it's not that horrible as it sounds. They actually have different > compatibles for edac driver with these differences (e.g. in interrupts). > They just do not use them and instead check for the basic (common?) > compatible and architecture... Anyway without testing I am not the > person to fix the edac driver. Ok, This should be fixed properly as you describe, but as a quick hack it wouldn't be hard to just change the #ifdef to check for CONFIG_64BIT instead of CONFIG_ARCH_STRATIX10 during the rename of the config symbol. Arnd
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 10/03/2021 16:47, Krzysztof Kozlowski wrote: > On 10/03/2021 16:39, Arnd Bergmann wrote: >> On Wed, Mar 10, 2021 at 4:06 PM Krzysztof Kozlowski >> wrote: >>> On 10/03/2021 15:45, Tom Rix wrote: On 3/10/21 1:45 AM, Lee Jones wrote: >>> >>> Many other architectures do not have vendor prefix (TEGRA, EXYNOS, >>> ZYNQMP etc). I would call it the same as in ARMv7 - ARCH_SOCFPGA - but >>> the Altera EDAC driver depends on these symbols to be different. >>> Anyway, I don't mind using something else for the name. >> >> I agree the name SOCFPGA is confusing, since it is really a class of >> device that is made by multiple manufacturers rather than a brand name, >> but renaming that now would be equally confusing. If the Intel folks >> could suggest a better name that describes all products in the platform >> and is less ambiguous, we could rename it to that. I think ARCH_ALTERA >> would make sense, but I don't know if that is a name that is getting >> phased out. (We once renamed the Marvell Orion platform to ARCH_MVEBU, >> but shortly afterwards, Marvell renamed their embedded business unit (EBU) >> and the name has no longer made sense since). > > I wait then for some ideas from Dinh (or anyone else). > >> >> Regardless of what name we end up with, I do think we should have the >> same name for 32-bit and 64-bit and instead fix the edac driver to do >> runtime detection based on the compatible string. > > I can rename ARCH_SOCFPGA on 32-bit ARM as well, however converting edac > driver from #ifdef ARCH_SOCFPGA64 to proper compatible string will be > too much for me: I am not able to test it. > > This edac Altera driver is very weird... it uses the same compatible > differently depending whether this is 32-bit or 64-bit (e.g. Stratix > 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we > have two. It's quite a new code (2019 from Intel), not some ancient > legacy, so it should never have been accepted... Oh, it's not that horrible as it sounds. They actually have different compatibles for edac driver with these differences (e.g. in interrupts). They just do not use them and instead check for the basic (common?) compatible and architecture... Anyway without testing I am not the person to fix the edac driver. Best regards, Krzysztof
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 10/03/2021 16:39, Arnd Bergmann wrote: > On Wed, Mar 10, 2021 at 4:06 PM Krzysztof Kozlowski > wrote: >> On 10/03/2021 15:45, Tom Rix wrote: >>> On 3/10/21 1:45 AM, Lee Jones wrote: >> >> Many other architectures do not have vendor prefix (TEGRA, EXYNOS, >> ZYNQMP etc). I would call it the same as in ARMv7 - ARCH_SOCFPGA - but >> the Altera EDAC driver depends on these symbols to be different. >> Anyway, I don't mind using something else for the name. > > I agree the name SOCFPGA is confusing, since it is really a class of > device that is made by multiple manufacturers rather than a brand name, > but renaming that now would be equally confusing. If the Intel folks > could suggest a better name that describes all products in the platform > and is less ambiguous, we could rename it to that. I think ARCH_ALTERA > would make sense, but I don't know if that is a name that is getting > phased out. (We once renamed the Marvell Orion platform to ARCH_MVEBU, > but shortly afterwards, Marvell renamed their embedded business unit (EBU) > and the name has no longer made sense since). I wait then for some ideas from Dinh (or anyone else). > > Regardless of what name we end up with, I do think we should have the > same name for 32-bit and 64-bit and instead fix the edac driver to do > runtime detection based on the compatible string. I can rename ARCH_SOCFPGA on 32-bit ARM as well, however converting edac driver from #ifdef ARCH_SOCFPGA64 to proper compatible string will be too much for me: I am not able to test it. This edac Altera driver is very weird... it uses the same compatible differently depending whether this is 32-bit or 64-bit (e.g. Stratix 10)! On ARMv7 the compatible means for example one IRQ... On ARMv8, we have two. It's quite a new code (2019 from Intel), not some ancient legacy, so it should never have been accepted... Best regards, Krzysztof
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On Wed, Mar 10, 2021 at 4:06 PM Krzysztof Kozlowski wrote: > On 10/03/2021 15:45, Tom Rix wrote: > > On 3/10/21 1:45 AM, Lee Jones wrote: > > Many other architectures do not have vendor prefix (TEGRA, EXYNOS, > ZYNQMP etc). I would call it the same as in ARMv7 - ARCH_SOCFPGA - but > the Altera EDAC driver depends on these symbols to be different. > Anyway, I don't mind using something else for the name. I agree the name SOCFPGA is confusing, since it is really a class of device that is made by multiple manufacturers rather than a brand name, but renaming that now would be equally confusing. If the Intel folks could suggest a better name that describes all products in the platform and is less ambiguous, we could rename it to that. I think ARCH_ALTERA would make sense, but I don't know if that is a name that is getting phased out. (We once renamed the Marvell Orion platform to ARCH_MVEBU, but shortly afterwards, Marvell renamed their embedded business unit (EBU) and the name has no longer made sense since). Regardless of what name we end up with, I do think we should have the same name for 32-bit and 64-bit and instead fix the edac driver to do runtime detection based on the compatible string. Arnd
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 10/03/2021 15:45, Tom Rix wrote: > > On 3/10/21 1:45 AM, Lee Jones wrote: >> On Wed, 10 Mar 2021, Krzysztof Kozlowski wrote: >> >>> Prepare for merging Stratix 10, Agilex and N5X into one arm64 >>> architecture by first renaming the ARCH_STRATIX10 into ARCH_SOCFPGA64. >>> >>> The existing ARCH_SOCFPGA (in ARMv7) Kconfig symbol cannot be used >>> because altera_edac driver builds differently between them (with >>> ifdefs). >>> >>> Signed-off-by: Krzysztof Kozlowski >>> --- >>> arch/arm64/Kconfig.platforms| 7 --- >>> arch/arm64/boot/dts/altera/Makefile | 2 +- >>> arch/arm64/configs/defconfig| 2 +- >>> drivers/clk/Makefile| 2 +- >>> drivers/clk/socfpga/Kconfig | 4 ++-- >>> drivers/edac/Kconfig| 2 +- >>> drivers/edac/altera_edac.c | 10 +- >>> drivers/firmware/Kconfig| 2 +- >>> drivers/fpga/Kconfig| 2 +- >>> drivers/mfd/Kconfig | 2 +- >> If it's okay with everyone else, it'll be okay with me: >> >> Acked-by: Lee Jones > > I think the name is too broad, from the description in the config > > + bool "Intel's SoCFPGA ARMv8 Families" > > A better name would be ARCH_INTEL_SOCFPGA64 > > So other vendors like Xilinx could do their own thing. Many other architectures do not have vendor prefix (TEGRA, EXYNOS, ZYNQMP etc). I would call it the same as in ARMv7 - ARCH_SOCFPGA - but the Altera EDAC driver depends on these symbols to be different. Anyway, I don't mind using something else for the name. Best regards, Krzysztof
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On 3/10/21 1:45 AM, Lee Jones wrote: > On Wed, 10 Mar 2021, Krzysztof Kozlowski wrote: > >> Prepare for merging Stratix 10, Agilex and N5X into one arm64 >> architecture by first renaming the ARCH_STRATIX10 into ARCH_SOCFPGA64. >> >> The existing ARCH_SOCFPGA (in ARMv7) Kconfig symbol cannot be used >> because altera_edac driver builds differently between them (with >> ifdefs). >> >> Signed-off-by: Krzysztof Kozlowski >> --- >> arch/arm64/Kconfig.platforms| 7 --- >> arch/arm64/boot/dts/altera/Makefile | 2 +- >> arch/arm64/configs/defconfig| 2 +- >> drivers/clk/Makefile| 2 +- >> drivers/clk/socfpga/Kconfig | 4 ++-- >> drivers/edac/Kconfig| 2 +- >> drivers/edac/altera_edac.c | 10 +- >> drivers/firmware/Kconfig| 2 +- >> drivers/fpga/Kconfig| 2 +- >> drivers/mfd/Kconfig | 2 +- > If it's okay with everyone else, it'll be okay with me: > > Acked-by: Lee Jones I think the name is too broad, from the description in the config + bool "Intel's SoCFPGA ARMv8 Families" A better name would be ARCH_INTEL_SOCFPGA64 So other vendors like Xilinx could do their own thing. Tom > >> drivers/net/ethernet/stmicro/stmmac/Kconfig | 4 ++-- >> drivers/reset/Kconfig | 2 +- >> 12 files changed, 21 insertions(+), 20 deletions(-)
Re: [RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
On Wed, 10 Mar 2021, Krzysztof Kozlowski wrote: > Prepare for merging Stratix 10, Agilex and N5X into one arm64 > architecture by first renaming the ARCH_STRATIX10 into ARCH_SOCFPGA64. > > The existing ARCH_SOCFPGA (in ARMv7) Kconfig symbol cannot be used > because altera_edac driver builds differently between them (with > ifdefs). > > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm64/Kconfig.platforms| 7 --- > arch/arm64/boot/dts/altera/Makefile | 2 +- > arch/arm64/configs/defconfig| 2 +- > drivers/clk/Makefile| 2 +- > drivers/clk/socfpga/Kconfig | 4 ++-- > drivers/edac/Kconfig| 2 +- > drivers/edac/altera_edac.c | 10 +- > drivers/firmware/Kconfig| 2 +- > drivers/fpga/Kconfig| 2 +- > drivers/mfd/Kconfig | 2 +- If it's okay with everyone else, it'll be okay with me: Acked-by: Lee Jones > drivers/net/ethernet/stmicro/stmmac/Kconfig | 4 ++-- > drivers/reset/Kconfig | 2 +- > 12 files changed, 21 insertions(+), 20 deletions(-) -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
[RFC v2 3/5] arm64: socfpga: rename ARCH_STRATIX10 to ARCH_SOCFPGA64
Prepare for merging Stratix 10, Agilex and N5X into one arm64 architecture by first renaming the ARCH_STRATIX10 into ARCH_SOCFPGA64. The existing ARCH_SOCFPGA (in ARMv7) Kconfig symbol cannot be used because altera_edac driver builds differently between them (with ifdefs). Signed-off-by: Krzysztof Kozlowski --- arch/arm64/Kconfig.platforms| 7 --- arch/arm64/boot/dts/altera/Makefile | 2 +- arch/arm64/configs/defconfig| 2 +- drivers/clk/Makefile| 2 +- drivers/clk/socfpga/Kconfig | 4 ++-- drivers/edac/Kconfig| 2 +- drivers/edac/altera_edac.c | 10 +- drivers/firmware/Kconfig| 2 +- drivers/fpga/Kconfig| 2 +- drivers/mfd/Kconfig | 2 +- drivers/net/ethernet/stmicro/stmmac/Kconfig | 4 ++-- drivers/reset/Kconfig | 2 +- 12 files changed, 21 insertions(+), 20 deletions(-) diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index cdfd5fed457f..78bba3633eab 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -254,10 +254,11 @@ config ARCH_SEATTLE help This enables support for AMD Seattle SOC Family -config ARCH_STRATIX10 - bool "Altera's Stratix 10 SoCFPGA Family" +config ARCH_SOCFPGA64 + bool "Intel's SoCFPGA ARMv8 Families" help - This enables support for Altera's Stratix 10 SoCFPGA Family. + This enables support for Intel's SoCFPGA ARMv8 families: + Stratix 10 (ex. Altera), Agilex and eASIC N5X. config ARCH_SYNQUACER bool "Socionext SynQuacer SoC Family" diff --git a/arch/arm64/boot/dts/altera/Makefile b/arch/arm64/boot/dts/altera/Makefile index 10119c7ab437..b45b92f8b4ec 100644 --- a/arch/arm64/boot/dts/altera/Makefile +++ b/arch/arm64/boot/dts/altera/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -dtb-$(CONFIG_ARCH_STRATIX10) += socfpga_stratix10_socdk.dtb \ +dtb-$(CONFIG_ARCH_SOCFPGA64) += socfpga_stratix10_socdk.dtb \ socfpga_stratix10_socdk_nand.dtb diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index d612f633b771..ee742900a150 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -50,7 +50,7 @@ CONFIG_ARCH_RENESAS=y CONFIG_ARCH_ROCKCHIP=y CONFIG_ARCH_S32=y CONFIG_ARCH_SEATTLE=y -CONFIG_ARCH_STRATIX10=y +CONFIG_ARCH_SOCFPGA64=y CONFIG_ARCH_SYNQUACER=y CONFIG_ARCH_TEGRA=y CONFIG_ARCH_SPRD=y diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 12e46b12e587..e34457539edf 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -107,7 +107,7 @@ obj-$(CONFIG_CLK_SIFIVE)+= sifive/ obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/ obj-$(CONFIG_ARCH_AGILEX) += socfpga/ obj-$(CONFIG_ARCH_N5X) += socfpga/ -obj-$(CONFIG_ARCH_STRATIX10) += socfpga/ +obj-$(CONFIG_ARCH_SOCFPGA64) += socfpga/ obj-$(CONFIG_PLAT_SPEAR) += spear/ obj-y += sprd/ obj-$(CONFIG_ARCH_STI) += st/ diff --git a/drivers/clk/socfpga/Kconfig b/drivers/clk/socfpga/Kconfig index 7d4772faf93d..8cf3dfdba255 100644 --- a/drivers/clk/socfpga/Kconfig +++ b/drivers/clk/socfpga/Kconfig @@ -2,5 +2,5 @@ config COMMON_CLK_SOCFPGA64 bool # Intel Stratix / Agilex / N5X clock controller support - default y if ARCH_AGILEX || ARCH_N5X || ARCH_STRATIX10 - depends on ARCH_AGILEX || ARCH_N5X || ARCH_STRATIX10 + default y if ARCH_AGILEX || ARCH_N5X || ARCH_SOCFPGA64 + depends on ARCH_AGILEX || ARCH_N5X || ARCH_SOCFPGA64 diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig index 27d0c4cdc58d..97fa2056bd47 100644 --- a/drivers/edac/Kconfig +++ b/drivers/edac/Kconfig @@ -396,7 +396,7 @@ config EDAC_THUNDERX config EDAC_ALTERA bool "Altera SOCFPGA ECC" - depends on EDAC=y && (ARCH_SOCFPGA || ARCH_STRATIX10) + depends on EDAC=y && (ARCH_SOCFPGA || ARCH_SOCFPGA64) help Support for error detection and correction on the Altera SOCs. This is the global enable for the diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c index e91cf1147a4e..ba11219664ca 100644 --- a/drivers/edac/altera_edac.c +++ b/drivers/edac/altera_edac.c @@ -1502,7 +1502,7 @@ static int altr_portb_setup(struct altr_edac_device_dev *device) dci->dev_name = ecc_name; /* Update the PortB IRQs - A10 has 4, S10 has 2, Index accordingly */ -#ifdef CONFIG_ARCH_STRATIX10 +#ifdef CONFIG_ARCH_SOCFPGA64 altdev->sb_irq = irq_of_parse_and_map(np, 1); #else altdev->sb_irq = irq_of_parse_and_map(np, 2); @@ -1521,7 +1521,7 @@ static int altr_portb_setup(struct altr_edac_device_dev *device) goto err_release_group_1; } -#ifdef
