serial: mxs-auart: fix the wrong setting order

PATCH In review Huang Shijie 5 participants 12 messages Sep 6, 2012 – Sep 13, 2012

Subsystems (2)

Arm i.MX and Layerscape TTY layer and serial drivers

lore

Share & tools

lore

Huang Shijie

author

2012-09-06 14:36 UTC #0 lore raw

PATCH 1/2 serial: mxs-auart: fix the wrong setting order

After set the AUART_CTRL0_CLKGATE, the UART will gate all the clocks off. So the following line will not take effect.

       ................................................................
 	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
 			u->membase + AUART_INTR_CLR);
       ................................................................

To fix this issue, the patch moves this gate-off line to the end of setting registers.

Signed-off-by Huang Shijie <[email protected]>

---
 drivers/tty/serial/mxs-auart.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/tty/serial/mxs-auart.c +2 -2

index dafeef2..ea5f888 100644
@@ -457,11 +457,11 @@ static void mxs_auart_shutdown(struct uart_port *u)
 
 	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
 
-	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
-
 	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
 			u->membase + AUART_INTR_CLR);
 
+	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
+
 	clk_disable_unprepare(s->clk);
 }

Signature · 1 line

1.7.4.4

Huang Shijie

author

2012-09-06 14:37 UTC #1 lore raw

PATCH 2/2 serial: mxs-auart: put the device in mxs_auart_probe()

We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove().

Signed-off-by Huang Shijie <[email protected]>

---
 drivers/tty/serial/mxs-auart.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/tty/serial/mxs-auart.c +1 -0

index ea5f888..6898413 100644
@@ -796,6 +796,7 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	put_device(s->dev);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);

Signature · 1 line

1.7.4.4

Shawn Guo

2012-09-07 02:43 UTC #2 lore raw

Re: [PATCH 2/2] serial: mxs-auart: put the device in mxs_auart_probe()

Quoted 14 lines · Huang Shijie

We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove().
Signed-off-by Huang Shijie <[email protected]>
---
 drivers/tty/serial/mxs-auart.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/tty/serial/mxs-auart.c +0 -0
index ea5f888..6898413 100644

drivers/tty/serial/mxs-auart.c static int __devexit mxs_auart_remove(struct platform_device *pdev) · line 802

Quoted 7 lines

drivers/tty/serial/mxs-auart.c +1 -0

@@ -796,6 +796,7 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	put_device(s->dev);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);

So the error path of probe needs function fixing too, right?

Signature · 2 lines

Regards,
Shawn

Huang Shijie

2012-09-07 02:33 UTC #3 lore raw

PATCH 1/1 serial: mxs-auart: put the device when exit or error

We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove() or in the error path of probe.

Signed-off-by Huang Shijie <[email protected]>

---
 drivers/tty/serial/mxs-auart.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/tty/serial/mxs-auart.c +2 -0

index 3a667ee..203e512 100644
@@ -781,6 +781,7 @@ out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
 out_free_clk:
+	put_device(s->dev);
 	clk_put(s->clk);
 out_free:
 	kfree(s);

@@ -796,6 +797,7 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	put_device(s->dev);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);

Signature · 1 line

1.7.0.4

Shawn Guo

Acked-by on v1 1/1

2012-09-07 03:14 UTC #4 lore raw

Re: [PATCH] serial: mxs-auart: put the device when exit or error

Quoted 6 lines · Huang Shijie

We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove() or in the error path of probe.
Signed-off-by Huang Shijie <[email protected]>

Acked-by Shawn Guo <[email protected]>

Quoted 28 lines

---
 drivers/tty/serial/mxs-auart.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/tty/serial/mxs-auart.c +2 -0

index 3a667ee..203e512 100644
@@ -781,6 +781,7 @@ out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
 out_free_clk:
+	put_device(s->dev);
 	clk_put(s->clk);
 out_free:
 	kfree(s);

@@ -796,6 +797,7 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 
 	auart_port[pdev->id] = NULL;
 
+	put_device(s->dev);
 	clk_put(s->clk);
 	free_irq(s->irq, s);
 	kfree(s);

Signature · 1 line

1.7.0.4

Greg KH

maintainer

2012-09-10 23:00 UTC #5 lore raw

Re: [PATCH] serial: mxs-auart: put the device when exit or error

In reply to Huang Shijie

Quoted 9 lines · Huang Shijie

We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove() or in the error path of probe.
Signed-off-by Huang Shijie <[email protected]>
---
 drivers/tty/serial/mxs-auart.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

I already applied the previous version, so can you make up a patch against the tty-next tree that fixes it up properly based on this patch?

thanks,

greg k-h

Huang Shijie

2012-09-11 07:54 UTC #6 lore raw

PATCH 1/1 serial: mxs-auart: put the device in the error path

The mxs_auart_probe() gets the device by the get_device(). So we should put the device in the error path to balance the device's reference counter.

Signed-off-by Huang Shijie <[email protected]>

---
 drivers/tty/serial/mxs-auart.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/tty/serial/mxs-auart.c +1 -0

index 6898413..6db3baa 100644
@@ -781,6 +781,7 @@ out_free_irq:
 	auart_port[pdev->id] = NULL;
 	free_irq(s->irq, s);
 out_free_clk:
+	put_device(s->dev);
 	clk_put(s->clk);
 out_free:
 	kfree(s);

Signature · 1 line

1.7.0.4

Huang Shijie

2012-09-07 02:46 UTC #7 lore raw

Re: [PATCH 2/2] serial: mxs-auart: put the device in mxs_auart_probe()

In reply to Shawn Guo

Quoted 16 lines · Shawn Guo

Quoted 15 lines · Huang Shijie
We call the get_device() in the mxs_auart_probe(). For the balance of the reference count, we should put the device in the mxs_auart_remove().
Signed-off-by Huang Shijie<[email protected]>
---
  drivers/tty/serial/mxs-auart.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
drivers/tty/serial/mxs-auart.c +0 -0
index ea5f888..6898413 100644
@@ -796,6 +796,7 @@ static int __devexit mxs_auart_remove(struct platform_device *pdev)
 

Quoted 8 lines

drivers/tty/serial/mxs-auart.c static int __devexit mxs_auart_remove(struct platform_device *pdev) · line 802
Quoted 6 lines
drivers/tty/serial/mxs-auart.c +1 -0
  	auart_port[pdev->id] = NULL;
 
+	put_device(s->dev);
  	clk_put(s->clk);
  	free_irq(s->irq, s);
  	kfree(s);
So the error path of probe needs function fixing too, right?

OK.

Send out the new version right now.

Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Shawn Guo

Acked-by on v1 1/2

2012-09-07 02:38 UTC #8 lore raw

In reply to Huang Shijie

Quoted 11 lines · Huang Shijie

After set the AUART_CTRL0_CLKGATE, the UART will gate all the clocks off. So the following line will not take effect.
       ................................................................
drivers/tty/serial/mxs-auart.c +0 -0
 	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
 			u->membase + AUART_INTR_CLR);
................................................................
To fix this issue, the patch moves this gate-off line to the end of setting registers.
Signed-off-by Huang Shijie <[email protected]>

Acked-by Shawn Guo <[email protected]>

Are you experiencing any user visible problem with this bug?

Regards,
Shawn

Quoted 25 lines

---
 drivers/tty/serial/mxs-auart.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/tty/serial/mxs-auart.c +2 -2

index dafeef2..ea5f888 100644
@@ -457,11 +457,11 @@ static void mxs_auart_shutdown(struct uart_port *u)
 
 	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
 
-	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
-
 	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
 			u->membase + AUART_INTR_CLR);
 
+	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
+
 	clk_disable_unprepare(s->clk);
 }

Signature · 1 line

1.7.4.4

Huang Shijie

2012-09-07 02:41 UTC #9 lore raw

Quoted 8 lines · Shawn Guo

Quoted 7 lines · Huang Shijie
After set the AUART_CTRL0_CLKGATE, the UART will gate all the clocks off. So the following line will not take effect.
        ................................................................
  	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
  			u->membase + AUART_INTR_CLR);
        ................................................................

Quoted 8 lines

Quoted 4 lines
To fix this issue, the patch moves this gate-off line to the end of setting registers.
Signed-off-by Huang Shijie<[email protected]>
Acked-by Shawn Guo<[email protected]>
Are you experiencing any user visible problem with this bug?

I do not experience any user problem.
I found this bug when i tried to add DMA support for MXS-AUART.

Best Regards
Huang Shijie

Quoted 25 lines

Regards,
Shawn

Quoted 22 lines

---
  drivers/tty/serial/mxs-auart.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

drivers/tty/serial/mxs-auart.c +2 -2

index dafeef2..ea5f888 100644
@@ -457,11 +457,11 @@ static void mxs_auart_shutdown(struct uart_port *u)
 
  	writel(AUART_CTRL2_UARTEN, u->membase + AUART_CTRL2_CLR);
 
-	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
-
  	writel(AUART_INTR_RXIEN | AUART_INTR_RTIEN | AUART_INTR_CTSMIEN,
  			u->membase + AUART_INTR_CLR);
 
+	writel(AUART_CTRL0_CLKGATE, u->membase + AUART_CTRL0_SET);
+
  	clk_disable_unprepare(s->clk);
  }

Quoted 8 lines

Quoted 3 lines
Signature · 1 line
1.7.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Lauri Hintsala

2012-09-13 05:28 UTC #10 lore raw

PATCH 1/2 serial: mxs-auart: fix the wrong setting order

Hi Huang,

Quoted 1 line · Huang Shijie

I found this bug when i tried to add DMA support for MXS-AUART.

What is the status of DMA support? We could help you testing your patches.

Best regards,
Lauri Hintsala

Huang Shijie

2012-09-13 05:42 UTC #11 lore raw

PATCH 1/2 serial: mxs-auart: fix the wrong setting order

Quoted 6 lines · Lauri Hintsala

Hi Huang,
Quoted 1 line · Huang Shijie
I found this bug when i tried to add DMA support for MXS-AUART.
What is the status of DMA support? We could help you testing your patches.

thanks.

I maybe send out the new patch set in next week. I resume this work recently.

BR
Huang Shijie

Quoted 4 lines

Best regards,
Lauri Hintsala

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html