2022-07-13 16:14:21

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] arm: bcmbca: Move BCM63138 ARCH_BCM_63XX to ARCH_BCMBCA

On 7.07.2022 09:00, William Zhang wrote:
> Remove ARCH_BCM_63XX Kconfig for BCM63138 and merge its selections to
> ARCH_BCMBCA. Delete bcm63xx.c as it is no longer needed.
>
> Signed-off-by: William Zhang <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/arm/mach-bcm/Kconfig | 26 +++++++++-----------------
> arch/arm/mach-bcm/Makefile | 7 +------
> arch/arm/mach-bcm/bcm63xx.c | 17 -----------------
> 3 files changed, 10 insertions(+), 40 deletions(-)
> delete mode 100644 arch/arm/mach-bcm/bcm63xx.c
>
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index f73a056bf560..25aa4ef4db07 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -182,23 +182,6 @@ config ARCH_BCM_53573
> The base chip is BCM53573 and there are some packaging modifications
> like BCM47189 and BCM47452.
>
> -config ARCH_BCM_63XX
> - bool "Broadcom BCM63xx DSL SoC"
> - depends on ARCH_MULTI_V7
> - select ARCH_HAS_RESET_CONTROLLER
> - select ARM_ERRATA_754322
> - select ARM_ERRATA_764369 if SMP
> - select ARM_GIC
> - select ARM_GLOBAL_TIMER
> - select CACHE_L2X0
> - select HAVE_ARM_ARCH_TIMER
> - select HAVE_ARM_TWD if SMP
> - select HAVE_ARM_SCU if SMP
> - help
> - This enables support for systems based on Broadcom DSL SoCs.
> - It currently supports the 'BCM63XX' ARM-based family, which includes
> - the BCM63138 variant.

Oh, wait a second. This is actually going to totally break builds for
people because there are a lot of Kconfig-s that depend on it!

I don't believe it can't be handled this way.

You need to first introduce a replacement / parent symbol.
Then (in another release if needed) update all Kconfig-s.
Then eventually you can get rid of ARCH_BCM_63XX.

2022-07-13 16:23:05

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] arm: bcmbca: Move BCM63138 ARCH_BCM_63XX to ARCH_BCMBCA

On 7.07.2022 09:00, William Zhang wrote:
> Remove ARCH_BCM_63XX Kconfig for BCM63138 and merge its selections to
> ARCH_BCMBCA. Delete bcm63xx.c as it is no longer needed.
>
> Signed-off-by: William Zhang <[email protected]>

What about ARCH_BCM_63XX references in
1. arch/arm/Kconfig.debug
2. arch/arm/configs/multi_v7_defconfig

Did you handle them in some other patchset?


This change is probably going to break "make oldconfig" for users. Is
there any kernel policy for that? Do we care about it? Should we leave
old symbol as hidden and make ARCH_BCMBCA auto-selected if it's set?


Finally it'd probably be more clean to first introduce
ARCH_BCMBCA_CORTEXA9 and then get rid of ARCH_BCM_63XX.


> ---
>
> (no changes since v1)
>
> arch/arm/mach-bcm/Kconfig | 26 +++++++++-----------------
> arch/arm/mach-bcm/Makefile | 7 +------
> arch/arm/mach-bcm/bcm63xx.c | 17 -----------------
> 3 files changed, 10 insertions(+), 40 deletions(-)
> delete mode 100644 arch/arm/mach-bcm/bcm63xx.c
>
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index f73a056bf560..25aa4ef4db07 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -182,23 +182,6 @@ config ARCH_BCM_53573
> The base chip is BCM53573 and there are some packaging modifications
> like BCM47189 and BCM47452.
>
> -config ARCH_BCM_63XX
> - bool "Broadcom BCM63xx DSL SoC"
> - depends on ARCH_MULTI_V7
> - select ARCH_HAS_RESET_CONTROLLER
> - select ARM_ERRATA_754322
> - select ARM_ERRATA_764369 if SMP
> - select ARM_GIC
> - select ARM_GLOBAL_TIMER
> - select CACHE_L2X0
> - select HAVE_ARM_ARCH_TIMER
> - select HAVE_ARM_TWD if SMP
> - select HAVE_ARM_SCU if SMP
> - help
> - This enables support for systems based on Broadcom DSL SoCs.
> - It currently supports the 'BCM63XX' ARM-based family, which includes
> - the BCM63138 variant.
> -
> config ARCH_BRCMSTB
> bool "Broadcom BCM7XXX based boards"
> depends on ARCH_MULTI_V7
> @@ -224,10 +207,19 @@ config ARCH_BCMBCA
> select ARM_AMBA
> select ARM_GIC
> select HAVE_ARM_ARCH_TIMER
> + select ARCH_HAS_RESET_CONTROLLER
> + select ARM_ERRATA_754322
> + select ARM_ERRATA_764369 if SMP
> + select ARM_GLOBAL_TIMER
> + select CACHE_L2X0
> + select HAVE_ARM_TWD if SMP
> + select HAVE_ARM_SCU if SMP
> +
> help
> Say Y if you intend to run the kernel on a Broadcom Broadband ARM-based
> BCA chipset.
>
> This enables support for Broadcom BCA ARM-based broadband chipsets,
> including the DSL, PON and Wireless family of chips.
> +
> endif
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 284ea2f8cbc2..25347d7049b8 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -50,12 +50,6 @@ ifeq ($(CONFIG_ARCH_BCM_5301X),y)
> obj-$(CONFIG_SMP) += platsmp.o
> endif
>
> -# BCM63XXx
> -ifeq ($(CONFIG_ARCH_BCM_63XX),y)
> -obj-y += bcm63xx.o
> -obj-$(CONFIG_SMP) += bcm63xx_smp.o bcm63xx_pmb.o
> -endif
> -
> ifeq ($(CONFIG_ARCH_BRCMSTB),y)
> CFLAGS_platsmp-brcmstb.o += -march=armv7-a
> obj-y += brcmstb.o
> @@ -64,5 +58,6 @@ endif
>
> # BCMBCA
> ifeq ($(CONFIG_ARCH_BCMBCA),y)
> +obj-$(CONFIG_SMP) += bcm63xx_smp.o bcm63xx_pmb.o
> obj-y += bcmbca.o
> endif
> diff --git a/arch/arm/mach-bcm/bcm63xx.c b/arch/arm/mach-bcm/bcm63xx.c
> deleted file mode 100644
> index f855e361dfba..000000000000
> --- a/arch/arm/mach-bcm/bcm63xx.c
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -// Copyright (C) 2014 Broadcom Corporation
> -
> -#include <linux/of_platform.h>
> -
> -#include <asm/mach/arch.h>
> -
> -static const char * const bcm63xx_dt_compat[] = {
> - "brcm,bcm63138",
> - NULL
> -};
> -
> -DT_MACHINE_START(BCM63XXX_DT, "BCM63xx DSL SoC")
> - .dt_compat = bcm63xx_dt_compat,
> - .l2c_aux_val = 0,
> - .l2c_aux_mask = ~0,
> -MACHINE_END

