From: Rafał Miłecki <[email protected]>
Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
documentation change regarding "depends" usage. Adjust Kconfig entry to
match current guidelines. Make this symbol available for relevant
architectures only.
Cc: Geert Uytterhoeven <[email protected]>
Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/tty/serial/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index e952ec5c7a7c..99313e16c2be 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
config SERIAL_BCM63XX
tristate "Broadcom BCM63xx/BCM33xx UART support"
select SERIAL_CORE
- depends on COMMON_CLK
+ depends on MIPS || ARM || ARM64 || COMPILE_TEST
+ default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
help
This enables the driver for the onchip UART core found on
the following chipsets:
--
2.34.1
On 10.03.2022 09:19, Geert Uytterhoeven wrote:
> On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <[email protected]> wrote:
>> From: Rafał Miłecki <[email protected]>
>>
>> Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
>> documentation change regarding "depends" usage. Adjust Kconfig entry to
>> match current guidelines. Make this symbol available for relevant
>> architectures only.
>>
>> Cc: Geert Uytterhoeven <[email protected]>
>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
>> Signed-off-by: Rafał Miłecki <[email protected]>
>
> Thanks for your patch!
>
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
>> config SERIAL_BCM63XX
>> tristate "Broadcom BCM63xx/BCM33xx UART support"
>> select SERIAL_CORE
>> - depends on COMMON_CLK
>> + depends on MIPS || ARM || ARM64 || COMPILE_TEST
>> + default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
>
> So ARCH_BCM4908 covers ARM64, and BCM63XX || BMIPS_GENERIC
> cover MIPS. Is there some symbol covering ARM so we can change the
> depends to
>
> depends on FOO || ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC || COMPILE_TEST
>
> ?
Florian, Kevin: do you know what other platforms need that driver?
Ref: c0ec3fd123e9 ("tty: serial: bcm63xx: Allow bcm63xx_uart to be built on other platforms")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c0ec3fd123e9e64e095fb221ace841e00c04e40b
Hi Rafał,
On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <[email protected]> wrote:
> From: Rafał Miłecki <[email protected]>
>
> Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
> documentation change regarding "depends" usage. Adjust Kconfig entry to
> match current guidelines. Make this symbol available for relevant
> architectures only.
>
> Cc: Geert Uytterhoeven <[email protected]>
> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
> Signed-off-by: Rafał Miłecki <[email protected]>
Thanks for your patch!
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
> config SERIAL_BCM63XX
> tristate "Broadcom BCM63xx/BCM33xx UART support"
> select SERIAL_CORE
> - depends on COMMON_CLK
> + depends on MIPS || ARM || ARM64 || COMPILE_TEST
> + default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
So ARCH_BCM4908 covers ARM64, and BCM63XX || BMIPS_GENERIC
cover MIPS. Is there some symbol covering ARM so we can change the
depends to
depends on FOO || ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC || COMPILE_TEST
?
Anyway, this is definitely a step in the good direction!
> help
> This enables the driver for the onchip UART core found on
> the following chipsets:
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 3/10/22 12:33 AM, Rafał Miłecki wrote:
> On 10.03.2022 09:19, Geert Uytterhoeven wrote:
>> On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <[email protected]> wrote:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
>>> documentation change regarding "depends" usage. Adjust Kconfig entry to
>>> match current guidelines. Make this symbol available for relevant
>>> architectures only.
>>>
>>> Cc: Geert Uytterhoeven <[email protected]>
>>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
>>> dependency practises")
>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>
>> Thanks for your patch!
>>
>>> --- a/drivers/tty/serial/Kconfig
>>> +++ b/drivers/tty/serial/Kconfig
>>> @@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
>>> config SERIAL_BCM63XX
>>> tristate "Broadcom BCM63xx/BCM33xx UART support"
>>> select SERIAL_CORE
>>> - depends on COMMON_CLK
>>> + depends on MIPS || ARM || ARM64 || COMPILE_TEST
>>> + default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
>>
>> So ARCH_BCM4908 covers ARM64, and BCM63XX || BMIPS_GENERIC
>> cover MIPS. Is there some symbol covering ARM so we can change the
>> depends to
>>
>> depends on FOO || ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC ||
>> COMPILE_TEST
>>
>> ?
>
> Florian, Kevin: do you know what other platforms need that driver?
Yes that would be ARCH_BCM_63XX.
--
Florian
On 3/11/22 1:13 AM, Geert Uytterhoeven wrote:
> Hi Florian,
>
> On Thu, Mar 10, 2022 at 10:49 PM Florian Fainelli <[email protected]> wrote:
>> On 3/10/22 12:33 AM, Rafał Miłecki wrote:
>>> On 10.03.2022 09:19, Geert Uytterhoeven wrote:
>>>> On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <[email protected]> wrote:
>>>>> From: Rafał Miłecki <[email protected]>
>>>>>
>>>>> Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
>>>>> documentation change regarding "depends" usage. Adjust Kconfig entry to
>>>>> match current guidelines. Make this symbol available for relevant
>>>>> architectures only.
>>>>>
>>>>> Cc: Geert Uytterhoeven <[email protected]>
>>>>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>>>>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
>>>>> dependency practises")
>>>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>>>
>>>> Thanks for your patch!
>>>>
>>>>> --- a/drivers/tty/serial/Kconfig
>>>>> +++ b/drivers/tty/serial/Kconfig
>>>>> @@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
>>>>> config SERIAL_BCM63XX
>>>>> tristate "Broadcom BCM63xx/BCM33xx UART support"
>>>>> select SERIAL_CORE
>>>>> - depends on COMMON_CLK
>>>>> + depends on MIPS || ARM || ARM64 || COMPILE_TEST
>>>>> + default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
>>>>
>>>> So ARCH_BCM4908 covers ARM64, and BCM63XX || BMIPS_GENERIC
>>>> cover MIPS. Is there some symbol covering ARM so we can change the
>>>> depends to
>>>>
>>>> depends on FOO || ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC ||
>>>> COMPILE_TEST
>>>>
>>>> ?
>>>
>>> Florian, Kevin: do you know what other platforms need that driver?
>>
>> Yes that would be ARCH_BCM_63XX.
>
> Had to read this twice, before I realized this is different from BCM63XX.
>
> As we already have
>
> drivers/char/hw_random/Kconfig: ARCH_BCM_63XX || BCM63XX ||
> BMIPS_GENERIC || COMPILE_TEST
> drivers/spi/Kconfig: depends on BCM63XX || BMIPS_GENERIC ||
> ARCH_BCM_63XX || COMPILE_TEST
>
> and drivers/tty/serial/Kconfig would be a third case, would it make
> sense to unify BCM63XX and ARCH_BCM_63XX into a single symbol?
> We already share several ARCH_* symbols between arm and arm64,
> and I expect more will be shared with riscv in the future
> (e.g. ARCH_RENESAS).
In hindsight, I could have used BCM63XX, however back then a number of
drivers that depended on BCM63XX were either not applicable, or not able
to build with an ARM-based kernel. For consistency with the other
platforms defined in arch/arm/mach-bcm/Kconfig, I went with
ARCH_BCM6_63XX to denote the difference.
>
> Or are MIPS BCM63xx and ARM BCM63xx too dissimilar?
The chips are an incremental update in a number of ways and did take a
number of the "legacy" cores present in previous chips in the UBUS bus
bridge, such as UART, GPIO, timer, watchdog, USB device controller, but
also added new ones in the AXI bus.
Changing the symbol names could be done, but it feels like it could be
some unnecessary churn now, no?
--
Florian
Hi Florian,
On Thu, Mar 10, 2022 at 10:49 PM Florian Fainelli <[email protected]> wrote:
> On 3/10/22 12:33 AM, Rafał Miłecki wrote:
> > On 10.03.2022 09:19, Geert Uytterhoeven wrote:
> >> On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <[email protected]> wrote:
> >>> From: Rafał Miłecki <[email protected]>
> >>>
> >>> Patches lowering SERIAL_BCM63XX dependencies led to a discussion and
> >>> documentation change regarding "depends" usage. Adjust Kconfig entry to
> >>> match current guidelines. Make this symbol available for relevant
> >>> architectures only.
> >>>
> >>> Cc: Geert Uytterhoeven <[email protected]>
> >>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
> >>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
> >>> dependency practises")
> >>> Signed-off-by: Rafał Miłecki <[email protected]>
> >>
> >> Thanks for your patch!
> >>
> >>> --- a/drivers/tty/serial/Kconfig
> >>> +++ b/drivers/tty/serial/Kconfig
> >>> @@ -1100,7 +1100,8 @@ config SERIAL_TIMBERDALE
> >>> config SERIAL_BCM63XX
> >>> tristate "Broadcom BCM63xx/BCM33xx UART support"
> >>> select SERIAL_CORE
> >>> - depends on COMMON_CLK
> >>> + depends on MIPS || ARM || ARM64 || COMPILE_TEST
> >>> + default ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC
> >>
> >> So ARCH_BCM4908 covers ARM64, and BCM63XX || BMIPS_GENERIC
> >> cover MIPS. Is there some symbol covering ARM so we can change the
> >> depends to
> >>
> >> depends on FOO || ARCH_BCM4908 || BCM63XX || BMIPS_GENERIC ||
> >> COMPILE_TEST
> >>
> >> ?
> >
> > Florian, Kevin: do you know what other platforms need that driver?
>
> Yes that would be ARCH_BCM_63XX.
Had to read this twice, before I realized this is different from BCM63XX.
As we already have
drivers/char/hw_random/Kconfig: ARCH_BCM_63XX || BCM63XX ||
BMIPS_GENERIC || COMPILE_TEST
drivers/spi/Kconfig: depends on BCM63XX || BMIPS_GENERIC ||
ARCH_BCM_63XX || COMPILE_TEST
and drivers/tty/serial/Kconfig would be a third case, would it make
sense to unify BCM63XX and ARCH_BCM_63XX into a single symbol?
We already share several ARCH_* symbols between arm and arm64,
and I expect more will be shared with riscv in the future
(e.g. ARCH_RENESAS).
Or are MIPS BCM63xx and ARM BCM63xx too dissimilar?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds