Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Wed, Feb 03, 2016 at 05:56:56PM +0100, Gregory CLEMENT wrote:
> Hi Mark,
>  
>  On mar., févr. 02 2016, Mark Rutland  wrote:
> >> diff --git a/Documentation/kernel-parameters.txt 
> >> b/Documentation/kernel-parameters.txt
> >> index 87d40a72f6a1..198f6bd56e84 100644
> >> --- a/Documentation/kernel-parameters.txt
> >> +++ b/Documentation/kernel-parameters.txt
> >> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also 
> >> be entirely omitted.
> >>A valid base address must be provided, and the serial
> >>port must already be setup and configured.
> >>  
> >> +  mvebu_uart,
> >> +  Start an early, polled-mode console on an some mvebu
> >> +  SoC (as the Armada-3700) serial port at the specified
> >> +  address. The serial port must already be setup and
> >> +  configured. Options are not yet supported.
> >> +
> >
> > Does the the mvebu UART vary between platforms at all?
> 
> I am not sure to undersatnd your question.
> 
> If you asked about the UART used on the other mvebu SoCs then my answer
> is: on all the other mvebu SoC until now third party IPs were used.

Ah, I hadn't realised that.

> This one is the first one dedicated to an mevbu SoC and currently only
> used on Armada 3700 but I don't know what are the plan for the future
> mvebu SoCs.

Given that, I think it would be better to use "armada3700_uart=" for
now, given it doesn't cover all mvebu SoCs.

Thanks,
Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Mark,
 
 On mar., févr. 02 2016, Mark Rutland  wrote:

>> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
>> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> new file mode 100644
>> index ..6087defd9f93
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> @@ -0,0 +1,13 @@
>> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
>> Armada-3700)
>> +
>> +Required properties:
>> +- compatible: "marvell,armada-3700-uart"
>> +- reg: offset and length of the register set for the device.
>> +- interrupts: device interrupt
>> +
>> +Example:
>> +serial@12000 {
>> +compatible = "marvell,armada-3700-uart";
>> +reg = <0x12000 0x400>;
>> +interrupts = <43>;
>> +};
>
> There are no external clock inputs?

Right, even if we don't use it in the driver we should ad it in the
binding because there is an external clock input.

>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index 87d40a72f6a1..198f6bd56e84 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>>  A valid base address must be provided, and the serial
>>  port must already be setup and configured.
>>  
>> +mvebu_uart,
>> +Start an early, polled-mode console on an some mvebu
>> +SoC (as the Armada-3700) serial port at the specified
>> +address. The serial port must already be setup and
>> +configured. Options are not yet supported.
>> +
>
> Does the the mvebu UART vary between platforms at all?

I am not sure to undersatnd your question.

If you asked about the UART used on the other mvebu SoCs then my answer
is: on all the other mvebu SoC until now third party IPs were used. This
one is the first one dedicated to an mevbu SoC and currently only used
on Armada 3700 but I don't know what are the plan for the future mvebu
SoCs.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Marcin,
 
 On mer., févr. 03 2016, Marcin Wojtas  wrote:

> Hi Gregory
>
>> +
>> +static int mvebu_uart_startup(struct uart_port *port)
>> +{
>> +   int ret;
>> +
>> +   writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
>> +  port->membase + UART_CTRL);
>> +   udelay(1);
>> +   writel(CTRL_RX_INT, port->membase + UART_CTRL);
>> +
>> +   ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, 
>> "serial",
>> + port);
>> +   if (ret) {
>> +   dev_err(port->dev, "failed to request irq\n");
>> +   return ret;
>> +   }
>> +
>> +   return 0;
>> +}
>> +
>
>> +static int mvebu_uart_probe(struct platform_device *pdev)
>> +{
>> +   struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 
>> 0);
>> +   struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 
>> 0);
>> +   struct uart_port *port;
>> +   struct mvebu_uart_data *data;
>> +   int ret;
>> +
>> +   if (!reg || !irq) {
>> +   dev_err(>dev, "no registers/irq defined\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   port = _uart_ports[0];
>> +
>> +   spin_lock_init(>lock);
>> +
>> +   port->dev= >dev;
>> +   port->type   = PORT_MVEBU;
>> +   port->ops= _uart_ops;
>> +   port->regshift   = 0;
>> +
>> +   port->fifosize   = 32;
>> +   port->iotype = UPIO_MEM32;
>> +   port->flags  = UPF_FIXED_PORT;
>> +   port->line   = 0; /* single port: force line number to  0 */
>> +
>> +   port->irq= irq->start;
>> +   port->irqflags   = 0;
>
> Please use port->irqflags = IRQF_SHARED;
> As ubuntu opens multiple consoles A3700 can't boot to it (only to
> buildroot with single console).

But this irq is not shared, this looks like a hack hidding the real
issue.

Gregory

>
> Best regards,
> Marcin

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Arnd,
 
 On mar., févr. 02 2016, Arnd Bergmann  wrote:

> On Tuesday 02 February 2016 19:07:39 Gregory CLEMENT wrote:
>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index 39721ec4f415..b291f934d51b 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>>  depends on SERIAL_STM32=y
>>  select SERIAL_CORE_CONSOLE
>>  
>> +config SERIAL_MVEBU_UART
>> +bool "Marvell EBU serial port support"
>
> Could this be a loadable module?

I don't see any reason to not being able to use it as a module. But I
didn't test it because currently te serial port is the only way ro
cimmunicate with the board.

>
>> +config SERIAL_MVEBU_CONSOLE
>> +bool "Console on Marvell EBU serial port"
>> +depends on SERIAL_MVEBU_UART
>
> If yes, this would become 
>
>   depends on SERIAL_MVEBU_UART=y

OK, let's try to enable it.

Thanks,

Gregory

>
>   Arnd
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Mark,
 
 On mar., févr. 02 2016, Mark Rutland  wrote:

> On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
>> From: Wilson Ding 
>> 
>> Armada-3700's uart is a simple serial port, which doesn't
>> support. Configuring the modem control lines. The uart port has a 32
>> bytes Tx FIFO and a 64 bytes Rx FIFO
>> 
>> The uart driver implements the uart core operations. It also support the
>> system (early) console based on Armada-3700's serial port.
>> 
>> Known Issue:
>> 
>> The uart driver currently doesn't support clock programming, which means
>> the baud-rate stays with the default value configured by the bootloader
>> at boot time
>
> To ensure that the bootloader and kernel match, it's best to place the
> rate in the stdout-path property (as in
> Documentation/devicetree/bindings/chosen.txt).
>
> Presumably that is what you want?

It is done in patch 7.

>
> Is it difficutl to add clock programming?

Currently there is no clock tree support before adding it I would like
to be able to test it.

Thanks,

Gregory

>
>> 
>> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
>> to enumerate]
>> 
>> Signed-off-by: Wilson Ding 
>> Signed-off-by: Nadav Haklai 
>> Signed-off-by: Gregory CLEMENT 
>> ---
>>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>>  Documentation/kernel-parameters.txt|   6 +
>>  drivers/tty/serial/Kconfig |  22 +
>>  drivers/tty/serial/Makefile|   1 +
>>  drivers/tty/serial/mvebu-uart.c| 649 
>> +
>>  include/uapi/linux/serial_core.h   |   3 +
>>  6 files changed, 694 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>>  create mode 100644 drivers/tty/serial/mvebu-uart.c
>> 
>> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
>> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> new file mode 100644
>> index ..6087defd9f93
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> @@ -0,0 +1,13 @@
>> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
>> Armada-3700)
>> +
>> +Required properties:
>> +- compatible: "marvell,armada-3700-uart"
>> +- reg: offset and length of the register set for the device.
>> +- interrupts: device interrupt
>> +
>> +Example:
>> +serial@12000 {
>> +compatible = "marvell,armada-3700-uart";
>> +reg = <0x12000 0x400>;
>> +interrupts = <43>;
>> +};
>
> There are no external clock inputs?
>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index 87d40a72f6a1..198f6bd56e84 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>>  A valid base address must be provided, and the serial
>>  port must already be setup and configured.
>>  
>> +mvebu_uart,
>> +Start an early, polled-mode console on an some mvebu
>> +SoC (as the Armada-3700) serial port at the specified
>> +address. The serial port must already be setup and
>> +configured. Options are not yet supported.
>> +
>
> Does the the mvebu UART vary between platforms at all?
>
> Thanks,
> Mark.

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi,
 
 On mar., févr. 02 2016, One Thousand Gnomes  wrote:

>> +static void mvebu_uart_set_termios(struct uart_port *port,
>> +   struct ktermios *termios,
>> +   struct ktermios *old)
>> +{
>> +unsigned long flags;
>> +unsigned int baud;
>> +
>> +spin_lock_irqsave(>lock, flags);
>> +
>> +port->read_status_mask = STAT_RX_RDY | STAT_OVR_ERR |
>> +STAT_TX_RDY | STAT_TX_FIFO_FUL;
>> +
>> +if (termios->c_iflag & INPCK)
>> +port->read_status_mask |= STAT_FRM_ERR | STAT_PAR_ERR;
>> +
>> +port->ignore_status_mask = 0;
>> +if (termios->c_iflag & IGNPAR)
>> +port->ignore_status_mask |=
>> +STAT_FRM_ERR | STAT_PAR_ERR | STAT_OVR_ERR;
>> +
>> +if ((termios->c_cflag & CREAD) == 0)
>> +port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;
>
> If you don't support parity or charactive size then you should be forcing
> those bits in the tty->termios so that the caller sees what settings they
> get. tty_termios_copy_hw is close to what you need except that you can
> support IGNPAR.

OK thanks for the pointer.

>
> You also want to provide the actual baud rate chosen (see how 8250.c does
> it using tty_termios_encode_baud_rate().
>
>
>> +static struct uart_driver mvebu_uart_driver = {
>> +.owner  = THIS_MODULE,
>> +.driver_name= "serial",
>> +.dev_name   = "ttyS",
>> +.major  = TTY_MAJOR,
>> +.minor  = 64,
>
> NAK
>
> TTY_MAJOR 64+ is the 8250 driver and ttyS is the 8250 driver name. You
> should be using a dynamic major (0) for all new drivers and you need to
> pick a different and unused ttyXXX format name.

I missed this one, I will remove .major and .minor and use our own
ttyXX.

Thanks,

Gregory

>
> Alan

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Marcin Wojtas]

Hi Gregory

> +
> +static int mvebu_uart_startup(struct uart_port *port)
> +{
> +   int ret;
> +
> +   writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
> +  port->membase + UART_CTRL);
> +   udelay(1);
> +   writel(CTRL_RX_INT, port->membase + UART_CTRL);
> +
> +   ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, "serial",
> + port);
> +   if (ret) {
> +   dev_err(port->dev, "failed to request irq\n");
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +

> +static int mvebu_uart_probe(struct platform_device *pdev)
> +{
> +   struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +   struct uart_port *port;
> +   struct mvebu_uart_data *data;
> +   int ret;
> +
> +   if (!reg || !irq) {
> +   dev_err(>dev, "no registers/irq defined\n");
> +   return -EINVAL;
> +   }
> +
> +   port = _uart_ports[0];
> +
> +   spin_lock_init(>lock);
> +
> +   port->dev= >dev;
> +   port->type   = PORT_MVEBU;
> +   port->ops= _uart_ops;
> +   port->regshift   = 0;
> +
> +   port->fifosize   = 32;
> +   port->iotype = UPIO_MEM32;
> +   port->flags  = UPF_FIXED_PORT;
> +   port->line   = 0; /* single port: force line number to  0 */
> +
> +   port->irq= irq->start;
> +   port->irqflags   = 0;

Please use port->irqflags = IRQF_SHARED;
As ubuntu opens multiple consoles A3700 can't boot to it (only to
buildroot with single console).

Best regards,
Marcin

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Marcin Wojtas]

Hi Gregory

> +
> +static int mvebu_uart_startup(struct uart_port *port)
> +{
> +   int ret;
> +
> +   writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
> +  port->membase + UART_CTRL);
> +   udelay(1);
> +   writel(CTRL_RX_INT, port->membase + UART_CTRL);
> +
> +   ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, "serial",
> + port);
> +   if (ret) {
> +   dev_err(port->dev, "failed to request irq\n");
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +

> +static int mvebu_uart_probe(struct platform_device *pdev)
> +{
> +   struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +   struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +   struct uart_port *port;
> +   struct mvebu_uart_data *data;
> +   int ret;
> +
> +   if (!reg || !irq) {
> +   dev_err(>dev, "no registers/irq defined\n");
> +   return -EINVAL;
> +   }
> +
> +   port = _uart_ports[0];
> +
> +   spin_lock_init(>lock);
> +
> +   port->dev= >dev;
> +   port->type   = PORT_MVEBU;
> +   port->ops= _uart_ops;
> +   port->regshift   = 0;
> +
> +   port->fifosize   = 32;
> +   port->iotype = UPIO_MEM32;
> +   port->flags  = UPF_FIXED_PORT;
> +   port->line   = 0; /* single port: force line number to  0 */
> +
> +   port->irq= irq->start;
> +   port->irqflags   = 0;

Please use port->irqflags = IRQF_SHARED;
As ubuntu opens multiple consoles A3700 can't boot to it (only to
buildroot with single console).

Best regards,
Marcin

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Marcin,
 
 On mer., févr. 03 2016, Marcin Wojtas  wrote:

> Hi Gregory
>
>> +
>> +static int mvebu_uart_startup(struct uart_port *port)
>> +{
>> +   int ret;
>> +
>> +   writel(CTRL_TXFIFO_RST | CTRL_RXFIFO_RST,
>> +  port->membase + UART_CTRL);
>> +   udelay(1);
>> +   writel(CTRL_RX_INT, port->membase + UART_CTRL);
>> +
>> +   ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, 
>> "serial",
>> + port);
>> +   if (ret) {
>> +   dev_err(port->dev, "failed to request irq\n");
>> +   return ret;
>> +   }
>> +
>> +   return 0;
>> +}
>> +
>
>> +static int mvebu_uart_probe(struct platform_device *pdev)
>> +{
>> +   struct resource *reg = platform_get_resource(pdev, IORESOURCE_MEM, 
>> 0);
>> +   struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 
>> 0);
>> +   struct uart_port *port;
>> +   struct mvebu_uart_data *data;
>> +   int ret;
>> +
>> +   if (!reg || !irq) {
>> +   dev_err(>dev, "no registers/irq defined\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   port = _uart_ports[0];
>> +
>> +   spin_lock_init(>lock);
>> +
>> +   port->dev= >dev;
>> +   port->type   = PORT_MVEBU;
>> +   port->ops= _uart_ops;
>> +   port->regshift   = 0;
>> +
>> +   port->fifosize   = 32;
>> +   port->iotype = UPIO_MEM32;
>> +   port->flags  = UPF_FIXED_PORT;
>> +   port->line   = 0; /* single port: force line number to  0 */
>> +
>> +   port->irq= irq->start;
>> +   port->irqflags   = 0;
>
> Please use port->irqflags = IRQF_SHARED;
> As ubuntu opens multiple consoles A3700 can't boot to it (only to
> buildroot with single console).

But this irq is not shared, this looks like a hack hidding the real
issue.

Gregory

>
> Best regards,
> Marcin

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi,
 
 On mar., févr. 02 2016, One Thousand Gnomes  wrote:

>> +static void mvebu_uart_set_termios(struct uart_port *port,
>> +   struct ktermios *termios,
>> +   struct ktermios *old)
>> +{
>> +unsigned long flags;
>> +unsigned int baud;
>> +
>> +spin_lock_irqsave(>lock, flags);
>> +
>> +port->read_status_mask = STAT_RX_RDY | STAT_OVR_ERR |
>> +STAT_TX_RDY | STAT_TX_FIFO_FUL;
>> +
>> +if (termios->c_iflag & INPCK)
>> +port->read_status_mask |= STAT_FRM_ERR | STAT_PAR_ERR;
>> +
>> +port->ignore_status_mask = 0;
>> +if (termios->c_iflag & IGNPAR)
>> +port->ignore_status_mask |=
>> +STAT_FRM_ERR | STAT_PAR_ERR | STAT_OVR_ERR;
>> +
>> +if ((termios->c_cflag & CREAD) == 0)
>> +port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;
>
> If you don't support parity or charactive size then you should be forcing
> those bits in the tty->termios so that the caller sees what settings they
> get. tty_termios_copy_hw is close to what you need except that you can
> support IGNPAR.

OK thanks for the pointer.

>
> You also want to provide the actual baud rate chosen (see how 8250.c does
> it using tty_termios_encode_baud_rate().
>
>
>> +static struct uart_driver mvebu_uart_driver = {
>> +.owner  = THIS_MODULE,
>> +.driver_name= "serial",
>> +.dev_name   = "ttyS",
>> +.major  = TTY_MAJOR,
>> +.minor  = 64,
>
> NAK
>
> TTY_MAJOR 64+ is the 8250 driver and ttyS is the 8250 driver name. You
> should be using a dynamic major (0) for all new drivers and you need to
> pick a different and unused ttyXXX format name.

I missed this one, I will remove .major and .minor and use our own
ttyXX.

Thanks,

Gregory

>
> Alan

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Mark,
 
 On mar., févr. 02 2016, Mark Rutland  wrote:

> On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
>> From: Wilson Ding 
>> 
>> Armada-3700's uart is a simple serial port, which doesn't
>> support. Configuring the modem control lines. The uart port has a 32
>> bytes Tx FIFO and a 64 bytes Rx FIFO
>> 
>> The uart driver implements the uart core operations. It also support the
>> system (early) console based on Armada-3700's serial port.
>> 
>> Known Issue:
>> 
>> The uart driver currently doesn't support clock programming, which means
>> the baud-rate stays with the default value configured by the bootloader
>> at boot time
>
> To ensure that the bootloader and kernel match, it's best to place the
> rate in the stdout-path property (as in
> Documentation/devicetree/bindings/chosen.txt).
>
> Presumably that is what you want?

It is done in patch 7.

>
> Is it difficutl to add clock programming?

Currently there is no clock tree support before adding it I would like
to be able to test it.

Thanks,

Gregory

>
>> 
>> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
>> to enumerate]
>> 
>> Signed-off-by: Wilson Ding 
>> Signed-off-by: Nadav Haklai 
>> Signed-off-by: Gregory CLEMENT 
>> ---
>>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>>  Documentation/kernel-parameters.txt|   6 +
>>  drivers/tty/serial/Kconfig |  22 +
>>  drivers/tty/serial/Makefile|   1 +
>>  drivers/tty/serial/mvebu-uart.c| 649 
>> +
>>  include/uapi/linux/serial_core.h   |   3 +
>>  6 files changed, 694 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>>  create mode 100644 drivers/tty/serial/mvebu-uart.c
>> 
>> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
>> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> new file mode 100644
>> index ..6087defd9f93
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> @@ -0,0 +1,13 @@
>> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
>> Armada-3700)
>> +
>> +Required properties:
>> +- compatible: "marvell,armada-3700-uart"
>> +- reg: offset and length of the register set for the device.
>> +- interrupts: device interrupt
>> +
>> +Example:
>> +serial@12000 {
>> +compatible = "marvell,armada-3700-uart";
>> +reg = <0x12000 0x400>;
>> +interrupts = <43>;
>> +};
>
> There are no external clock inputs?
>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index 87d40a72f6a1..198f6bd56e84 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>>  A valid base address must be provided, and the serial
>>  port must already be setup and configured.
>>  
>> +mvebu_uart,
>> +Start an early, polled-mode console on an some mvebu
>> +SoC (as the Armada-3700) serial port at the specified
>> +address. The serial port must already be setup and
>> +configured. Options are not yet supported.
>> +
>
> Does the the mvebu UART vary between platforms at all?
>
> Thanks,
> Mark.

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Mark,
 
 On mar., févr. 02 2016, Mark Rutland  wrote:

>> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
>> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> new file mode 100644
>> index ..6087defd9f93
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>> @@ -0,0 +1,13 @@
>> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
>> Armada-3700)
>> +
>> +Required properties:
>> +- compatible: "marvell,armada-3700-uart"
>> +- reg: offset and length of the register set for the device.
>> +- interrupts: device interrupt
>> +
>> +Example:
>> +serial@12000 {
>> +compatible = "marvell,armada-3700-uart";
>> +reg = <0x12000 0x400>;
>> +interrupts = <43>;
>> +};
>
> There are no external clock inputs?

Right, even if we don't use it in the driver we should ad it in the
binding because there is an external clock input.

>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index 87d40a72f6a1..198f6bd56e84 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>>  A valid base address must be provided, and the serial
>>  port must already be setup and configured.
>>  
>> +mvebu_uart,
>> +Start an early, polled-mode console on an some mvebu
>> +SoC (as the Armada-3700) serial port at the specified
>> +address. The serial port must already be setup and
>> +configured. Options are not yet supported.
>> +
>
> Does the the mvebu UART vary between platforms at all?

I am not sure to undersatnd your question.

If you asked about the UART used on the other mvebu SoCs then my answer
is: on all the other mvebu SoC until now third party IPs were used. This
one is the first one dedicated to an mevbu SoC and currently only used
on Armada 3700 but I don't know what are the plan for the future mvebu
SoCs.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Wed, Feb 03, 2016 at 05:56:56PM +0100, Gregory CLEMENT wrote:
> Hi Mark,
>  
>  On mar., févr. 02 2016, Mark Rutland  wrote:
> >> diff --git a/Documentation/kernel-parameters.txt 
> >> b/Documentation/kernel-parameters.txt
> >> index 87d40a72f6a1..198f6bd56e84 100644
> >> --- a/Documentation/kernel-parameters.txt
> >> +++ b/Documentation/kernel-parameters.txt
> >> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also 
> >> be entirely omitted.
> >>A valid base address must be provided, and the serial
> >>port must already be setup and configured.
> >>  
> >> +  mvebu_uart,
> >> +  Start an early, polled-mode console on an some mvebu
> >> +  SoC (as the Armada-3700) serial port at the specified
> >> +  address. The serial port must already be setup and
> >> +  configured. Options are not yet supported.
> >> +
> >
> > Does the the mvebu UART vary between platforms at all?
> 
> I am not sure to undersatnd your question.
> 
> If you asked about the UART used on the other mvebu SoCs then my answer
> is: on all the other mvebu SoC until now third party IPs were used.

Ah, I hadn't realised that.

> This one is the first one dedicated to an mevbu SoC and currently only
> used on Armada 3700 but I don't know what are the plan for the future
> mvebu SoCs.

Given that, I think it would be better to use "armada3700_uart=" for
now, given it doesn't cover all mvebu SoCs.

Thanks,
Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

Hi Arnd,
 
 On mar., févr. 02 2016, Arnd Bergmann  wrote:

> On Tuesday 02 February 2016 19:07:39 Gregory CLEMENT wrote:
>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index 39721ec4f415..b291f934d51b 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>>  depends on SERIAL_STM32=y
>>  select SERIAL_CORE_CONSOLE
>>  
>> +config SERIAL_MVEBU_UART
>> +bool "Marvell EBU serial port support"
>
> Could this be a loadable module?

I don't see any reason to not being able to use it as a module. But I
didn't test it because currently te serial port is the only way ro
cimmunicate with the board.

>
>> +config SERIAL_MVEBU_CONSOLE
>> +bool "Console on Marvell EBU serial port"
>> +depends on SERIAL_MVEBU_UART
>
> If yes, this would become 
>
>   depends on SERIAL_MVEBU_UART=y

OK, let's try to enable it.

Thanks,

Gregory

>
>   Arnd
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Rob Herring]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
> 
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time
> 
> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +

Acked-by: Rob Herring 

>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Arnd Bergmann]

On Tuesday 02 February 2016 19:07:39 Gregory CLEMENT wrote:

> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 39721ec4f415..b291f934d51b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>   depends on SERIAL_STM32=y
>   select SERIAL_CORE_CONSOLE
>  
> +config SERIAL_MVEBU_UART
> + bool "Marvell EBU serial port support"

Could this be a loadable module?

> +config SERIAL_MVEBU_CONSOLE
> + bool "Console on Marvell EBU serial port"
> + depends on SERIAL_MVEBU_UART

If yes, this would become 

depends on SERIAL_MVEBU_UART=y

Arnd

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
> 
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time

To ensure that the bootloader and kernel match, it's best to place the
rate in the stdout-path property (as in
Documentation/devicetree/bindings/chosen.txt).

Presumably that is what you want?

Is it difficutl to add clock programming?

> 
> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> new file mode 100644
> index ..6087defd9f93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> @@ -0,0 +1,13 @@
> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
> Armada-3700)
> +
> +Required properties:
> +- compatible: "marvell,armada-3700-uart"
> +- reg: offset and length of the register set for the device.
> +- interrupts: device interrupt
> +
> +Example:
> + serial@12000 {
> + compatible = "marvell,armada-3700-uart";
> + reg = <0x12000 0x400>;
> + interrupts = <43>;
> + };