2022-07-13 16:49:16

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] arm: bcmbca: Move BCM63138 ARCH_BCM_63XX to ARCH_BCMBCA

On 7/13/22 08:35, Rafał Miłecki wrote:
> On 7.07.2022 09:00, William Zhang wrote:
>> Remove ARCH_BCM_63XX Kconfig for BCM63138 and merge its selections to
>> ARCH_BCMBCA. Delete bcm63xx.c as it is no longer needed.
>>
>> Signed-off-by: William Zhang <[email protected]>
>> ---
>>
>> (no changes since v1)
>>
>>   arch/arm/mach-bcm/Kconfig   | 26 +++++++++-----------------
>>   arch/arm/mach-bcm/Makefile  |  7 +------
>>   arch/arm/mach-bcm/bcm63xx.c | 17 -----------------
>>   3 files changed, 10 insertions(+), 40 deletions(-)
>>   delete mode 100644 arch/arm/mach-bcm/bcm63xx.c
>>
>> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> index f73a056bf560..25aa4ef4db07 100644
>> --- a/arch/arm/mach-bcm/Kconfig
>> +++ b/arch/arm/mach-bcm/Kconfig
>> @@ -182,23 +182,6 @@ config ARCH_BCM_53573
>>         The base chip is BCM53573 and there are some packaging
>> modifications
>>         like BCM47189 and BCM47452.
>> -config ARCH_BCM_63XX
>> -    bool "Broadcom BCM63xx DSL SoC"
>> -    depends on ARCH_MULTI_V7
>> -    select ARCH_HAS_RESET_CONTROLLER
>> -    select ARM_ERRATA_754322
>> -    select ARM_ERRATA_764369 if SMP
>> -    select ARM_GIC
>> -    select ARM_GLOBAL_TIMER
>> -    select CACHE_L2X0
>> -    select HAVE_ARM_ARCH_TIMER
>> -    select HAVE_ARM_TWD if SMP
>> -    select HAVE_ARM_SCU if SMP
>> -    help
>> -      This enables support for systems based on Broadcom DSL SoCs.
>> -      It currently supports the 'BCM63XX' ARM-based family, which
>> includes
>> -      the BCM63138 variant.
>
> Oh, wait a second. This is actually going to totally break builds for
> people because there are a lot of Kconfig-s that depend on it!
>
> I don't believe it can't be handled this way.
>
> You need to first introduce a replacement / parent symbol.
> Then (in another release if needed) update all Kconfig-s.
> Then eventually you can get rid of ARCH_BCM_63XX.

This works because ARCH_BCMBCA was introdcuced in v5.19-rc1 with
b32c613b3fda3 and in patch 8 the defconfig is updated to select ARCH_BCMBCA.
--
Florian

2022-07-13 16:53:39

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] arm: bcmbca: Move BCM63138 ARCH_BCM_63XX to ARCH_BCMBCA

On 7/13/22 08:31, Rafał Miłecki wrote:
> On 7.07.2022 09:00, William Zhang wrote:
>> Remove ARCH_BCM_63XX Kconfig for BCM63138 and merge its selections to
>> ARCH_BCMBCA. Delete bcm63xx.c as it is no longer needed.
>>
>> Signed-off-by: William Zhang <[email protected]>
>
> What about ARCH_BCM_63XX references in
> 1. arch/arm/Kconfig.debug

That one was dealt with:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=fa0ef5a086a05306fc5322bbda73459725eda923


> 2. arch/arm/configs/multi_v7_defconfig

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a65ee523a9e41d0bf046cf0107e6bbf05d068af2

patch 8/8 of this series

>
> Did you handle them in some other patchset?
>
>
> This change is probably going to break "make oldconfig" for users. Is
> there any kernel policy for that? Do we care about it? Should we leave
> old symbol as hidden and make ARCH_BCMBCA auto-selected if it's set?

I do not believe anyone but me was enabling CONFIG_ARCH_BCM_63XX so this
did not have any users until the BCA team decided to take over.

>
>
> Finally it'd probably be more clean to first introduce
> ARCH_BCMBCA_CORTEXA9 and then get rid of ARCH_BCM_63XX.

That is fair, however a bit late now that the pull requests have been
accepted by the soc maintainers.

Thanks!
--
Florian