| Message ID | 1455617295-23736-6-git-send-email-vladimir.murzin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 16, 2016 at 12:08 PM, Vladimir Murzin <vladimir.murzin@arm.com> wrote: > This adds support early console for MPS2 UART which can be enabled via > earlycon=mps2,0x40004000 > --- a/drivers/tty/serial/mps2-uart.c > +++ b/drivers/tty/serial/mps2-uart.c > @@ -435,6 +435,36 @@ static struct console mps2_uart_console = { > > #define MPS2_SERIAL_CONSOLE (&mps2_uart_console) > > +static void mps2_early_putchar(struct uart_port *port, int ch) > +{ > + > + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) > + cpu_relax(); Infinite busy loop? > + > + writeb((unsigned char)ch, port->membase + UARTn_DATA); > +} > + > + > +static void mps2_early_write(struct console *con, const char *s, unsigned n) > +{ > + struct earlycon_device *dev = con->data; > + > + uart_console_write(&dev->port, s, n, mps2_early_putchar); > +} > + > +static int __init mps2_early_console_setup(struct earlycon_device *device, > + const char *opt) > +{ > + if (!device->port.membase) > + return -ENODEV; > + > + device->con->write = mps2_early_write; > + > + return 0; > +} > +EARLYCON_DECLARE(mps2, mps2_early_console_setup); > +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ one is enough.
On 16/02/16 10:36, Andy Shevchenko wrote: > On Tue, Feb 16, 2016 at 12:08 PM, Vladimir Murzin > <vladimir.murzin@arm.com> wrote: >> This adds support early console for MPS2 UART which can be enabled via >> earlycon=mps2,0x40004000 > > >> --- a/drivers/tty/serial/mps2-uart.c >> +++ b/drivers/tty/serial/mps2-uart.c >> @@ -435,6 +435,36 @@ static struct console mps2_uart_console = { >> >> #define MPS2_SERIAL_CONSOLE (&mps2_uart_console) >> >> +static void mps2_early_putchar(struct uart_port *port, int ch) >> +{ >> + >> + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) >> + cpu_relax(); > > Infinite busy loop? In case of broken hw, yes, but I'm quite unsure how we can help to fix it. Do you have something in mind? > >> + >> + writeb((unsigned char)ch, port->membase + UARTn_DATA); >> +} >> + >> + >> +static void mps2_early_write(struct console *con, const char *s, unsigned n) >> +{ >> + struct earlycon_device *dev = con->data; >> + >> + uart_console_write(&dev->port, s, n, mps2_early_putchar); >> +} >> + >> +static int __init mps2_early_console_setup(struct earlycon_device *device, >> + const char *opt) >> +{ >> + if (!device->port.membase) >> + return -ENODEV; >> + >> + device->con->write = mps2_early_write; >> + >> + return 0; >> +} >> +EARLYCON_DECLARE(mps2, mps2_early_console_setup); >> +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); > > IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ > one is enough. > I've just tried with OF_ only and seems it works fine. Thanks for pointing at it! Cheers Vladimir
On Tue, Feb 16, 2016 at 3:09 PM, Vladimir Murzin <vladimir.murzin@arm.com> wrote: > On 16/02/16 10:36, Andy Shevchenko wrote: >> On Tue, Feb 16, 2016 at 12:08 PM, Vladimir Murzin >> <vladimir.murzin@arm.com> wrote: >>> This adds support early console for MPS2 UART which can be enabled via >>> earlycon=mps2,0x40004000 >>> +static void mps2_early_putchar(struct uart_port *port, int ch) >>> +{ >>> + >>> + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) >>> + cpu_relax(); >> >> Infinite busy loop? > > In case of broken hw, yes, but I'm quite unsure how we can help to fix > it. Do you have something in mind? Set sane amount of loops like int count = 100; … while (… && --count) Though I have no idea what to do if count == 0. Perhaps it's a TX overrun state. You may try to recover by flushing TX queue in HW (fifo and / or Tx shift register) if HW supports it.
On 16/02/16 13:13, Andy Shevchenko wrote: > On Tue, Feb 16, 2016 at 3:09 PM, Vladimir Murzin > <vladimir.murzin@arm.com> wrote: >> On 16/02/16 10:36, Andy Shevchenko wrote: >>> On Tue, Feb 16, 2016 at 12:08 PM, Vladimir Murzin >>> <vladimir.murzin@arm.com> wrote: >>>> This adds support early console for MPS2 UART which can be enabled via >>>> earlycon=mps2,0x40004000 > >>>> +static void mps2_early_putchar(struct uart_port *port, int ch) >>>> +{ >>>> + >>>> + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) >>>> + cpu_relax(); >>> >>> Infinite busy loop? >> >> In case of broken hw, yes, but I'm quite unsure how we can help to fix >> it. Do you have something in mind? > > Set sane amount of loops like > > int count = 100; > … > while (… && --count) > > Though I have no idea what to do if count == 0. Perhaps it's a TX overrun state. I thought about counter too, like some of drivers do, but had no idea what to do next after counter reaches zero. In case of TX overrun we could get it either because of broken hw or because we entered Linux with that state already set. In both cases, it looks handy to me to attach a debugger and see that we are spinning in this putchar loop. If you do insist to have a counter I'll add one, but if you are not strong about it I'd prefer to leave it as is. > > You may try to recover by flushing TX queue in HW (fifo and / or Tx > shift register) if HW supports it. > I'm afraid this is not supported. Thanks Vladimir
On Tue, Feb 16, 2016 at 3:38 PM, Vladimir Murzin <vladimir.murzin@arm.com> wrote: > On 16/02/16 13:13, Andy Shevchenko wrote: >> On Tue, Feb 16, 2016 at 3:09 PM, Vladimir Murzin >> <vladimir.murzin@arm.com> wrote: >>> On 16/02/16 10:36, Andy Shevchenko wrote: >>>> On Tue, Feb 16, 2016 at 12:08 PM, Vladimir Murzin >>>> <vladimir.murzin@arm.com> wrote: >>>>> +static void mps2_early_putchar(struct uart_port *port, int ch) >>>>> +{ >>>>> + >>>>> + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) >>>>> + cpu_relax(); >>>> >>>> Infinite busy loop? >>> >>> In case of broken hw, yes, but I'm quite unsure how we can help to fix >>> it. Do you have something in mind? >> >> Set sane amount of loops like >> >> int count = 100; >> … >> while (… && --count) >> >> Though I have no idea what to do if count == 0. Perhaps it's a TX overrun state. > > I thought about counter too, like some of drivers do, but had no idea > what to do next after counter reaches zero. In case of TX overrun we > could get it either because of broken hw or because we entered Linux > with that state already set. In both cases, it looks handy to me to > attach a debugger and see that we are spinning in this putchar loop. > > If you do insist to have a counter I'll add one, but if you are not > strong about it I'd prefer to leave it as is. I would wait for Peter Hurley, Alan, or other experienced guys to talk. >> You may try to recover by flushing TX queue in HW (fifo and / or Tx >> shift register) if HW supports it. >> > > I'm afraid this is not supported. Maybe soft reset? Anyway, see above.
> > If you do insist to have a counter I'll add one, but if you are not > > strong about it I'd prefer to leave it as is. > > I would wait for Peter Hurley, Alan, or other experienced guys to talk. I would leave it. Some of the drivers have defensive code of this form, but mostly because they can be hot unplugged and we don't want to spin into oblivion because the user ejected a serial card adapter. Alan
On 16/02/16 13:09, Vladimir Murzin wrote: >>> +EARLYCON_DECLARE(mps2, mps2_early_console_setup); >>> >> +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); >> > >> > IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ >> > one is enough. >> > > I've just tried with OF_ only and seems it works fine. Thanks for > pointing at it! > ..and the reason for that was that I had stdout pointing at uart node. Now, while testing v4 I end-up with configuration: 1) no EARLYCON_DECLARE in mps2-uart.c 2) stdout not set 3) cmdline has "earlycon=mps2,0x40004000 console=/dev/ttyMPS" and I don't see bootconsole, but if I avoid 1) everything works fine. So, I'd leave EARLYCON_DECLARE in this patch. Cheers Vladimir
On Fri, Feb 19, 2016 at 11:45 AM, Vladimir Murzin <vladimir.murzin@arm.com> wrote: > On 16/02/16 13:09, Vladimir Murzin wrote: >>>> +EARLYCON_DECLARE(mps2, mps2_early_console_setup); >>>> >> +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); >>> > >>> > IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ >>> > one is enough. >>> > >> I've just tried with OF_ only and seems it works fine. Thanks for >> pointing at it! >> > > ..and the reason for that was that I had stdout pointing at uart node. > Now, while testing v4 I end-up with configuration: > > 1) no EARLYCON_DECLARE in mps2-uart.c > 2) stdout not set > 3) cmdline has "earlycon=mps2,0x40004000 console=/dev/ttyMPS" > > and I don't see bootconsole, but if I avoid 1) everything works fine. > So, I'd leave EARLYCON_DECLARE in this patch. Peter, what is your comment on this?
On 02/19/2016 01:57 AM, Andy Shevchenko wrote: > On Fri, Feb 19, 2016 at 11:45 AM, Vladimir Murzin > <vladimir.murzin@arm.com> wrote: >> On 16/02/16 13:09, Vladimir Murzin wrote: >>>>> +EARLYCON_DECLARE(mps2, mps2_early_console_setup); >>>>>>> +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); >>>>> >>>>> IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ >>>>> one is enough. >>>>> >>> I've just tried with OF_ only and seems it works fine. Thanks for >>> pointing at it! >>> >> >> ..and the reason for that was that I had stdout pointing at uart node. >> Now, while testing v4 I end-up with configuration: >> >> 1) no EARLYCON_DECLARE in mps2-uart.c >> 2) stdout not set >> 3) cmdline has "earlycon=mps2,0x40004000 console=/dev/ttyMPS" >> >> and I don't see bootconsole, but if I avoid 1) everything works fine. >> So, I'd leave EARLYCON_DECLARE in this patch. > > Peter, what is your comment on this? That Vladimir is not testing with linux-next. linux-next has common framework for both command line and OF-defined earlycons. Vladimir, if you pull in Greg's tty-next branch, then just OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); should enable both command-line and OF-defined earlycons. Regards, Peter Hurley
On 19/02/16 15:46, Peter Hurley wrote: > On 02/19/2016 01:57 AM, Andy Shevchenko wrote: >> On Fri, Feb 19, 2016 at 11:45 AM, Vladimir Murzin >> <vladimir.murzin@arm.com> wrote: >>> On 16/02/16 13:09, Vladimir Murzin wrote: >>>>>> +EARLYCON_DECLARE(mps2, mps2_early_console_setup); >>>>>>>> +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); >>>>>> >>>>>> IIRC Peter Hurley mentioned you don't need to put both anymore, OF_ >>>>>> one is enough. >>>>>> >>>> I've just tried with OF_ only and seems it works fine. Thanks for >>>> pointing at it! >>>> >>> >>> ..and the reason for that was that I had stdout pointing at uart node. >>> Now, while testing v4 I end-up with configuration: >>> >>> 1) no EARLYCON_DECLARE in mps2-uart.c >>> 2) stdout not set >>> 3) cmdline has "earlycon=mps2,0x40004000 console=/dev/ttyMPS" >>> >>> and I don't see bootconsole, but if I avoid 1) everything works fine. >>> So, I'd leave EARLYCON_DECLARE in this patch. >> >> Peter, what is your comment on this? > > That Vladimir is not testing with linux-next. > > linux-next has common framework for both command line and OF-defined > earlycons. > > Vladimir, if you pull in Greg's tty-next branch, then just > > OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); > > should enable both command-line and OF-defined earlycons. Confirmed! No doubt now EARLYCON_DECLARE can be dropped from this patch safely. Thanks! Vladimir > > Regards, > Peter Hurley > >
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 3a248be..4ed6e51 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -1454,6 +1454,7 @@ config SERIAL_MPS2_UART_CONSOLE bool "MPS2 UART console support" depends on SERIAL_MPS2_UART select SERIAL_CORE_CONSOLE + select SERIAL_EARLYCON config SERIAL_MPS2_UART bool "MPS2 UART port" diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c index d97f3a9..f3a3bd1 100644 --- a/drivers/tty/serial/mps2-uart.c +++ b/drivers/tty/serial/mps2-uart.c @@ -435,6 +435,36 @@ static struct console mps2_uart_console = { #define MPS2_SERIAL_CONSOLE (&mps2_uart_console) +static void mps2_early_putchar(struct uart_port *port, int ch) +{ + + while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL) + cpu_relax(); + + writeb((unsigned char)ch, port->membase + UARTn_DATA); +} + + +static void mps2_early_write(struct console *con, const char *s, unsigned n) +{ + struct earlycon_device *dev = con->data; + + uart_console_write(&dev->port, s, n, mps2_early_putchar); +} + +static int __init mps2_early_console_setup(struct earlycon_device *device, + const char *opt) +{ + if (!device->port.membase) + return -ENODEV; + + device->con->write = mps2_early_write; + + return 0; +} +EARLYCON_DECLARE(mps2, mps2_early_console_setup); +OF_EARLYCON_DECLARE(mps2, "arm,mps2-uart", mps2_early_console_setup); + #else #define MPS2_SERIAL_CONSOLE NULL #endif
This adds support early console for MPS2 UART which can be enabled via earlycon=mps2,0x40004000 Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> --- drivers/tty/serial/Kconfig | 1 + drivers/tty/serial/mps2-uart.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)