There are no external clock inputs?

> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index 87d40a72f6a1..198f6bd56e84 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
>   A valid base address must be provided, and the serial
>   port must already be setup and configured.
>  
> + mvebu_uart,
> + Start an early, polled-mode console on an some mvebu
> + SoC (as the Armada-3700) serial port at the specified
> + address. The serial port must already be setup and
> + configured. Options are not yet supported.
> +

Does the the mvebu UART vary between platforms at all?

Thanks,
Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 06:19:29PM +, Mark Rutland wrote:
> On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> > From: Wilson Ding 
> > 
> > Armada-3700's uart is a simple serial port, which doesn't
> > support. Configuring the modem control lines. The uart port has a 32
> > bytes Tx FIFO and a 64 bytes Rx FIFO
> > 
> > The uart driver implements the uart core operations. It also support the
> > system (early) console based on Armada-3700's serial port.
> >
> > Known Issue:
> > 
> > The uart driver currently doesn't support clock programming, which means
> > the baud-rate stays with the default value configured by the bootloader
> > at boot time
> 
> This looks like any other early console.
> 
> Why does this not use the earlycon infrastructure?

Ah, it does, and I misread the diff quite badly.

Apologies for the noise.

Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[One Thousand Gnomes]

> +static void mvebu_uart_set_termios(struct uart_port *port,
> +struct ktermios *termios,
> +struct ktermios *old)
> +{
> + unsigned long flags;
> + unsigned int baud;
> +
> + spin_lock_irqsave(>lock, flags);
> +
> + port->read_status_mask = STAT_RX_RDY | STAT_OVR_ERR |
> + STAT_TX_RDY | STAT_TX_FIFO_FUL;
> +
> + if (termios->c_iflag & INPCK)
> + port->read_status_mask |= STAT_FRM_ERR | STAT_PAR_ERR;
> +
> + port->ignore_status_mask = 0;
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |=
> + STAT_FRM_ERR | STAT_PAR_ERR | STAT_OVR_ERR;
> +
> + if ((termios->c_cflag & CREAD) == 0)
> + port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;

If you don't support parity or charactive size then you should be forcing
those bits in the tty->termios so that the caller sees what settings they
get. tty_termios_copy_hw is close to what you need except that you can
support IGNPAR.

You also want to provide the actual baud rate chosen (see how 8250.c does
it using tty_termios_encode_baud_rate().


> +static struct uart_driver mvebu_uart_driver = {
> + .owner  = THIS_MODULE,
> + .driver_name= "serial",
> + .dev_name   = "ttyS",
> + .major  = TTY_MAJOR,
> + .minor  = 64,

NAK

TTY_MAJOR 64+ is the 8250 driver and ttyS is the 8250 driver name. You
should be using a dynamic major (0) for all new drivers and you need to
pick a different and unused ttyXXX format name.

Alan

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
>
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time

This looks like any other early console.

Why does this not use the earlycon infrastructure?

See {OF_,}EARLYCON_DECLARE (e.g. in drivers/tty/serial/amba-pl011.c).

Mark.

> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> new file mode 100644
> index ..6087defd9f93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> @@ -0,0 +1,13 @@
> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
> Armada-3700)
> +
> +Required properties:
> +- compatible: "marvell,armada-3700-uart"
> +- reg: offset and length of the register set for the device.
> +- interrupts: device interrupt
> +
> +Example:
> + serial@12000 {
> + compatible = "marvell,armada-3700-uart";
> + reg = <0x12000 0x400>;
> + interrupts = <43>;
> + };
> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index 87d40a72f6a1..198f6bd56e84 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
>   A valid base address must be provided, and the serial
>   port must already be setup and configured.
>  
> + mvebu_uart,
> + Start an early, polled-mode console on an some mvebu
> + SoC (as the Armada-3700) serial port at the specified
> + address. The serial port must already be setup and
> + configured. Options are not yet supported.
> +
>   earlyprintk=[X86,SH,BLACKFIN,ARM,M68k]
>   earlyprintk=vga
>   earlyprintk=efi
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 39721ec4f415..b291f934d51b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>   depends on SERIAL_STM32=y
>   select SERIAL_CORE_CONSOLE
>  
> +config SERIAL_MVEBU_UART
> + bool "Marvell EBU serial port support"
> + select SERIAL_CORE
> + help
> +   This driver is for Marvell EBU SoC's UART. If you have a machine
> +   based on the Armada-3700 SoC and wish to use the on-board serial
> +   port,
> +   say 'Y' here.
> +   Otherwise, say 'N'.
> +
> +config SERIAL_MVEBU_CONSOLE
> + bool "Console on Marvell EBU serial port"
> + depends on SERIAL_MVEBU_UART
> + select SERIAL_CORE_CONSOLE
> + select SERIAL_EARLYCON
> + default y
> + help
> +   Say 'Y' here if you wish to use Armada-3700 UART as the system 
> console.
> +   (the system console is the device which receives all kernel messages
> +   and warnings and which allows logins in single user mode)
> +   Otherwise, say 'N'.
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index b391c9b31960..988167595330 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR) += 
> digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)+= men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>  obj-$(CONFIG_SERIAL_STM32)   += stm32-usart.o
> +obj-$(CONFIG_SERIAL_MVEBU_UART)  += mvebu-uart.o
>  
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)  += serial_mctrl_gpio.o
> diff --git 

[PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

From: Wilson Ding 

Armada-3700's uart is a simple serial port, which doesn't
support. Configuring the modem control lines. The uart port has a 32
bytes Tx FIFO and a 64 bytes Rx FIFO

The uart driver implements the uart core operations. It also support the
system (early) console based on Armada-3700's serial port.

Known Issue:

The uart driver currently doesn't support clock programming, which means
the baud-rate stays with the default value configured by the bootloader
at boot time

[gregory.clem...@free-electrons.com: Rewrite many part which are too long
to enumerate]

Signed-off-by: Wilson Ding 
Signed-off-by: Nadav Haklai 
Signed-off-by: Gregory CLEMENT 
---
 .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
 Documentation/kernel-parameters.txt|   6 +
 drivers/tty/serial/Kconfig |  22 +
 drivers/tty/serial/Makefile|   1 +
 drivers/tty/serial/mvebu-uart.c| 649 +
 include/uapi/linux/serial_core.h   |   3 +
 6 files changed, 694 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
 create mode 100644 drivers/tty/serial/mvebu-uart.c

diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
new file mode 100644
index ..6087defd9f93
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
@@ -0,0 +1,13 @@
+* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
Armada-3700)
+
+Required properties:
+- compatible: "marvell,armada-3700-uart"
+- reg: offset and length of the register set for the device.
+- interrupts: device interrupt
+
+Example:
+   serial@12000 {
+   compatible = "marvell,armada-3700-uart";
+   reg = <0x12000 0x400>;
+   interrupts = <43>;
+   };
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 87d40a72f6a1..198f6bd56e84 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
A valid base address must be provided, and the serial
port must already be setup and configured.
 
