[PATCH v2 0/2] serial: 8250/ingenic: Add support for the JZ4750

[PATCH v2 0/2] serial: 8250/ingenic: Add support for the JZ4750

[Siarhei Volkau]

JZ4750 and JZ4755 have an extra clock divisor in CGU called CPCCR.ECS.
It needs to be handled properly in the early console driver.

V1 diff:
 - splitted into separate patchset
 - code refactored to avoid peek in CGU register
 - Krzysztof's ack picked

Siarhei Volkau (2):
  dt-bindings: serial: ingenic: Add support for the JZ4750/55 SoCs
  serial: 8250/ingenic: Add support for the JZ4750/JZ4755

 .../bindings/serial/ingenic,uart.yaml         |  4 ++
 drivers/tty/serial/8250/8250_ingenic.c        | 50 ++++++++++++++++---
 2 files changed, 47 insertions(+), 7 deletions(-)

-- 
2.36.1

[PATCH v2 1/2] dt-bindings: serial: ingenic: Add support for the JZ4750/55 SoCs

[Siarhei Volkau]

These SoCs UART block are the same as JZ4725b' one, the difference is
outside of the block - it is in the clock generation unit (CGU).
The difference requires to make a quirk for early console init.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
---
 Documentation/devicetree/bindings/serial/ingenic,uart.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
index 9ca7a18ec..315ceb722 100644
--- a/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
+++ b/Documentation/devicetree/bindings/serial/ingenic,uart.yaml
@@ -20,6 +20,7 @@ properties:
     oneOf:
       - enum:
           - ingenic,jz4740-uart
+          - ingenic,jz4750-uart
           - ingenic,jz4760-uart
           - ingenic,jz4780-uart
           - ingenic,x1000-uart
@@ -31,6 +32,9 @@ properties:
       - items:
           - const: ingenic,jz4725b-uart
           - const: ingenic,jz4740-uart
+      - items:
+          - const: ingenic,jz4755-uart
+          - const: ingenic,jz4750-uart
 
   reg:
     maxItems: 1
-- 
2.36.1

[PATCH v2 2/2] serial: 8250/ingenic: Add support for the JZ4750/JZ4755

[Siarhei Volkau]

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 | 50 ++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 2b2f5d8d2..3ffa6b722 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,
-					      const char *opt)
+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;
-
 	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);
+
+	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;
+
+	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

Re: [PATCH v2 2/2] serial: 8250/ingenic: Add support for the JZ4750/JZ4755

[Paul Cercueil]

Hi Siarhei,

Le sam. 22 oct. 2022 à 18:12:24 +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 | 50 
> ++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_ingenic.c 
> b/drivers/tty/serial/8250/8250_ingenic.c
> index 2b2f5d8d2..3ffa6b722 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,
> -					      const char *opt)
> +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;

You can keep this here - no need to move it (and it'd avoid duplicating 
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);
> +
> +	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 would assume you could just do:
dev->port.uartclk = 12000000;

Since you'd always get a 12 MHz clock (either with a 12 MHz oscillator 
or a 24 MHz oscillator with a /2 divider).

With that said - I am fine with that code, it would allow to fine-tune 
the oscillator value (although I hightly doubt anybody is going to do 
that).

The 16 MHz value sounds very arbitrary, but I'll give it a pass.

Cheers,
-Paul

> +
> +	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
>