2023-12-01 17:16:23

by Francesco Dolcini

[permalink] [raw]
Subject: [PATCH v2] arm64: defconfig: increase SERIAL_8250_NR_UARTS

Increase CONFIG_SERIAL_8250_NR_UARTS from 4 to 8, the current legacy value
is not adequate for embedded systems that use SoCs where it's common to
have a large number of serial ports.

No need to change CONFIG_SERIAL_8250_RUNTIME_UARTS, see commit 9d86719f8769
("serial: 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS").

The need to increase this value was noticed while working with Toradex
Verdin AM62, this board has 4 serial UART instances available to the user
plus an internal one that is connected to a Bluetooth module. Without this
change the fifth UART connected to the BT module is not instantiated and BT
is not working.

Instead of increasing the number to the bare minimum (5) that would be
required to solve this specific issue, we increase this to 8 which seems a
more reasonable number to have in the defconfig and should cover more valid
use cases.

Cc: Tony Lindgren <[email protected]>
Reviewed-by: Tony Lindgren <[email protected]>
Signed-off-by: Francesco Dolcini <[email protected]>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b60aa1f89343..ecd365cd1d87 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -448,6 +448,7 @@ CONFIG_SERIO_AMBAKMI=y
CONFIG_LEGACY_PTY_COUNT=16
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_NR_UARTS=8
CONFIG_SERIAL_8250_EXTENDED=y
CONFIG_SERIAL_8250_SHARE_IRQ=y
CONFIG_SERIAL_8250_BCM2835AUX=y
--
2.39.2


2023-12-01 19:21:16

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: defconfig: increase SERIAL_8250_NR_UARTS

On 18:15-20231201, Francesco Dolcini wrote:
> Increase CONFIG_SERIAL_8250_NR_UARTS from 4 to 8, the current legacy value
> is not adequate for embedded systems that use SoCs where it's common to
> have a large number of serial ports.
>
> No need to change CONFIG_SERIAL_8250_RUNTIME_UARTS, see commit 9d86719f8769
> ("serial: 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS").
>
> The need to increase this value was noticed while working with Toradex
> Verdin AM62, this board has 4 serial UART instances available to the user
> plus an internal one that is connected to a Bluetooth module. Without this
> change the fifth UART connected to the BT module is not instantiated and BT
> is not working.
>
> Instead of increasing the number to the bare minimum (5) that would be
> required to solve this specific issue, we increase this to 8 which seems a
> more reasonable number to have in the defconfig and should cover more valid
> use cases.

To address Arnd's concern on size increase, it will be good to add:
---
With this change kernel image increases by ~3.2k. bloat-o-meter summary:
add/remove: 1/1 grow/shrink: 7/0 up/down: 3220/-8 (3212)
---

Full bloat-o-meter report: aarch64-none-linux-gnu-gcc (11.3.1 20220712)
$ ./scripts/bloat-o-meter ./vmlinux.old vmlinux
add/remove: 1/1 grow/shrink: 7/0 up/down: 3220/-8 (3212)
Function old new delta
serial8250_ports 3136 6272 +3136
serial8250_register_8250_port 1176 1208 +32
serial8250_remove 120 136 +16
serial8250_unregister_port 288 296 +8
serial8250_suspend 120 128 +8
serial8250_setup_port 212 220 +8
e843419@0d28_000102dd_3a64 - 8 +8
serial8250_init 468 472 +4
e843419@0c0a_0000eb9e_8cc 8 - -8
Total: Before=28756614, After=28759826, chg +0.01%

>
> Cc: Tony Lindgren <[email protected]>
> Reviewed-by: Tony Lindgren <[email protected]>
> Signed-off-by: Francesco Dolcini <[email protected]>
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index b60aa1f89343..ecd365cd1d87 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -448,6 +448,7 @@ CONFIG_SERIO_AMBAKMI=y
> CONFIG_LEGACY_PTY_COUNT=16
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> +CONFIG_SERIAL_8250_NR_UARTS=8
> CONFIG_SERIAL_8250_EXTENDED=y
> CONFIG_SERIAL_8250_SHARE_IRQ=y
> CONFIG_SERIAL_8250_BCM2835AUX=y
> --
> 2.39.2
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-12-01 19:53:25

by Francesco Dolcini

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: defconfig: increase SERIAL_8250_NR_UARTS

On Fri, Dec 01, 2023 at 01:19:58PM -0600, Nishanth Menon wrote:
> On 18:15-20231201, Francesco Dolcini wrote:
> > Increase CONFIG_SERIAL_8250_NR_UARTS from 4 to 8, the current legacy value
> > is not adequate for embedded systems that use SoCs where it's common to
> > have a large number of serial ports.
> >
> > No need to change CONFIG_SERIAL_8250_RUNTIME_UARTS, see commit 9d86719f8769
> > ("serial: 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS").
> >
> > The need to increase this value was noticed while working with Toradex
> > Verdin AM62, this board has 4 serial UART instances available to the user
> > plus an internal one that is connected to a Bluetooth module. Without this
> > change the fifth UART connected to the BT module is not instantiated and BT
> > is not working.
> >
> > Instead of increasing the number to the bare minimum (5) that would be
> > required to solve this specific issue, we increase this to 8 which seems a
> > more reasonable number to have in the defconfig and should cover more valid
> > use cases.
>
> To address Arnd's concern on size increase, it will be good to add:

