2022-11-24 12:42:08

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH v3 0/2] earlycon: Let users set the clock frequency

Some platforms, namely AMD Picasso, use non standard uart clocks (48M),
witch makes it impossible to use with earlycon.

Let the user select its own frequency.

To: Jonathan Corbet <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Jiri Slaby <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Ricardo Ribalda <[email protected]>

---
Changes in v3:
- Revert patch to use kstrtouint for baudrate
- Parse the number not the ,number (Thanks Jiri :) )
- Add optional patch to increase the options size
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Add a patch to fix handling of baudrate
- Use kstrtouint instead of simple_strtoul
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Ricardo Ribalda (2):
earlycon: Let users set the clock frequency
earlycon: Increase options size

Documentation/admin-guide/kernel-parameters.txt | 12 +++++++-----
drivers/tty/serial/earlycon.c | 9 ++++++++-
include/linux/serial_core.h | 2 +-
3 files changed, 16 insertions(+), 7 deletions(-)
---
base-commit: 4312098baf37ee17a8350725e6e0d0e8590252d4
change-id: 20221123-serial-clk-85db701ada57

Best regards,
--
Ricardo Ribalda <[email protected]>


2022-11-24 12:42:54

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH v3 1/2] earlycon: Let users set the clock frequency

Some platforms, namely AMD Picasso, use non standard uart clocks (48M),
witch makes it impossible to use with earlycon.

Let the user select its own frequency.

Signed-off-by: Ricardo Ribalda <[email protected]>

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a465d5242774..9efb6c3b0486 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1182,10 +1182,10 @@
specified, the serial port must already be setup and
configured.

- uart[8250],io,<addr>[,options]
- uart[8250],mmio,<addr>[,options]
- uart[8250],mmio32,<addr>[,options]
- uart[8250],mmio32be,<addr>[,options]
+ uart[8250],io,<addr>[,options[,uartclk]]
+ uart[8250],mmio,<addr>[,options[,uartclk]]
+ uart[8250],mmio32,<addr>[,options[,uartclk]]
+ uart[8250],mmio32be,<addr>[,options[,uartclk]]
uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
@@ -1194,7 +1194,9 @@
If none of [io|mmio|mmio32|mmio32be], <addr> is assumed
to be equivalent to 'mmio'. 'options' are specified
in the same format described for "console=ttyS<n>"; if
- unspecified, the h/w is not initialized.
+ unspecified, the h/w is not initialized. 'uartclk' is
+ the uart clock frequency; if unspecified, it is set
+ to 'BASE_BAUD' * 16.

pl011,<addr>
pl011,mmio32,<addr>
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a5f380584cda..3a0c88419b6c 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -120,7 +120,13 @@ static int __init parse_options(struct earlycon_device *device, char *options)
}

if (options) {
+ char *uartclk;
+
device->baud = simple_strtoul(options, NULL, 0);
+ uartclk = strchr(options, ',');
+ if (uartclk && kstrtouint(uartclk + 1, 0, &port->uartclk) < 0)
+ pr_warn("[%s] unsupported earlycon uart clkrate option\n",
+ options);
length = min(strcspn(options, " ") + 1,
(size_t)(sizeof(device->options)));
strscpy(device->options, options, length);
@@ -139,7 +145,8 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
buf = NULL;

spin_lock_init(&port->lock);
- port->uartclk = BASE_BAUD * 16;
+ if (!port->uartclk)
+ port->uartclk = BASE_BAUD * 16;
if (port->mapbase)
port->membase = earlycon_map(port->mapbase, 64);


--
b4 0.11.0-dev-d93f8

2022-11-24 12:54:51

by Ricardo Ribalda

[permalink] [raw]
Subject: [PATCH v3 2/2] earlycon: Increase options size

Now that the clock frequency is also part of the options, 16 bytes is
too little.

Without this patch dmesg does not show the whole options, Eg:

earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200n8,480000')

instead of: '115200n8,48000000'

Signed-off-by: Ricardo Ribalda <[email protected]>

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d657f2a42a7b..f555927195da 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -701,7 +701,7 @@ static inline int uart_poll_timeout(struct uart_port *port)
struct earlycon_device {
struct console *con;
struct uart_port port;
- char options[16]; /* e.g., 115200n8 */
+ char options[32]; /* e.g., 115200n8 */
unsigned int baud;
};


--
b4 0.11.0-dev-d93f8

2022-12-02 17:40:43

by Ricardo Ribalda

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] earlycon: Let users set the clock frequency

Hi Jiri

is there something else that I am missing here?

Thanks!

