Skip to contentSkip to conversation

dt-bindings: serial: lantiq: Update for new SoC

PATCH Andy Shevchenko 1 participant 5 messages Aug 7, 2019 6 messages not in the archive TTY layer and serial drivers

lore
Share & tools
lore

Andy Shevchenko

author
#2 lore raw
In reply to a message not in the archive
Quoted 3 lines · Rahul Tanwar

There is a new Intel Atom based Lightning Mountain(LGM) network processor SoC which reuses Lantiq ASC serial controller IP. This patch adds new compatible string and its expected property value in order to support the driver for LGM as well.

I think it makes sense to convert to YAML before adding new properties.

Quoted 43 lines
Signed-off-by Rahul Tanwar <[email protected]>
---
 Documentation/devicetree/bindings/serial/lantiq_asc.txt | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/serial/lantiq_asc.txt +15 -2
index 40e81a5818f6..18b45dd13a61 100644
@@ -1,10 +1,14 @@
 Lantiq SoC ASC serial controller
 
 Required properties:
-- compatible : Should be "lantiq,asc"
+- compatible : Should be "lantiq,asc" or "intel,lgm-asc"
 - reg : Address and length of the register set for the device
-- interrupts: the 3 (tx rx err) interrupt numbers. The interrupt specifier
+- interrupts:
+  For "lantiq,asc" - the 3 (tx rx err) interrupt numbers. The interrupt specifier
   depends on the interrupt-parent interrupt controller.
+	or
+  For "intel,lgm-asc" - the common interrupt number for all of tx rx & err interrupts
+  followed by level/sense specifier.
 
 Optional properties:
 - clocks: Should contain frequency clock and gate clock

@@ -29,3 +33,12 @@ asc1: serial@e100c00 {
 	interrupt-parent = <&icu0>;
 	interrupts = <112 113 114>;
 };
+
+asc0: serial@e0a00000 {
+	compatible = "intel,lgm-asc";
+	reg = <0xe0a00000 0x1000>;
+	interrupt-parent = <&ioapic1>;
+	interrupts = <128 1>;
+	clocks = <&cgu0 LGM_CLK_NOC4>, <&cgu0 LGM_GCLK_ASC0>;
+	clock-names = "freq", "asc";
+};
Signature · 1 line
2.11.0
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author Reviewed-by
#4 lore raw

Re: [PATCH 1/5] serial: lantiq: Add SMP support

In reply to a message not in the archive
Quoted 3 lines · Rahul Tanwar

The existing driver can only support single core SoC. But new multicore platforms which reuse the same driver/IP need SMP support. This patch adds multicore support in the driver.

Reviewed-by Andy Shevchenko <[email protected]>
Quoted 194 lines
Signed-off-by Rahul Tanwar <[email protected]>
---
 drivers/tty/serial/lantiq.c | 47 ++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)
drivers/tty/serial/lantiq.c +32 -15
index 9de9f0f239a1..42e27b48e9cc 100644
@@ -99,7 +99,6 @@
 static void lqasc_tx_chars(struct uart_port *port);
 static struct ltq_uart_port *lqasc_port[MAXPORTS];
 static struct uart_driver lqasc_reg;
-static DEFINE_SPINLOCK(ltq_asc_lock);
 
 struct ltq_uart_port {
 	struct uart_port	port;

@@ -110,6 +109,7 @@ struct ltq_uart_port {
 	unsigned int		tx_irq;
 	unsigned int		rx_irq;
 	unsigned int		err_irq;
+	spinlock_t		lock; /* exclusive access for multi core */
 };
 
 static inline void asc_update_bits(u32 clear, u32 set, void __iomem *reg)

@@ -135,9 +135,11 @@ static void
 lqasc_start_tx(struct uart_port *port)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&ltq_asc_lock, flags);
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	lqasc_tx_chars(port);
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	return;
 }
 

