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@xxxxxxxxx> 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@xxxxxxxxx> wrote:
>>>>> From: Rafał Miłecki <rafal@xxxxxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxxxxxx>
>>>>> Ref: f35a07f92616 ("tty: serial: bcm63xx: lower driver dependencies")
>>>>> Ref: 18084e435ff6 ("Documentation/kbuild: Document platform
>>>>> dependency practises")
>>>>> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
>>>>
>>>> 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
