lkml.org 
[lkml]   [2022]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 2/2] serial: 8250/ingenic: Add support for the JZ4750/JZ4755
    сб, 22 окт. 2022 г. в 23:07, Paul Cercueil <paul@crapouillou.net>:
    >
    > 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;
    > > -
    > > 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 don't understand, didn't we came up to the conclusion in your V1 that
    > it was better to force-enable the EXT/2 divider in the ingenic init
    > code?
    >
    > -Paul
    >

    That was better at that moment. I dived deeper in the situation and found
    a more simple and universal solution, as I think.
    Your proposal doesn't cover following situations:
    - JZ475x with 12MHz crystal
    - JZ4760 with 24MHz crystal
    which are legal and might appear in some hardware.

    > > +
    > > + 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-23 07:31    [W:4.015 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site