@@ -245,9 +247,11 @@ lqasc_tx_int(int irq, void *_port)
 {
 	unsigned long flags;
 	struct uart_port *port = (struct uart_port *)_port;
-	spin_lock_irqsave(&ltq_asc_lock, flags);
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	__raw_writel(ASC_IRNCR_TIR, port->membase + LTQ_ASC_IRNCR);
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	lqasc_start_tx(port);
 	return IRQ_HANDLED;
 }

@@ -257,11 +261,13 @@ lqasc_err_int(int irq, void *_port)
 {
 	unsigned long flags;
 	struct uart_port *port = (struct uart_port *)_port;
-	spin_lock_irqsave(&ltq_asc_lock, flags);
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	/* clear any pending interrupts */
 	asc_update_bits(0, ASCWHBSTATE_CLRPE | ASCWHBSTATE_CLRFE |
 		ASCWHBSTATE_CLRROE, port->membase + LTQ_ASC_WHBSTATE);
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	return IRQ_HANDLED;
 }
 

@@ -270,10 +276,12 @@ lqasc_rx_int(int irq, void *_port)
 {
 	unsigned long flags;
 	struct uart_port *port = (struct uart_port *)_port;
-	spin_lock_irqsave(&ltq_asc_lock, flags);
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	__raw_writel(ASC_IRNCR_RIR, port->membase + LTQ_ASC_IRNCR);
 	lqasc_rx_chars(port);
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	return IRQ_HANDLED;
 }
 

@@ -307,11 +315,13 @@ lqasc_startup(struct uart_port *port)
 {
 	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
 	int retval;
+	unsigned long flags;
 
 	if (!IS_ERR(ltq_port->clk))
 		clk_prepare_enable(ltq_port->clk);
 	port->uartclk = clk_get_rate(ltq_port->freqclk);
 
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
 		port->membase + LTQ_ASC_CLC);
 

