linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
@ 2022-03-10  7:22 Rafał Miłecki
  2022-03-10  8:19 ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2022-03-10  7:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-serial, Florian Fainelli, linux-arm-kernel,
	linux-mips, bcm-kernel-feedback-list, linux-kernel,
	Rafał Miłecki, Geert Uytterhoeven

From: Rafał Miłecki <rafal@milecki.pl>

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 <geert@linux-m68k.org>
Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
  2022-03-10  7:22 [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol Rafał Miłecki
@ 2022-03-10  8:19 ` Geert Uytterhoeven
  2022-03-10  8:33   ` Rafał Miłecki
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2022-03-10  8:19 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
	Florian Fainelli, Linux ARM, open list:BROADCOM NVRAM DRIVER,
	bcm-kernel-feedback-list, Linux Kernel Mailing List,
	Rafał Miłecki

Hi Rafał,

On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> 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 <geert@linux-m68k.org>
> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
  2022-03-10  8:19 ` Geert Uytterhoeven
@ 2022-03-10  8:33   ` Rafał Miłecki
  2022-03-10 21:49     ` Florian Fainelli
  0 siblings, 1 reply; 6+ messages in thread
From: Rafał Miłecki @ 2022-03-10  8:33 UTC (permalink / raw)
  To: Geert Uytterhoeven, Kevin Cernekee, Florian Fainelli
  Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
	Linux ARM, open list:BROADCOM NVRAM DRIVER,
	bcm-kernel-feedback-list, Linux Kernel Mailing List,
	Rafał Miłecki

On 10.03.2022 09:19, Geert Uytterhoeven wrote:
> On Thu, Mar 10, 2022 at 8:22 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> 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 <geert@linux-m68k.org>
>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform dependency practises")
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> 
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
  2022-03-10  8:33   ` Rafał Miłecki
@ 2022-03-10 21:49     ` Florian Fainelli
  2022-03-11  9:13       ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2022-03-10 21:49 UTC (permalink / raw)
  To: Rafał Miłecki, Geert Uytterhoeven, Kevin Cernekee
  Cc: Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
	Linux ARM, open list:BROADCOM NVRAM DRIVER,
	bcm-kernel-feedback-list, Linux Kernel Mailing List,
	Rafał Miłecki

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 <zajec5@gmail.com> wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> 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 <geert@linux-m68k.org>
>>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
>>> dependency practises")
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>
>> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
  2022-03-10 21:49     ` Florian Fainelli
@ 2022-03-11  9:13       ` Geert Uytterhoeven
  2022-03-11 20:38         ` Florian Fainelli
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2022-03-11  9:13 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Kevin Cernekee, Greg Kroah-Hartman,
	Jiri Slaby, open list:SERIAL DRIVERS, Linux ARM,
	open list:BROADCOM NVRAM DRIVER, bcm-kernel-feedback-list,
	Linux Kernel Mailing List, Rafał Miłecki,
	Arnd Bergmann

Hi Florian,

On Thu, Mar 10, 2022 at 10:49 PM Florian Fainelli <f.fainelli@gmail.com> 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 <zajec5@gmail.com> wrote:
> >>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>
> >>> 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 <geert@linux-m68k.org>
> >>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
> >>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
> >>> dependency practises")
> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> >>
> >> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol
  2022-03-11  9:13       ` Geert Uytterhoeven
@ 2022-03-11 20:38         ` Florian Fainelli
  0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2022-03-11 20:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Rafał Miłecki, Kevin Cernekee, Greg Kroah-Hartman,
	Jiri Slaby, open list:SERIAL DRIVERS, Linux ARM,
	open list:BROADCOM NVRAM DRIVER, bcm-kernel-feedback-list,
	Linux Kernel Mailing List, Rafał Miłecki,
	Arnd Bergmann

On 3/11/22 1:13 AM, Geert Uytterhoeven wrote:
> Hi Florian,
> 
> On Thu, Mar 10, 2022 at 10:49 PM Florian Fainelli <f.fainelli@gmail.com> 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 <zajec5@gmail.com> wrote:
>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>
>>>>> 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 <geert@linux-m68k.org>
>>>>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>>>>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
>>>>> dependency practises")
>>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-03-11 20:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10  7:22 [PATCH] tty: serial: bcm63xx: use more precise Kconfig symbol Rafał Miłecki
2022-03-10  8:19 ` Geert Uytterhoeven
2022-03-10  8:33   ` Rafał Miłecki
2022-03-10 21:49     ` Florian Fainelli
2022-03-11  9:13       ` Geert Uytterhoeven
2022-03-11 20:38         ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).