lkml.org 
[lkml]   [2022]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 2/2] serial: 8250/ingenic: Add support for the JZ4750/JZ4755
Hi Siarhei,

Le sam. 22 oct. 2022 à 19:50:47 +0300, Siarhei Volkau
<lis8215@gmail.com> a écrit :
> JZ4750/55/60 (but not JZ4760b) have an extra divisor in between extclk
> and peripheral clock, called CPCCR.ECS, the driver can't figure out
> the
> real state of the divisor without dirty hack - peek CGU CPCCR
> register.
> However, we can rely on a vendor's bootloader (u-boot 1.1.6) behavior:
> if (extclk > 16MHz)
> the divisor is enabled, so the UART driving clock is extclk/2.
>
> This behavior relies on hardware differences: most boards (if not all)
> with those SoCs have 12 or 24 MHz oscillators but many peripherals
> want
> 12Mhz to operate properly (AIC and USB-PHY at least).
>
> The patch doesn't affect JZ4760's behavior as it is subject for
> another
> patchset with re-classification of all supported ingenic UARTs.
>
> Link:
> https://github.com/carlos-wong/uboot_jz4755/blob/master/cpu/mips/jz_serial.c#L158
> Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> ---
> drivers/tty/serial/8250/8250_ingenic.c | 48
> ++++++++++++++++++++++----
> 1 file changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_ingenic.c
> b/drivers/tty/serial/8250/8250_ingenic.c
> index 2b2f5d8d2..744705467 100644
> --- a/drivers/tty/serial/8250/8250_ingenic.c
> +++ b/drivers/tty/serial/8250/8250_ingenic.c
> @@ -87,24 +87,19 @@ static void __init
> ingenic_early_console_setup_clock(struct earlycon_device *dev
> dev->port.uartclk = be32_to_cpup(prop);
> }
>
> -static int __init ingenic_early_console_setup(struct earlycon_device
> *dev,
> +static int __init ingenic_earlycon_setup_tail(struct earlycon_device
> *dev,
> const char *opt)
> {
> struct uart_port *port = &dev->port;
> unsigned int divisor;
> int baud = 115200;
>
> - if (!dev->port.membase)
> - return -ENODEV;

Again, as I said on your v2, you can keep this here. Then you won't
have to duplicate code.

> -
> if (opt) {
> unsigned int parity, bits, flow; /* unused for now */
>
> uart_parse_options(opt, &baud, &parity, &bits, &flow);
> }
>
> - ingenic_early_console_setup_clock(dev);
> -
> if (dev->baud)
> baud = dev->baud;
> divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * baud);
> @@ -129,9 +124,49 @@ static int __init
> ingenic_early_console_setup(struct earlycon_device *dev,
> return 0;
> }
>
> +static int __init ingenic_early_console_setup(struct earlycon_device
> *dev,
> + const char *opt)
> +{
> + if (!dev->port.membase)
> + return -ENODEV;
> +
> + ingenic_early_console_setup_clock(dev);
> +
> + return ingenic_earlycon_setup_tail(dev, opt);
> +}
> +
> +static int __init jz4750_early_console_setup(struct earlycon_device
> *dev,
> + const char *opt)
> +{
> + if (!dev->port.membase)
> + return -ENODEV;
> +
> + /*
> + * JZ4750/55/60 (not JZ4760b) have an extra divisor
> + * between extclk and peripheral clock, the
> + * driver can't figure out the real state of the
> + * divisor without dirty hacks (peek CGU register).
> + * However, we can rely on a vendor's behavior:
> + * if (extclk > 16MHz)
> + * the divisor is enabled.
> + * This behavior relies on hardware differences:
> + * most boards with those SoCs have 12 or 24 MHz
> + * oscillators but many peripherals want 12Mhz
> + * to operate properly (AIC and USB-phy at least).
> + */
> + ingenic_early_console_setup_clock(dev);
> + if (dev->port.uartclk > 16000000)
> + dev->port.uartclk /= 2;

I'm OK with this code, but the comment is not very clear.

What about:

"JZ4750/55/60 have an optional /2 divider between the EXT oscillator
and some peripherals including UART, which will be enabled if using a
24 MHz oscillator, and disabled when using a 12 MHz oscillator."

Cheers,
-Paul

> +
> + return ingenic_earlycon_setup_tail(dev, opt);
> +}
> +
> OF_EARLYCON_DECLARE(jz4740_uart, "ingenic,jz4740-uart",
> ingenic_early_console_setup);
>
> +OF_EARLYCON_DECLARE(jz4750_uart, "ingenic,jz4750-uart",
> + jz4750_early_console_setup);
> +
> OF_EARLYCON_DECLARE(jz4770_uart, "ingenic,jz4770-uart",
> ingenic_early_console_setup);
>
> @@ -328,6 +363,7 @@ static const struct ingenic_uart_config
> x1000_uart_config = {
>
> static const struct of_device_id of_match[] = {
> { .compatible = "ingenic,jz4740-uart", .data = &jz4740_uart_config
> },
> + { .compatible = "ingenic,jz4750-uart", .data = &jz4760_uart_config
> },
> { .compatible = "ingenic,jz4760-uart", .data = &jz4760_uart_config
> },
> { .compatible = "ingenic,jz4770-uart", .data = &jz4760_uart_config
> },
> { .compatible = "ingenic,jz4775-uart", .data = &jz4760_uart_config
> },
> --
> 2.36.1
>


\
 
 \ /
  Last update: 2022-10-25 01:11    [W:0.075 / U:0.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site