@@ -331,6 +341,8 @@ lqasc_startup(struct uart_port *port)
 	asc_update_bits(0, ASCCON_M_8ASYNC | ASCCON_FEN | ASCCON_TOEN |
 		ASCCON_ROEN, port->membase + LTQ_ASC_CON);
 
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
+
 	retval = request_irq(ltq_port->tx_irq, lqasc_tx_int,
 		0, "asc_tx", port);
 	if (retval) {

@@ -367,15 +379,19 @@ static void
 lqasc_shutdown(struct uart_port *port)
 {
 	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+	unsigned long flags;
+
 	free_irq(ltq_port->tx_irq, port);
 	free_irq(ltq_port->rx_irq, port);
 	free_irq(ltq_port->err_irq, port);
 
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	__raw_writel(0, port->membase + LTQ_ASC_CON);
 	asc_update_bits(ASCRXFCON_RXFEN, ASCRXFCON_RXFFLU,
 		port->membase + LTQ_ASC_RXFCON);
 	asc_update_bits(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
 		port->membase + LTQ_ASC_TXFCON);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	if (!IS_ERR(ltq_port->clk))
 		clk_disable_unprepare(ltq_port->clk);
 }

@@ -390,6 +406,7 @@ lqasc_set_termios(struct uart_port *port,
 	unsigned int baud;
 	unsigned int con = 0;
 	unsigned long flags;
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
 
 	cflag = new->c_cflag;
 	iflag = new->c_iflag;

@@ -443,7 +460,7 @@ lqasc_set_termios(struct uart_port *port,
 	/* set error signals  - framing, parity  and overrun, enable receiver */
 	con |= ASCCON_FEN | ASCCON_TOEN | ASCCON_ROEN;
 
-	spin_lock_irqsave(&ltq_asc_lock, flags);
+	spin_lock_irqsave(&ltq_port->lock, flags);
 
 	/* set up CON */
 	asc_update_bits(0, con, port->membase + LTQ_ASC_CON);

@@ -471,7 +488,7 @@ lqasc_set_termios(struct uart_port *port,
 	/* enable rx */
 	__raw_writel(ASCWHBSTATE_SETREN, port->membase + LTQ_ASC_WHBSTATE);
 
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 
 	/* Don't rewrite B0 */
 	if (tty_termios_baud_rate(new))

@@ -589,17 +606,14 @@ lqasc_console_putchar(struct uart_port *port, int ch)
 static void lqasc_serial_port_write(struct uart_port *port, const char *s,
 				    u_int count)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ltq_asc_lock, flags);
 	uart_console_write(port, s, count, lqasc_console_putchar);
-	spin_unlock_irqrestore(&ltq_asc_lock, flags);
 }
 
 static void
 lqasc_console_write(struct console *co, const char *s, u_int count)
 {
 	struct ltq_uart_port *ltq_port;
+	unsigned long flags;
 
 	if (co->index >= MAXPORTS)
 		return;

@@ -608,7 +622,9 @@ lqasc_console_write(struct console *co, const char *s, u_int count)
 	if (!ltq_port)
 		return;
 
+	spin_lock_irqsave(&ltq_port->lock, flags);
 	lqasc_serial_port_write(&ltq_port->port, s, count);
+	spin_unlock_irqrestore(&ltq_port->lock, flags);
 }
 
 static int __init

@@ -766,6 +782,7 @@ lqasc_probe(struct platform_device *pdev)
 	ltq_port->rx_irq = irqres[1].start;
 	ltq_port->err_irq = irqres[2].start;
 
+	spin_lock_init(&ltq_port->lock);
 	lqasc_port[line] = ltq_port;
 	platform_set_drvdata(pdev, ltq_port);
Signature · 1 line
2.11.0
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#6 lore raw

Re: [PATCH 2/5] serial: lantiq: Use proper DT compatible string

In reply to a message not in the archive
Quoted 2 lines · Rahul Tanwar

The patch adds change to use explicit string instead of a macro for DT compatible string.

For consistency you may need to convert OF_EARLYCON_DECLARE() as well.

Perhaps commit message should explain the rationale, i.e. the following patches will add another compatible string and thus it makes sense to have them explicitly mentioned.

Quoted 22 lines
Suggested-by Andy Shevchenko <[email protected]>
Signed-off-by Rahul Tanwar <[email protected]>
---
 drivers/tty/serial/lantiq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/lantiq.c +1 -1
index 42e27b48e9cc..1116261c973e 100644
@@ -792,7 +792,7 @@ lqasc_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id ltq_asc_match[] = {
-	{ .compatible = DRVNAME },
+	{ .compatible = "lantiq,asc" },
 	{},
 };
Signature · 1 line
2.11.0
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#8 lore raw

Re: [PATCH 3/5] serial: lantiq: Make IRQ & ISR assignment dynamic

In reply to a message not in the archive

Review

Quoted 9 lines · Rahul Tanwar

This driver/IP is reused across multiple SoCs. Older SoCs supported three separate IRQs for tx, rx & err interrupts. Newer Lightning Mountain SoC supports single IRQ for all of tx/rx/err interrupts. This patch modifies the driver design to support dynamic assignment of IRQ resources & ISRs based on devicetree node compatible entries.

(quoted diff) +3 -0
+struct ltq_soc_data {
+	int	(*fetch_irq)(struct platform_device *pdev,
+				 struct ltq_uart_port *ltq_port);

This can be simple

	int	(*fetch_irq)(struct device *dev, struct ltq_uart_port *ltq_port);

(Note one line and struct device instead of platform_device)

Review

Quoted 6 lines
(quoted diff) +5 -0
+	int	(*request_irq)(struct uart_port *port);
+	void	(*free_irq)(struct uart_port *port);
+};
 
+	retval = ltq_port->soc->request_irq(port);
+	if(retval)

Space is missed.

Quoted 14 lines
(quoted diff) +11 -0
 		return retval;
 
 
+static int fetch_irq_lantiq(struct platform_device *pdev,
+			    struct ltq_uart_port *ltq_port)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct uart_port *port = &ltq_port->port;
+	struct resource irqres[3];
+	int ret;
+
+	ret = of_irq_to_resource_table(node, irqres, 3);
+	if (ret != 3) {
+		dev_err(&pdev->dev,

Review

Quoted 10 lines
(quoted diff) +9 -0
+			"failed to get IRQs for serial port\n");
+		return -ENODEV;
+	}
+	ltq_port->tx_irq = irqres[0].start;
+	ltq_port->rx_irq = irqres[1].start;
+	ltq_port->err_irq = irqres[2].start;
+	port->irq = irqres[0].start;
+
 
+	return ret;

I'm not sure you need to return known value. 0 will be good enough...

Review

Quoted 6 lines
(quoted diff) +4 -0
+}
 
+	ltq_port->soc = of_device_get_match_data(&pdev->dev);
+	ret = ltq_port->soc->fetch_irq(pdev, ltq_port);
 
+	if (ret < 0)

...and thus simple...

if (ret)

...may be used.

Quoted 1 line
(quoted diff) +1 -0
+		return ret;
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#10 lore raw

Re: [PATCH 5/5] serial: lantiq: Add support for Lightning Mountain SoC

In reply to a message not in the archive
Quoted 3 lines · Rahul Tanwar

This patch adds IRQ & ISR support in the driver for Lightning Mountain SoC.

+#define ASC_IRNCR_MASK 0x7

GENMASK() ?

Quoted 5 lines
(quoted diff) +5 -0
+static irqreturn_t lqasc_irq(int irq, void *p)
+{
+	unsigned long flags;
+	u32 stat;
+	struct uart_port *port = p;

Review

Quoted 10 lines
(quoted diff) +9 -0
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+
+	spin_lock_irqsave(&ltq_port->lock, flags);
+	stat = readl(port->membase + LTQ_ASC_IRNCR);
+	if (!(stat & ASC_IRNCR_MASK)) {
+		spin_unlock_irqrestore(&ltq_port->lock, flags);
+		return IRQ_NONE;
+	}
 
+	spin_unlock_irqrestore(&ltq_port->lock, flags);

Are you sure the below does not need a serialization?

If it's not the case, you may unlock the lock immediately after readl().

Quoted 15 lines
(quoted diff) +14 -0
+
+	if (stat & ASC_IRNCR_TIR)
+		lqasc_tx_int(irq, p);
+
+	if (stat & ASC_IRNCR_RIR)
+		lqasc_rx_int(irq, p);
+
+	if (stat & ASC_IRNCR_EIR)
+		lqasc_err_int(irq, p);
+
+	return IRQ_HANDLED;
+}
 
+static int fetch_irq_intel(struct platform_device *pdev,
+			   struct ltq_uart_port *ltq_port)

Review

Quoted 10 lines
(quoted diff) +9 -0
+{
+	struct uart_port *port = &ltq_port->port;
+	int ret;
+
+	ret = of_irq_get(pdev->dev.of_node, 0);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"failed to fetch IRQ for serial port\n");
 
+		return -ENODEV;

	return ret;

Review

Quoted 6 lines
(quoted diff) +5 -0
+	}
+	ltq_port->common_irq = ret;
+	port->irq = ret;
+
 
+	return ret;

Same as per patch 3, i.e.

	return 0;
Quoted 6 lines
(quoted diff) +5 -0
+}
 
+static int request_irq_intel(struct uart_port *port)
+{
+	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+	int retval;

Review

Quoted 10 lines
(quoted diff) +9 -0
+
+	retval = request_irq(ltq_port->common_irq, lqasc_irq, 0,
+			     "asc_irq", port);
 
+	if (retval) {
+		dev_err(port->dev, "failed to request asc_irq\n");
+		return retval;
+	}
+
+	return 0;

	if (retval)
		dev_err();

	return retval;

Review

Quoted 4 lines
(quoted diff) +3 -0
+}
 
+
+

One blank line is enough.

Signature · 2 lines
With Best Regards,
Andy Shevchenko