On Thu, 24 Nov 2022 at 13:39, Ricardo Ribalda <[email protected]> wrote:
>
> Some platforms, namely AMD Picasso, use non standard uart clocks (48M),
> witch makes it impossible to use with earlycon.
>
> Let the user select its own frequency.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a465d5242774..9efb6c3b0486 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1182,10 +1182,10 @@
> specified, the serial port must already be setup and
> configured.
>
> - uart[8250],io,<addr>[,options]
> - uart[8250],mmio,<addr>[,options]
> - uart[8250],mmio32,<addr>[,options]
> - uart[8250],mmio32be,<addr>[,options]
> + uart[8250],io,<addr>[,options[,uartclk]]
> + uart[8250],mmio,<addr>[,options[,uartclk]]
> + uart[8250],mmio32,<addr>[,options[,uartclk]]
> + uart[8250],mmio32be,<addr>[,options[,uartclk]]
> uart[8250],0x<addr>[,options]
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> @@ -1194,7 +1194,9 @@
> If none of [io|mmio|mmio32|mmio32be], <addr> is assumed
> to be equivalent to 'mmio'. 'options' are specified
> in the same format described for "console=ttyS<n>"; if
> - unspecified, the h/w is not initialized.
> + unspecified, the h/w is not initialized. 'uartclk' is
> + the uart clock frequency; if unspecified, it is set
> + to 'BASE_BAUD' * 16.
>
> pl011,<addr>
> pl011,mmio32,<addr>
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index a5f380584cda..3a0c88419b6c 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -120,7 +120,13 @@ static int __init parse_options(struct earlycon_device *device, char *options)
> }
>
> if (options) {
> + char *uartclk;
> +
> device->baud = simple_strtoul(options, NULL, 0);
> + uartclk = strchr(options, ',');
> + if (uartclk && kstrtouint(uartclk + 1, 0, &port->uartclk) < 0)
> + pr_warn("[%s] unsupported earlycon uart clkrate option\n",
> + options);
> length = min(strcspn(options, " ") + 1,
> (size_t)(sizeof(device->options)));
> strscpy(device->options, options, length);
> @@ -139,7 +145,8 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
> buf = NULL;
>
> spin_lock_init(&port->lock);
> - port->uartclk = BASE_BAUD * 16;
> + if (!port->uartclk)
> + port->uartclk = BASE_BAUD * 16;
> if (port->mapbase)
> port->membase = earlycon_map(port->mapbase, 64);
>
>
> --
> b4 0.11.0-dev-d93f8



--
Ricardo Ribalda

2022-12-06 07:16:15

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] earlycon: Let users set the clock frequency

On 24. 11. 22, 13:39, Ricardo Ribalda wrote:
> Some platforms, namely AMD Picasso, use non standard uart clocks (48M),
> witch makes it impossible to use with earlycon.
>
> Let the user select its own frequency.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>

Reviewed-by: Jiri Slaby <[email protected]>

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a465d5242774..9efb6c3b0486 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1182,10 +1182,10 @@
> specified, the serial port must already be setup and
> configured.
>
> - uart[8250],io,<addr>[,options]
> - uart[8250],mmio,<addr>[,options]
> - uart[8250],mmio32,<addr>[,options]
> - uart[8250],mmio32be,<addr>[,options]
> + uart[8250],io,<addr>[,options[,uartclk]]
> + uart[8250],mmio,<addr>[,options[,uartclk]]
> + uart[8250],mmio32,<addr>[,options[,uartclk]]
> + uart[8250],mmio32be,<addr>[,options[,uartclk]]
> uart[8250],0x<addr>[,options]
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> @@ -1194,7 +1194,9 @@
> If none of [io|mmio|mmio32|mmio32be], <addr> is assumed
> to be equivalent to 'mmio'. 'options' are specified
> in the same format described for "console=ttyS<n>"; if
> - unspecified, the h/w is not initialized.
> + unspecified, the h/w is not initialized. 'uartclk' is
> + the uart clock frequency; if unspecified, it is set
> + to 'BASE_BAUD' * 16.
>
> pl011,<addr>
> pl011,mmio32,<addr>
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index a5f380584cda..3a0c88419b6c 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -120,7 +120,13 @@ static int __init parse_options(struct earlycon_device *device, char *options)
> }
>
> if (options) {
> + char *uartclk;
> +
> device->baud = simple_strtoul(options, NULL, 0);
> + uartclk = strchr(options, ',');
> + if (uartclk && kstrtouint(uartclk + 1, 0, &port->uartclk) < 0)
> + pr_warn("[%s] unsupported earlycon uart clkrate option\n",
> + options);
> length = min(strcspn(options, " ") + 1,
> (size_t)(sizeof(device->options)));
> strscpy(device->options, options, length);
> @@ -139,7 +145,8 @@ static int __init register_earlycon(char *buf, const struct earlycon_id *match)
> buf = NULL;
>
> spin_lock_init(&port->lock);
> - port->uartclk = BASE_BAUD * 16;
> + if (!port->uartclk)
> + port->uartclk = BASE_BAUD * 16;
> if (port->mapbase)
> port->membase = earlycon_map(port->mapbase, 64);
>
>

--
js
suse labs

2022-12-06 07:36:43

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] earlycon: Increase options size

On 24. 11. 22, 13:39, Ricardo Ribalda wrote:
> Now that the clock frequency is also part of the options, 16 bytes is
> too little.
>
> Without this patch dmesg does not show the whole options, Eg:
>
> earlycon: uart0 at MMIO32 0x00000000fedc9000 (options '115200n8,480000')
>
> instead of: '115200n8,48000000'
>
> Signed-off-by: Ricardo Ribalda <[email protected]>

Reviewed-by: Jiri Slaby <[email protected]>

> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index d657f2a42a7b..f555927195da 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -701,7 +701,7 @@ static inline int uart_poll_timeout(struct uart_port *port)
> struct earlycon_device {
> struct console *con;
> struct uart_port port;
> - char options[16]; /* e.g., 115200n8 */
> + char options[32]; /* e.g., 115200n8 */
> unsigned int baud;
> };
>
>

--
js
suse labs

2022-12-12 14:46:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] earlycon: Let users set the clock frequency

On Fri, Dec 02, 2022 at 06:24:19PM +0100, Ricardo Ribalda wrote:
> Hi Jiri
>
> is there something else that I am missing here?

Sorry, been busy with other reviews. I will queue this up after 6.2-rc1
is out, thanks.

greg k-h