Hi Miquel, On ven., oct. 06 2017, Miquel Raynal <miquel.raynal@xxxxxxxxxxxxxxxxxx> wrote: > While the standard UART port can use a single IRQ that 'sums' both RX > and TX interrupts, the extended port cannot and has to use two different > ISR, one for each direction. The standard port also has the hability ability > to use two separate interrupts (one for each direction). > > The logic is then: either there is only one unnamed interrupt on the > standard port and this interrupt must be used for both directions > (this is legacy bindings); or all the interrupts must be described and > named 'uart-sum' (if available), 'uart-rx', 'uart-tx' and two separate > handlers for each direction will be used. > > Suggested-by: Allen Yan <yanwei@xxxxxxxxxxx> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxxxxxxxxx> > --- > drivers/tty/serial/mvebu-uart.c | 129 ++++++++++++++++++++++++++++++++++++---- > 1 file changed, 118 insertions(+), 11 deletions(-) > > diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c > index 46d10209637a..b52cbe8c0558 100644 > --- a/drivers/tty/serial/mvebu-uart.c > +++ b/drivers/tty/serial/mvebu-uart.c > @@ -79,7 +79,16 @@ > #define MVEBU_UART_TYPE "mvebu-uart" > #define DRIVER_NAME "mvebu_serial" > > -/* Register offsets, different depending on the UART */ > +enum { > + /* Either there is only one summed IRQ... */ > + UART_IRQ_SUM = 0, > + /* ...or there are two separate IRQ for RX and TX */ > + UART_RX_IRQ = 0, > + UART_TX_IRQ, > + UART_IRQ_COUNT > +}; > + > +/* Diverging register offsets */ > struct uart_regs_layout { > unsigned int rbr; > unsigned int tsh; > @@ -106,6 +115,8 @@ struct mvebu_uart_driver_data { > struct mvebu_uart { > struct uart_port *port; > struct clk *clk; > + int irq[UART_IRQ_COUNT]; > + unsigned char __iomem *nb; > struct mvebu_uart_driver_data *data; > }; > > @@ -313,9 +324,32 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id) > unsigned int st = readl(port->membase + UART_STAT); > > if (st & (STAT_RX_RDY(port) | STAT_OVR_ERR | STAT_FRM_ERR | > + STAT_BRK_DET)) > + mvebu_uart_rx_chars(port, st); > + > + if (st & STAT_TX_RDY(port)) > + mvebu_uart_tx_chars(port, st); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t mvebu_uart_rx_isr(int irq, void *dev_id) > +{ > + struct uart_port *port = (struct uart_port *)dev_id; > + unsigned int st = readl(port->membase + UART_STAT); > + > + if (st & (STAT_RX_RDY(port) | STAT_OVR_ERR | STAT_FRM_ERR | > STAT_BRK_DET)) > mvebu_uart_rx_chars(port, st); > > + return IRQ_HANDLED; > +} > + > +static irqreturn_t mvebu_uart_tx_isr(int irq, void *dev_id) > +{ > + struct uart_port *port = (struct uart_port *)dev_id; > + unsigned int st = readl(port->membase + UART_STAT); > + > if (st & STAT_TX_RDY(port)) > mvebu_uart_tx_chars(port, st); > > @@ -324,6 +358,7 @@ static irqreturn_t mvebu_uart_isr(int irq, void *dev_id) > > static int mvebu_uart_startup(struct uart_port *port) > { > + struct mvebu_uart *mvuart = to_mvuart(port); > unsigned int ctl; > int ret; > > @@ -342,11 +377,37 @@ static int mvebu_uart_startup(struct uart_port *port) > ctl |= CTRL_RX_RDY_INT(port); > writel(ctl, port->membase + UART_INTR(port)); > > - ret = request_irq(port->irq, mvebu_uart_isr, port->irqflags, > - DRIVER_NAME, port); > - if (ret) { > - dev_err(port->dev, "failed to request irq\n"); > - return ret; > + if (!mvuart->irq[UART_TX_IRQ]) { > + /* Old bindings with just one interrupt (UART0 only) */ > + ret = devm_request_irq(port->dev, mvuart->irq[UART_IRQ_SUM], > + mvebu_uart_isr, port->irqflags, > + dev_name(port->dev), port); > + if (ret) { > + dev_err(port->dev, "unable to request IRQ %d\n", > + mvuart->irq[UART_IRQ_SUM]); > + return ret; > + } > + } else { > + /* New bindings with an IRQ for RX and TX (both UART) */ > + ret = devm_request_irq(port->dev, mvuart->irq[UART_RX_IRQ], > + mvebu_uart_rx_isr, port->irqflags, > + dev_name(port->dev), port); > + if (ret) { > + dev_err(port->dev, "unable to request IRQ %d\n", > + mvuart->irq[UART_RX_IRQ]); > + return ret; > + } > + > + ret = devm_request_irq(port->dev, mvuart->irq[UART_TX_IRQ], > + mvebu_uart_tx_isr, port->irqflags, > + dev_name(port->dev), > + port); > + if (ret) { > + dev_err(port->dev, "unable to request IRQ %d\n", > + mvuart->irq[UART_TX_IRQ]); > + free_irq(mvuart->irq[UART_RX_IRQ], port); If you register with devm_request_irq then you have to free with devm_free_irq(). > + return ret; > + } > } > > return 0; > @@ -354,9 +415,16 @@ static int mvebu_uart_startup(struct uart_port *port) > > static void mvebu_uart_shutdown(struct uart_port *port) > { > + struct mvebu_uart *mvuart = to_mvuart(port); > + > writel(0, port->membase + UART_INTR(port)); > > - free_irq(port->irq, port); > + if (!mvuart->irq[UART_TX_IRQ]) { > + free_irq(mvuart->irq[UART_IRQ_SUM], port); same here use devm_free_irq(). > + } else { > + free_irq(mvuart->irq[UART_RX_IRQ], port); > + free_irq(mvuart->irq[UART_TX_IRQ], port); And here again. Gregory > + } > } > > static int mvebu_uart_baud_rate_set(struct uart_port *port, unsigned int baud) > @@ -658,15 +726,15 @@ static const struct of_device_id mvebu_uart_of_match[]; > 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); > const struct of_device_id *match = of_match_device(mvebu_uart_of_match, > &pdev->dev); > struct uart_port *port; > struct mvebu_uart *mvuart; > + int irq; > int ret; > > - if (!reg || !irq) { > - dev_err(&pdev->dev, "no registers/irq defined\n"); > + if (!reg) { > + dev_err(&pdev->dev, "no registers defined\n"); > return -EINVAL; > } > > @@ -693,7 +761,12 @@ static int mvebu_uart_probe(struct platform_device *pdev) > port->flags = UPF_FIXED_PORT; > port->line = pdev->id; > > - port->irq = irq->start; > + /* > + * IRQ number is not stored in this structure because we may have two of > + * them per port (RX and TX). Instead, use the driver UART structure > + * array so called ->irq[]. > + */ > + port->irq = 0; > port->irqflags = 0; > port->mapbase = reg->start; > > @@ -728,6 +801,40 @@ static int mvebu_uart_probe(struct platform_device *pdev) > port->uartclk = clk_get_rate(mvuart->clk); > } > > + /* Manage interrupts */ > + memset(mvuart->irq, 0, UART_IRQ_COUNT); > + if (platform_irq_count(pdev) == 1) { > + /* Old bindings: no name on the single unamed UART0 IRQ */ > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(&pdev->dev, "unable to get UART IRQ\n"); > + return irq; > + } > + > + mvuart->irq[UART_IRQ_SUM] = irq; > + } else { > + /* > + * New bindings: named interrupts (RX, TX) for both UARTS, > + * only make use of uart-rx and uart-tx interrupts, do not use > + * uart-sum of UART0 port. > + */ > + irq = platform_get_irq_byname(pdev, "uart-rx"); > + if (irq < 0) { > + dev_err(&pdev->dev, "unable to get 'uart-rx' IRQ\n"); > + return irq; > + } > + > + mvuart->irq[UART_RX_IRQ] = irq; > + > + irq = platform_get_irq_byname(pdev, "uart-tx"); > + if (irq < 0) { > + dev_err(&pdev->dev, "unable to get 'uart-tx' IRQ\n"); > + return irq; > + } > + > + mvuart->irq[UART_TX_IRQ] = irq; > + } > + > /* UART Soft Reset*/ > writel(CTRL_SOFT_RST, port->membase + UART_CTRL(port)); > udelay(1); > -- > 2.11.0 > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html