I can and I will add it in a v3 - it takes less time to do it than reply to
this email and thanks for taking the time to provide the actual data.

With that said my understanding is that the goal of the arm64 defconfig is
to enable the supported arm64 hardware and the related kernel development.
It's not supposed to be a minimal config in size nor an optimal
configuration from the performance point of view. It's a single
configuration that includes support for each and every platform [1]
supported by Linux arm64 ...

To me Arnd was just stating a fact, not raising a concern that was supposed
to be addressed (just correct me if I'm wrong, of course).

[1] well, apart AMD Pensando and Bitmain, at the moment, but you get the
point, I'm sure ;-).

> ---
> With this change kernel image increases by ~3.2k. bloat-o-meter summary:
> add/remove: 1/1 grow/shrink: 7/0 up/down: 3220/-8 (3212)
> ---

Francesco

2023-12-01 23:27:06

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: defconfig: increase SERIAL_8250_NR_UARTS

On 20:52-20231201, Francesco Dolcini wrote:
> On Fri, Dec 01, 2023 at 01:19:58PM -0600, Nishanth Menon wrote:
> > On 18:15-20231201, Francesco Dolcini wrote:
> > > Increase CONFIG_SERIAL_8250_NR_UARTS from 4 to 8, the current legacy value
> > > is not adequate for embedded systems that use SoCs where it's common to
> > > have a large number of serial ports.
> > >
> > > No need to change CONFIG_SERIAL_8250_RUNTIME_UARTS, see commit 9d86719f8769
> > > ("serial: 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS").
> > >
> > > The need to increase this value was noticed while working with Toradex
> > > Verdin AM62, this board has 4 serial UART instances available to the user
> > > plus an internal one that is connected to a Bluetooth module. Without this
> > > change the fifth UART connected to the BT module is not instantiated and BT
> > > is not working.
> > >
> > > Instead of increasing the number to the bare minimum (5) that would be
> > > required to solve this specific issue, we increase this to 8 which seems a
> > > more reasonable number to have in the defconfig and should cover more valid
> > > use cases.
> >
> > To address Arnd's concern on size increase, it will be good to add:
>
> I can and I will add it in a v3 - it takes less time to do it than reply to
> this email and thanks for taking the time to provide the actual data.
>
> With that said my understanding is that the goal of the arm64 defconfig is
> to enable the supported arm64 hardware and the related kernel development.
> It's not supposed to be a minimal config in size nor an optimal
> configuration from the performance point of view. It's a single
> configuration that includes support for each and every platform [1]
> supported by Linux arm64 ...
>
> To me Arnd was just stating a fact, not raising a concern that was supposed
> to be addressed (just correct me if I'm wrong, of course).
>
> [1] well, apart AMD Pensando and Bitmain, at the moment, but you get the
> point, I'm sure ;-).
>

Hehe, thanks Francesco.

Arnd, my memory was a bit right on the topic though.. .. I think we
had gone down a similar road before in trying to increase the number
of 8250 UARTs [1] but without this strong a reason.. Been a while, I
think our v3 is much stronger case now.

[1] https://lore.kernel.org/all/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/#t
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-12-05 08:31:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: defconfig: increase SERIAL_8250_NR_UARTS

On Sat, Dec 2, 2023, at 00:26, Nishanth Menon wrote:
> On 20:52-20231201, Francesco Dolcini wrote:
>> On Fri, Dec 01, 2023 at 01:19:58PM -0600, Nishanth Menon wrote:
>> > On 18:15-20231201, Francesco Dolcini wrote:
>> To me Arnd was just stating a fact, not raising a concern that was supposed
>> to be addressed (just correct me if I'm wrong, of course).
>>
>> [1] well, apart AMD Pensando and Bitmain, at the moment, but you get the
>> point, I'm sure ;-).
>>
>
> Hehe, thanks Francesco.
>
> Arnd, my memory was a bit right on the topic though.. .. I think we
> had gone down a similar road before in trying to increase the number
> of 8250 UARTs [1] but without this strong a reason.. Been a while, I
> think our v3 is much stronger case now.
>
> [1]
> https://lore.kernel.org/all/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/#t

I think in that case the problem was that the aliases were
done in the .dtsi file but enabled in the board specific .dts,
which would be pointless and confusing.

If we have a board that actually wires up more uarts to connectors
or devices, can clearly raise the compile time limit to whatever
that needs.

Arnd