+   mvebu_uart,
+   Start an early, polled-mode console on an some mvebu
+   SoC (as the Armada-3700) serial port at the specified
+   address. The serial port must already be setup and
+   configured. Options are not yet supported.
+
earlyprintk=[X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 39721ec4f415..b291f934d51b 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
depends on SERIAL_STM32=y
select SERIAL_CORE_CONSOLE
 
+config SERIAL_MVEBU_UART
+   bool "Marvell EBU serial port support"
+   select SERIAL_CORE
+   help
+ This driver is for Marvell EBU SoC's UART. If you have a machine
+ based on the Armada-3700 SoC and wish to use the on-board serial
+ port,
+ say 'Y' here.
+ Otherwise, say 'N'.
+
+config SERIAL_MVEBU_CONSOLE
+   bool "Console on Marvell EBU serial port"
+   depends on SERIAL_MVEBU_UART
+   select SERIAL_CORE_CONSOLE
+   select SERIAL_EARLYCON
+   default y
+   help
+ Say 'Y' here if you wish to use Armada-3700 UART as the system 
console.
+ (the system console is the device which receives all kernel messages
+ and warnings and which allows logins in single user mode)
+ Otherwise, say 'N'.
+
 endmenu
 
 config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index b391c9b31960..988167595330 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
digicolor-usart.o
 obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
 obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
 obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
+obj-$(CONFIG_SERIAL_MVEBU_UART)+= mvebu-uart.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
new file mode 100644
index ..9624070d6058
--- /dev/null
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -0,0 +1,649 @@
+/*
+* ***
+* Copyright (C) 2015 Marvell International Ltd.
+* ***
+* This program is 

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 06:19:29PM +, Mark Rutland wrote:
> On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> > From: Wilson Ding 
> > 
> > Armada-3700's uart is a simple serial port, which doesn't
> > support. Configuring the modem control lines. The uart port has a 32
> > bytes Tx FIFO and a 64 bytes Rx FIFO
> > 
> > The uart driver implements the uart core operations. It also support the
> > system (early) console based on Armada-3700's serial port.
> >
> > Known Issue:
> > 
> > The uart driver currently doesn't support clock programming, which means
> > the baud-rate stays with the default value configured by the bootloader
> > at boot time
> 
> This looks like any other early console.
> 
> Why does this not use the earlycon infrastructure?

Ah, it does, and I misread the diff quite badly.

Apologies for the noise.

Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
> 
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time

To ensure that the bootloader and kernel match, it's best to place the
rate in the stdout-path property (as in
Documentation/devicetree/bindings/chosen.txt).

Presumably that is what you want?

Is it difficutl to add clock programming?

> 
> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> new file mode 100644
> index ..6087defd9f93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> @@ -0,0 +1,13 @@
> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
> Armada-3700)
> +
> +Required properties:
> +- compatible: "marvell,armada-3700-uart"
> +- reg: offset and length of the register set for the device.
> +- interrupts: device interrupt
> +
> +Example:
> + serial@12000 {
> + compatible = "marvell,armada-3700-uart";
> + reg = <0x12000 0x400>;
> + interrupts = <43>;
> + };

There are no external clock inputs?

> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index 87d40a72f6a1..198f6bd56e84 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
>   A valid base address must be provided, and the serial
>   port must already be setup and configured.
>  
> + mvebu_uart,
> + Start an early, polled-mode console on an some mvebu
> + SoC (as the Armada-3700) serial port at the specified
> + address. The serial port must already be setup and
> + configured. Options are not yet supported.
> +

Does the the mvebu UART vary between platforms at all?

Thanks,
Mark.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[One Thousand Gnomes]

> +static void mvebu_uart_set_termios(struct uart_port *port,
> +struct ktermios *termios,
> +struct ktermios *old)
> +{
> + unsigned long flags;
> + unsigned int baud;
> +
> + spin_lock_irqsave(>lock, flags);
> +
> + port->read_status_mask = STAT_RX_RDY | STAT_OVR_ERR |
> + STAT_TX_RDY | STAT_TX_FIFO_FUL;
> +
> + if (termios->c_iflag & INPCK)
> + port->read_status_mask |= STAT_FRM_ERR | STAT_PAR_ERR;
> +
> + port->ignore_status_mask = 0;
> + if (termios->c_iflag & IGNPAR)
> + port->ignore_status_mask |=
> + STAT_FRM_ERR | STAT_PAR_ERR | STAT_OVR_ERR;
> +
> + if ((termios->c_cflag & CREAD) == 0)
> + port->ignore_status_mask |= STAT_RX_RDY | STAT_BRK_ERR;

If you don't support parity or charactive size then you should be forcing
those bits in the tty->termios so that the caller sees what settings they
get. tty_termios_copy_hw is close to what you need except that you can
support IGNPAR.

You also want to provide the actual baud rate chosen (see how 8250.c does
it using tty_termios_encode_baud_rate().


> +static struct uart_driver mvebu_uart_driver = {
> + .owner  = THIS_MODULE,
> + .driver_name= "serial",
> + .dev_name   = "ttyS",
> + .major  = TTY_MAJOR,
> + .minor  = 64,

NAK

TTY_MAJOR 64+ is the 8250 driver and ttyS is the 8250 driver name. You
should be using a dynamic major (0) for all new drivers and you need to
pick a different and unused ttyXXX format name.

Alan

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Mark Rutland]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
>
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time

This looks like any other early console.

Why does this not use the earlycon infrastructure?

See {OF_,}EARLYCON_DECLARE (e.g. in drivers/tty/serial/amba-pl011.c).

Mark.

> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c
> 
> diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
> b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> new file mode 100644
> index ..6087defd9f93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
> @@ -0,0 +1,13 @@
> +* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
> Armada-3700)
> +
> +Required properties:
> +- compatible: "marvell,armada-3700-uart"
> +- reg: offset and length of the register set for the device.
> +- interrupts: device interrupt
> +
> +Example:
> + serial@12000 {
> + compatible = "marvell,armada-3700-uart";
> + reg = <0x12000 0x400>;
> + interrupts = <43>;
> + };
> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index 87d40a72f6a1..198f6bd56e84 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
>   A valid base address must be provided, and the serial
>   port must already be setup and configured.
>  
> + mvebu_uart,
> + Start an early, polled-mode console on an some mvebu
> + SoC (as the Armada-3700) serial port at the specified
> + address. The serial port must already be setup and
> + configured. Options are not yet supported.
> +
>   earlyprintk=[X86,SH,BLACKFIN,ARM,M68k]
>   earlyprintk=vga
>   earlyprintk=efi
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 39721ec4f415..b291f934d51b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>   depends on SERIAL_STM32=y
>   select SERIAL_CORE_CONSOLE
>  
> +config SERIAL_MVEBU_UART
> + bool "Marvell EBU serial port support"
> + select SERIAL_CORE
> + help
> +   This driver is for Marvell EBU SoC's UART. If you have a machine
> +   based on the Armada-3700 SoC and wish to use the on-board serial
> +   port,
> +   say 'Y' here.
> +   Otherwise, say 'N'.
> +
> +config SERIAL_MVEBU_CONSOLE
> + bool "Console on Marvell EBU serial port"
> + depends on SERIAL_MVEBU_UART
> + select SERIAL_CORE_CONSOLE
> + select SERIAL_EARLYCON
> + default y
> + help
> +   Say 'Y' here if you wish to use Armada-3700 UART as the system 
> console.
> +   (the system console is the device which receives all kernel messages
> +   and warnings and which allows logins in single user mode)
> +   Otherwise, say 'N'.
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index b391c9b31960..988167595330 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR) += 
> digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)+= men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>  obj-$(CONFIG_SERIAL_STM32)   += stm32-usart.o
> +obj-$(CONFIG_SERIAL_MVEBU_UART)  += mvebu-uart.o
>  
>  # GPIOLIB helpers for modem 

[PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Gregory CLEMENT]

From: Wilson Ding 

Armada-3700's uart is a simple serial port, which doesn't
support. Configuring the modem control lines. The uart port has a 32
bytes Tx FIFO and a 64 bytes Rx FIFO

The uart driver implements the uart core operations. It also support the
system (early) console based on Armada-3700's serial port.

Known Issue:

The uart driver currently doesn't support clock programming, which means
the baud-rate stays with the default value configured by the bootloader
at boot time

[gregory.clem...@free-electrons.com: Rewrite many part which are too long
to enumerate]

Signed-off-by: Wilson Ding 
Signed-off-by: Nadav Haklai 
Signed-off-by: Gregory CLEMENT 
---
 .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +
 Documentation/kernel-parameters.txt|   6 +
 drivers/tty/serial/Kconfig |  22 +
 drivers/tty/serial/Makefile|   1 +
 drivers/tty/serial/mvebu-uart.c| 649 +
 include/uapi/linux/serial_core.h   |   3 +
 6 files changed, 694 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
 create mode 100644 drivers/tty/serial/mvebu-uart.c

diff --git a/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt 
b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
new file mode 100644
index ..6087defd9f93
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
@@ -0,0 +1,13 @@
+* Marvell UART : Non standard UART used in some of Marvell EBU SoCs (e.g., 
Armada-3700)
+
+Required properties:
+- compatible: "marvell,armada-3700-uart"
+- reg: offset and length of the register set for the device.
+- interrupts: device interrupt
+
+Example:
+   serial@12000 {
+   compatible = "marvell,armada-3700-uart";
+   reg = <0x12000 0x400>;
+   interrupts = <43>;
+   };
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 87d40a72f6a1..198f6bd56e84 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1058,6 +1058,12 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
A valid base address must be provided, and the serial
port must already be setup and configured.
 
+   mvebu_uart,
+   Start an early, polled-mode console on an some mvebu
+   SoC (as the Armada-3700) serial port at the specified
+   address. The serial port must already be setup and
+   configured. Options are not yet supported.
+
earlyprintk=[X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 39721ec4f415..b291f934d51b 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
depends on SERIAL_STM32=y
select SERIAL_CORE_CONSOLE
 
+config SERIAL_MVEBU_UART
+   bool "Marvell EBU serial port support"
+   select SERIAL_CORE
+   help
+ This driver is for Marvell EBU SoC's UART. If you have a machine
+ based on the Armada-3700 SoC and wish to use the on-board serial
+ port,
+ say 'Y' here.
+ Otherwise, say 'N'.
+
+config SERIAL_MVEBU_CONSOLE
+   bool "Console on Marvell EBU serial port"
+   depends on SERIAL_MVEBU_UART
+   select SERIAL_CORE_CONSOLE
+   select SERIAL_EARLYCON
+   default y
+   help
+ Say 'Y' here if you wish to use Armada-3700 UART as the system 
console.
+ (the system console is the device which receives all kernel messages
+ and warnings and which allows logins in single user mode)
+ Otherwise, say 'N'.
+
 endmenu
 
 config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index b391c9b31960..988167595330 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)   += 
digicolor-usart.o
 obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
 obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
 obj-$(CONFIG_SERIAL_STM32) += stm32-usart.o
+obj-$(CONFIG_SERIAL_MVEBU_UART)+= mvebu-uart.o
 
 # GPIOLIB helpers for modem control lines
 obj-$(CONFIG_SERIAL_MCTRL_GPIO)+= serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
new file mode 100644
index ..9624070d6058
--- /dev/null
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -0,0 +1,649 @@
+/*
+* ***
+* Copyright (C) 2015 Marvell International Ltd.

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Arnd Bergmann]

On Tuesday 02 February 2016 19:07:39 Gregory CLEMENT wrote:

> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 39721ec4f415..b291f934d51b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1606,6 +1606,28 @@ config SERIAL_STM32_CONSOLE
>   depends on SERIAL_STM32=y
>   select SERIAL_CORE_CONSOLE
>  
> +config SERIAL_MVEBU_UART
> + bool "Marvell EBU serial port support"

Could this be a loadable module?

> +config SERIAL_MVEBU_CONSOLE
> + bool "Console on Marvell EBU serial port"
> + depends on SERIAL_MVEBU_UART

If yes, this would become 

depends on SERIAL_MVEBU_UART=y

Arnd

Re: [PATCH 01/10] serial: mvebu-uart: initial support for Armada-3700 serial port

[Rob Herring]

On Tue, Feb 02, 2016 at 07:07:39PM +0100, Gregory CLEMENT wrote:
> From: Wilson Ding 
> 
> Armada-3700's uart is a simple serial port, which doesn't
> support. Configuring the modem control lines. The uart port has a 32
> bytes Tx FIFO and a 64 bytes Rx FIFO
> 
> The uart driver implements the uart core operations. It also support the
> system (early) console based on Armada-3700's serial port.
> 
> Known Issue:
> 
> The uart driver currently doesn't support clock programming, which means
> the baud-rate stays with the default value configured by the bootloader
> at boot time
> 
> [gregory.clem...@free-electrons.com: Rewrite many part which are too long
> to enumerate]
> 
> Signed-off-by: Wilson Ding 
> Signed-off-by: Nadav Haklai 
> Signed-off-by: Gregory CLEMENT 
> ---
>  .../devicetree/bindings/tty/serial/mvebu-uart.txt  |  13 +

Acked-by: Rob Herring 

>  Documentation/kernel-parameters.txt|   6 +
>  drivers/tty/serial/Kconfig |  22 +
>  drivers/tty/serial/Makefile|   1 +
>  drivers/tty/serial/mvebu-uart.c| 649 
> +
>  include/uapi/linux/serial_core.h   |   3 +
>  6 files changed, 694 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/tty/serial/mvebu-uart.txt
>  create mode 100644 drivers/tty/serial/mvebu-uart.c