[PATCHv2 0/4] serial: mxs-auart: devm conversion, clock rework, and IRQ ordering fixes

[PATCHv2 0/4] serial: mxs-auart: devm conversion, clock rework, and IRQ ordering fixes

[Rosen Penev]

This series cleans up the mxs-auart driver by converting to devm-managed
resources, fixing clock prepare/enable ordering, and addressing IRQ
registration races.

Patch 1 fixes compilation on 64-bit build with W=1

Patch 2 reworks the clock handling to use devm_clk_get_enabled and
reorders clk_prepare_enable after clk_set_rate to avoid
CLK_SET_RATE_GATE failures.

Patch 3 converts iomem mapping and GPIO IRQ requests to devm,
removing the manual cleanup paths.

Patch 4 moves the main UART IRQ registration after uart_add_one_port
so the port state is initialized before the handler can run, and
manages the module clock for console vs non-console ports correctly.

v2: split off of_device_get_match_data change.

Rosen Penev (4):
  serial: mxs-auart: fix cast type for of_device_get_match_data
  serial: mxs-auart: rework clock handling in mxs_get_clks and probe
  serial: mxs-auart: use devm resources for iomem and GPIO IRQs
  serial: mxs-auart: fix IRQ registration ordering and manage console
    clock

 drivers/tty/serial/mxs-auart.c | 141 +++++++++++++++------------------
 1 file changed, 63 insertions(+), 78 deletions(-)

--
2.54.0

[PATCHv2 1/4] serial: mxs-auart: fix cast type for of_device_get_match_data

[Rosen Penev]

of_device_get_match_data returns const void*. Cast to unsigned long to
avoid implicit integer truncation warnings. All the data parameters are
correct anyway.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/tty/serial/mxs-auart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 697318dbb146..de97c0f74e7d 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
+	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
 
 	ret = mxs_get_clks(s, pdev);
 	if (ret)
-- 
2.54.0

[PATCHv2 2/4] serial: mxs-auart: rework clock handling in mxs_get_clks and probe

[Rosen Penev]

Use devm_clk_get_enabled for the AHB clock so its enable/disable
lifetime is managed by the driver model. Move the mod clock
(clk) prepare_enable out of mxs_get_clks and into probe so that
clk_set_rate is called while the clock is still disabled, avoiding
CLK_SET_RATE_GATE failures. Clean up the error labels accordingly.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/tty/serial/mxs-auart.c | 47 ++++++++++++----------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index de97c0f74e7d..aa59a48bfad7 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1470,34 +1470,22 @@ static int mxs_get_clks(struct mxs_auart_port *s,
 		return PTR_ERR(s->clk);
 	}
 
-	s->clk_ahb = devm_clk_get(s->dev, "ahb");
+	s->clk_ahb = devm_clk_get_enabled(s->dev, "ahb");
 	if (IS_ERR(s->clk_ahb)) {
 		dev_err(s->dev, "Failed to get \"ahb\" clk\n");
 		return PTR_ERR(s->clk_ahb);
 	}
 
-	err = clk_prepare_enable(s->clk_ahb);
-	if (err) {
-		dev_err(s->dev, "Failed to enable ahb_clk!\n");
-		return err;
-	}
-
+	/*
+	 * Set mod clock rate while it is still disabled so
+	 * CLK_SET_RATE_GATE does not cause clk_set_rate to fail.
+	 * The mod clock will be enabled in mxs_auart_startup()
+	 * and in probe after mxs_get_clks returns.
+	 */
 	err = clk_set_rate(s->clk, clk_get_rate(s->clk_ahb));
-	if (err) {
+	if (err)
 		dev_err(s->dev, "Failed to set rate!\n");
-		goto disable_clk_ahb;
-	}
 
-	err = clk_prepare_enable(s->clk);
-	if (err) {
-		dev_err(s->dev, "Failed to enable clk!\n");
-		goto disable_clk_ahb;
-	}
-
-	return 0;
-
-disable_clk_ahb:
-	clk_disable_unprepare(s->clk_ahb);
 	return err;
 }
 
@@ -1604,17 +1592,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = clk_prepare_enable(s->clk);
+	if (ret)
+		return ret;
+
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
 		ret = -ENXIO;
-		goto out_disable_clks;
+		goto out_disable_clk;
 	}
 
 	s->port.mapbase = r->start;
 	s->port.membase = ioremap(r->start, resource_size(r));
 	if (!s->port.membase) {
 		ret = -ENOMEM;
-		goto out_disable_clks;
+		goto out_disable_clk;
 	}
 	s->port.ops = &mxs_auart_ops;
 	s->port.iotype = UPIO_MEM;
@@ -1681,11 +1673,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
 out_iounmap:
 	iounmap(s->port.membase);
 
-out_disable_clks:
-	if (is_asm9260_auart(s)) {
-		clk_disable_unprepare(s->clk);
-		clk_disable_unprepare(s->clk_ahb);
-	}
+out_disable_clk:
+	clk_disable_unprepare(s->clk);
 	return ret;
 }
 
@@ -1697,10 +1686,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
 	auart_port[pdev->id] = NULL;
 	mxs_auart_free_gpio_irq(s);
 	iounmap(s->port.membase);
-	if (is_asm9260_auart(s)) {
+	if (is_asm9260_auart(s))
 		clk_disable_unprepare(s->clk);
-		clk_disable_unprepare(s->clk_ahb);
-	}
 }
 
 static struct platform_driver mxs_auart_driver = {
-- 
2.54.0

[PATCHv2 3/4] serial: mxs-auart: use devm resources for iomem and GPIO IRQs

[Rosen Penev]

Replace platform_get_resource + ioremap with
devm_platform_get_and_ioremap_resource and convert GPIO IRQ
request_irq/free_irq to devm_request_irq. This eliminates the
mxs_auart_free_gpio_irq function and its call sites, and the
out_iounmap error label. Simplify the remove function accordingly.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/tty/serial/mxs-auart.c | 53 +++++++---------------------------
 1 file changed, 11 insertions(+), 42 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index aa59a48bfad7..4499e3206e85 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1517,15 +1517,6 @@ static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
 	return 0;
 }
 
-static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
-{
-	enum mctrl_gpio_idx i;
-
-	for (i = 0; i < UART_GPIO_MAX; i++)
-		if (s->gpio_irq[i] >= 0)
-			free_irq(s->gpio_irq[i], s);
-}
-
 static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
 {
 	int *irq = s->gpio_irq;
@@ -1537,21 +1528,13 @@ static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
 			continue;
 
 		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
-		err = request_irq(irq[i], mxs_auart_irq_handle,
-				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
+		err = devm_request_irq(s->dev, irq[i], mxs_auart_irq_handle,
+				       IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
 		if (err)
 			dev_err(s->dev, "%s - Can't get %d irq\n",
 				__func__, irq[i]);
 	}
 
-	/*
-	 * If something went wrong, rollback.
-	 * Be careful: i may be unsigned.
-	 */
-	while (err && (i-- > 0))
-		if (irq[i] >= 0)
-			free_irq(irq[i], s);
-
 	return err;
 }
 
@@ -1596,18 +1579,12 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!r) {
-		ret = -ENXIO;
+	s->port.membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
+	if (IS_ERR(s->port.membase)) {
+		ret = PTR_ERR(s->port.membase);
 		goto out_disable_clk;
 	}
-
 	s->port.mapbase = r->start;
-	s->port.membase = ioremap(r->start, resource_size(r));
-	if (!s->port.membase) {
-		ret = -ENOMEM;
-		goto out_disable_clk;
-	}
 	s->port.ops = &mxs_auart_ops;
 	s->port.iotype = UPIO_MEM;
 	s->port.fifosize = MXS_AUART_FIFO_SIZE;
@@ -1622,21 +1599,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		ret = irq;
-		goto out_iounmap;
+		goto out_disable_clk;
 	}
 
 	s->port.irq = irq;
 	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
 			       dev_name(&pdev->dev), s);
 	if (ret)
-		goto out_iounmap;
+		goto out_disable_clk;
 
 	platform_set_drvdata(pdev, s);
 
 	ret = mxs_auart_init_gpios(s, &pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize GPIOs.\n");
-		goto out_iounmap;
+		goto out_disable_clk;
 	}
 
 	/*
@@ -1644,7 +1621,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	 */
 	ret = mxs_auart_request_gpio_irq(s);
 	if (ret)
-		goto out_iounmap;
+		goto out_disable_clk;
 
 	auart_port[s->port.line] = s;
 
@@ -1667,11 +1644,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	return 0;
 
 out_free_qpio_irq:
-	mxs_auart_free_gpio_irq(s);
-	auart_port[pdev->id] = NULL;
-
-out_iounmap:
-	iounmap(s->port.membase);
+	auart_port[s->port.line] = NULL;
 
 out_disable_clk:
 	clk_disable_unprepare(s->clk);
@@ -1683,11 +1656,7 @@ static void mxs_auart_remove(struct platform_device *pdev)
 	struct mxs_auart_port *s = platform_get_drvdata(pdev);
 
 	uart_remove_one_port(&auart_driver, &s->port);
-	auart_port[pdev->id] = NULL;
-	mxs_auart_free_gpio_irq(s);
-	iounmap(s->port.membase);
-	if (is_asm9260_auart(s))
-		clk_disable_unprepare(s->clk);
+	auart_port[s->port.line] = NULL;
 }
 
 static struct platform_driver mxs_auart_driver = {
-- 
2.54.0

[PATCHv2 4/4] serial: mxs-auart: fix IRQ registration ordering and manage console clock

[Rosen Penev]

Move the main UART IRQ registration after uart_add_one_port so that
s->port.state and s->port.lock are initialized before the interrupt
handler can run. Mask all UART interrupts before adding the port to
prevent spurious IRQs left by the bootloader.

After probe succeeds, disable the module clock for non-console ports
since startup will re-enable it on port open. For console ports, keep
the clock prepared so auart_console_write() can safely call
clk_enable() from atomic context.

Guard the IRQ handler and get_mctrl with clk_enable/clk_disable since
GPIO IRQs and serial-core status queries can fire while the clock is
disabled for non-console ports.

In remove, disable the clock for console ports to balance the enable
done in probe, preventing a clock leak on unbind.

Assisted-by: opencode:big-pickle
---
 drivers/tty/serial/mxs-auart.c | 49 +++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 4499e3206e85..e2b656638ab3 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -738,9 +738,13 @@ static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
 static u32 mxs_auart_get_mctrl(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
-	u32 stat = mxs_read(s, REG_STAT);
+	u32 stat;
 	u32 mctrl = 0;
 
+	clk_enable(s->clk);
+	stat = mxs_read(s, REG_STAT);
+	clk_disable(s->clk);
+
 	if (stat & AUART_STAT_CTS)
 		mctrl |= TIOCM_CTS;
 
@@ -1079,6 +1083,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 	struct mxs_auart_port *s = context;
 	u32 mctrl_temp = s->mctrl_prev;
 
+	clk_enable(s->clk);
 	uart_port_lock(&s->port);
 
 	stat = mxs_read(s, REG_STAT);
@@ -1118,6 +1123,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
 	}
 
 	uart_port_unlock(&s->port);
+	clk_disable(s->clk);
 
 	return IRQ_HANDLED;
 }
@@ -1603,10 +1609,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	}
 
 	s->port.irq = irq;
-	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
-			       dev_name(&pdev->dev), s);
-	if (ret)
-		goto out_disable_clk;
 
 	platform_set_drvdata(pdev, s);
 
@@ -1627,9 +1629,28 @@ static int mxs_auart_probe(struct platform_device *pdev)
 
 	mxs_auart_reset_deassert(s);
 
+	/* Mask all UART interrupts to prevent spurious IRQs from bootloader */
+	mxs_write(0, s, REG_INTR);
+
 	ret = uart_add_one_port(&auart_driver, &s->port);
-	if (ret)
-		goto out_free_qpio_irq;
+	if (ret) {
+		auart_port[s->port.line] = NULL;
+		goto out_disable_clk;
+	}
+
+	/*
+	 * Request the main IRQ after uart_add_one_port so that
+	 * s->port.state and s->port.lock are initialized before
+	 * the handler can run in response to a bootloader-left
+	 * interrupt.
+	 */
+	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
+			       dev_name(&pdev->dev), s);
+	if (ret) {
+		uart_remove_one_port(&auart_driver, &s->port);
+		auart_port[s->port.line] = NULL;
+		goto out_disable_clk;
+	}
 
 	/* ASM9260 don't have version reg */
 	if (is_asm9260_auart(s)) {
@@ -1641,10 +1662,16 @@ static int mxs_auart_probe(struct platform_device *pdev)
 			 (version >> 16) & 0xff, version & 0xffff);
 	}
 
-	return 0;
+	/*
+	 * Disable clock -- startup will re-enable when the port is opened.
+	 * For the console port the clock must stay prepared so that
+	 * auart_console_write() can safely call clk_enable() from
+	 * atomic context.
+	 */
+	if (!uart_console(&s->port))
+		clk_disable_unprepare(s->clk);
 
-out_free_qpio_irq:
-	auart_port[s->port.line] = NULL;
+	return 0;
 
 out_disable_clk:
 	clk_disable_unprepare(s->clk);
@@ -1657,6 +1684,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
 
 	uart_remove_one_port(&auart_driver, &s->port);
 	auart_port[s->port.line] = NULL;
+	if (uart_console(&s->port))
+		clk_disable_unprepare(s->clk);
 }
 
 static struct platform_driver mxs_auart_driver = {
-- 
2.54.0

Re: [PATCHv2 1/4] serial: mxs-auart: fix cast type for of_device_get_match_data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds array write to negative index (-1) due to using pdev->id.
- [High] Use-after-unmap race condition due to mixing devm_request_irq with manual iounmap.
--

commit 65b6a79a32694ff06e9c834fc282d2c8989565ff
Author: Rosen Penev <rosenp@gmail.com>

serial: mxs-auart: fix cast type for of_device_get_match_data

of_device_get_match_data returns const void*. Cast to unsigned long to
avoid implicit integer truncation warnings. All the data parameters are
correct anyway.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 693b491f1e755..3e14c2768c48e 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> +	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
>  
>  	ret = mxs_get_clks(s, pdev);
>  	if (ret)

[ ... ]

>  out_free_qpio_irq:
>  	mxs_auart_free_gpio_irq(s);
>  	auart_port[pdev->id] = NULL;

[Severity: High]
This isn't a bug introduced by this patch, but does this result in an
out-of-bounds write to a negative index?

For devices instantiated via Device Tree, pdev->id is initialized to
PLATFORM_DEVID_NONE (-1).

The driver correctly registers the port using auart_port[s->port.line] = s;
earlier in probe, but on the error path it attempts to clean up using
auart_port[pdev->id] = NULL;.

Could this corrupt the adjacent global memory in the BSS/Data section? It also
seems to fail to clear the actual dangling pointer at
auart_port[s->port.line], which might lead to a use-after-free later.

>  
>  out_iounmap:
>  	iounmap(s->port.membase);

[Severity: High]
This is a pre-existing issue, but does this manual iounmap() create a
use-after-unmap race condition?

The driver requests the main UART interrupt using devm_request_irq().
Devm-managed resources are freed automatically, but only after the probe
function returns.

Since the memory is manually unmapped here, there is a window where the
interrupt handler mxs_auart_irq_handle() remains registered and active.

If a spurious interrupt fires during this window, the handler could execute an
MMIO read via mxs_read(s, REG_STAT) on the unmapped memory, potentially causing
a kernel page fault.

[ ... ]

>  static void mxs_auart_remove(struct platform_device *pdev)
>  {
>  	struct mxs_auart_port *s = platform_get_drvdata(pdev);
>  
>  	uart_remove_one_port(&auart_driver, &s->port);
>  	auart_port[pdev->id] = NULL;

[Severity: High]
This is a pre-existing issue, but doesn't this also result in the same
out-of-bounds write to a negative index as seen in the probe error path?

>  	mxs_auart_free_gpio_irq(s);
>  	iounmap(s->port.membase);

[Severity: High]
This isn't a bug introduced by this patch, but does this create the same
use-after-unmap race condition during device removal?

Since the devm subsystem cleans up the IRQ handler after remove() completes,
manually unmapping the memory here leaves a window where an active interrupt
could trigger an MMIO read on the unmapped memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609223717.41670-2-rosenp@gmail.com?part=1

Re: [PATCHv2 2/4] serial: mxs-auart: rework clock handling in mxs_get_clks and probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Unconditional clock enablement in `probe` for non-ASM9260 devices causes a resource leak on driver unbind and defeats dynamic clock gating.

Pre-existing issues:
- [Critical] Out-of-bounds array write in error and remove paths due to mismatched array indexing (`pdev->id` vs `s->port.line`).
- [Critical] Missing bounds check on hardware-provided RX count leads to an out-of-bounds read and information leak.
- [High] Missing explicit termination of in-flight DMA transfers causes Use-After-Free during shutdown/error.
--

commit 610c2d30ee113ae6d3222a8a5d42e3dc565ef441
Author: Rosen Penev <rosenp@gmail.com>

serial: mxs-auart: rework clock handling in mxs_get_clks and probe

This patch reworks the clock handling in the mxs-auart driver. It uses
devm_clk_get_enabled() for the AHB clock so its enable/disable lifetime
is managed by the driver model. It also moves the mod clock preparation
and enablement out of mxs_get_clks() and into the probe function.

> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 693b491f1e755..8d050ceb72d2f 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c

[ ... ]

> @@ -1604,17 +1592,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = clk_prepare_enable(s->clk);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
This patch moves clk_prepare_enable(s->clk) from the ASM9260-specific portion
of mxs_get_clks() to mxs_auart_probe(), which causes it to execute for all
device variants.

However, mxs_auart_remove() was not updated to unconditionally disable the
clock. Does this leak the clock reference for non-ASM9260 devices on unbind?

Additionally, by holding a baseline clock reference from probe to remove,
might the clock's reference count never reach zero when the UART port is
closed? This would unconditionally keep the clock running and defeat the
driver's dynamic clock gating logic in mxs_auart_startup() and
mxs_auart_shutdown(), potentially preventing SoC deep sleep states.

[ ... ]

> @@ -1697,10 +1686,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
>  	auart_port[pdev->id] = NULL;

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential out-of-bounds array
write here.

During registration in mxs_auart_probe(), the allocated port pointer is
saved using the device tree alias index:

    auart_port[s->port.line] = s;

However, in the out_free_qpio_irq error path and here in mxs_auart_remove(),
the code clears the slot using auart_port[pdev->id] = NULL.

For devices instantiated from a Device Tree, pdev->id is typically -1
(PLATFORM_DEVID_NONE). Could writing to index -1 cause an out-of-bounds
memory write in the BSS segment immediately preceding the auart_port array?
Also, does this leave the actual active pointer dangling instead of clearing
it?

>  	mxs_auart_free_gpio_irq(s);
>  	iounmap(s->port.membase);
> -	if (is_asm9260_auart(s)) {
> +	if (is_asm9260_auart(s))
>  		clk_disable_unprepare(s->clk);
> -		clk_disable_unprepare(s->clk_ahb);
> -	}
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
during shutdown or error handling in mxs_auart_dma_exit_channel():

    if (s->tx_dma_chan) {
        dma_release_channel(s->tx_dma_chan);
        s->tx_dma_chan = NULL;
    }
    ...
    kfree(s->tx_dma_buf);
    kfree(s->rx_dma_buf);

The driver fails to call dmaengine_terminate_sync() before releasing the
channels. Since releasing a channel does not safely abort active transfers,
if an asynchronous DMA transfer is in flight, could the DMA controller
continue operating and execute the DMA callback after the buffers have been
freed?

[Severity: Critical]
This is also a pre-existing issue, but does dma_rx_callback() lack a bounds
check on the hardware-provided RX count?

    count = stat & AUART_STAT_RXCOUNT_MASK;
    tty_insert_flip_string(port, s->rx_dma_buf, count);

The mask is 0xFFFF, allowing values up to 65535. The driver passes this
unfiltered count to tty_insert_flip_string(), which copies count bytes
from s->rx_dma_buf. 

Since s->rx_dma_buf is allocated with a fixed size of UART_XMIT_SIZE
(4096 bytes), could a hardware fault or a compromised peripheral reporting
a count larger than 4096 cause an out-of-bounds read, potentially copying
up to 61KB of kernel memory to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609223717.41670-3-rosenp@gmail.com?part=1

Re: [PATCHv2 1/4] serial: mxs-auart: fix cast type for of_device_get_match_data

[Frank Li]

On Tue, Jun 09, 2026 at 03:37:14PM -0700, Rosen Penev wrote:
> of_device_get_match_data returns const void*. Cast to unsigned long to
> avoid implicit integer truncation warnings. All the data parameters are
> correct anyway.

It is not worth to fix it by this ways. cast void * to int/long is not good
at all.

struct drvdata
{
	enum mxs_auart_type type;
}

static const struct drvata chip_imx28;

&chip_imx28 as drv data.

Frank
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/tty/serial/mxs-auart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 697318dbb146..de97c0f74e7d 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>
> -	s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> +	s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
>
>  	ret = mxs_get_clks(s, pdev);
>  	if (ret)
> --
> 2.54.0
>
>

Re: [PATCHv2 2/4] serial: mxs-auart: rework clock handling in mxs_get_clks and probe

[Frank Li]

On Tue, Jun 09, 2026 at 03:37:15PM -0700, Rosen Penev wrote:
> Use devm_clk_get_enabled for the AHB clock so its enable/disable
> lifetime is managed by the driver model. Move the mod clock
> (clk) prepare_enable out of mxs_get_clks and into probe so that
> clk_set_rate is called while the clock is still disabled, avoiding
> CLK_SET_RATE_GATE failures. Clean up the error labels accordingly.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/tty/serial/mxs-auart.c | 47 ++++++++++++----------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index de97c0f74e7d..aa59a48bfad7 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1470,34 +1470,22 @@ static int mxs_get_clks(struct mxs_auart_port *s,
>  		return PTR_ERR(s->clk);
>  	}
>
> -	s->clk_ahb = devm_clk_get(s->dev, "ahb");
> +	s->clk_ahb = devm_clk_get_enabled(s->dev, "ahb");
>  	if (IS_ERR(s->clk_ahb)) {
>  		dev_err(s->dev, "Failed to get \"ahb\" clk\n");
>  		return PTR_ERR(s->clk_ahb);
>  	}
>
> -	err = clk_prepare_enable(s->clk_ahb);
> -	if (err) {
> -		dev_err(s->dev, "Failed to enable ahb_clk!\n");
> -		return err;
> -	}
> -
> +	/*
> +	 * Set mod clock rate while it is still disabled so
> +	 * CLK_SET_RATE_GATE does not cause clk_set_rate to fail.
> +	 * The mod clock will be enabled in mxs_auart_startup()
> +	 * and in probe after mxs_get_clks returns.
> +	 */
>  	err = clk_set_rate(s->clk, clk_get_rate(s->clk_ahb));
> -	if (err) {
> +	if (err)
>  		dev_err(s->dev, "Failed to set rate!\n");
> -		goto disable_clk_ahb;
> -	}
>
> -	err = clk_prepare_enable(s->clk);
> -	if (err) {
> -		dev_err(s->dev, "Failed to enable clk!\n");
> -		goto disable_clk_ahb;
> -	}
> -
> -	return 0;
> -
> -disable_clk_ahb:
> -	clk_disable_unprepare(s->clk_ahb);

if I understand correct, after apply patch,

	if (err)
		dev_err(s->dev, ...)

	return err;


Perfer method is

	if (err)
		return dev_err_probe(s->dev, err, ...);

	return 0;

Frank

>  	return err;
>  }
>
> @@ -1604,17 +1592,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	ret = clk_prepare_enable(s->clk);
> +	if (ret)
> +		return ret;
> +
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r) {
>  		ret = -ENXIO;
> -		goto out_disable_clks;
> +		goto out_disable_clk;
>  	}
>
>  	s->port.mapbase = r->start;
>  	s->port.membase = ioremap(r->start, resource_size(r));
>  	if (!s->port.membase) {
>  		ret = -ENOMEM;
> -		goto out_disable_clks;
> +		goto out_disable_clk;
>  	}
>  	s->port.ops = &mxs_auart_ops;
>  	s->port.iotype = UPIO_MEM;
> @@ -1681,11 +1673,8 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  out_iounmap:
>  	iounmap(s->port.membase);
>
> -out_disable_clks:
> -	if (is_asm9260_auart(s)) {
> -		clk_disable_unprepare(s->clk);
> -		clk_disable_unprepare(s->clk_ahb);
> -	}
> +out_disable_clk:
> +	clk_disable_unprepare(s->clk);
>  	return ret;
>  }
>
> @@ -1697,10 +1686,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
>  	auart_port[pdev->id] = NULL;
>  	mxs_auart_free_gpio_irq(s);
>  	iounmap(s->port.membase);
> -	if (is_asm9260_auart(s)) {
> +	if (is_asm9260_auart(s))
>  		clk_disable_unprepare(s->clk);
> -		clk_disable_unprepare(s->clk_ahb);
> -	}
>  }
>
>  static struct platform_driver mxs_auart_driver = {
> --
> 2.54.0
>
>

Re: [PATCHv2 3/4] serial: mxs-auart: use devm resources for iomem and GPIO IRQs

[Frank Li]

On Tue, Jun 09, 2026 at 03:37:16PM -0700, Rosen Penev wrote:
> Replace platform_get_resource + ioremap with
> devm_platform_get_and_ioremap_resource and convert GPIO IRQ
> request_irq/free_irq to devm_request_irq. This eliminates the
> mxs_auart_free_gpio_irq function and its call sites, and the
> out_iounmap error label. Simplify the remove function accordingly.

as my said before, all function need add ()

Frank
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/tty/serial/mxs-auart.c | 53 +++++++---------------------------
>  1 file changed, 11 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index aa59a48bfad7..4499e3206e85 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -1517,15 +1517,6 @@ static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
>  	return 0;
>  }
>
> -static void mxs_auart_free_gpio_irq(struct mxs_auart_port *s)
> -{
> -	enum mctrl_gpio_idx i;
> -
> -	for (i = 0; i < UART_GPIO_MAX; i++)
> -		if (s->gpio_irq[i] >= 0)
> -			free_irq(s->gpio_irq[i], s);
> -}
> -
>  static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
>  {
>  	int *irq = s->gpio_irq;
> @@ -1537,21 +1528,13 @@ static int mxs_auart_request_gpio_irq(struct mxs_auart_port *s)
>  			continue;
>
>  		irq_set_status_flags(irq[i], IRQ_NOAUTOEN);
> -		err = request_irq(irq[i], mxs_auart_irq_handle,
> -				IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
> +		err = devm_request_irq(s->dev, irq[i], mxs_auart_irq_handle,
> +				       IRQ_TYPE_EDGE_BOTH, dev_name(s->dev), s);
>  		if (err)
>  			dev_err(s->dev, "%s - Can't get %d irq\n",
>  				__func__, irq[i]);
>  	}
>
> -	/*
> -	 * If something went wrong, rollback.
> -	 * Be careful: i may be unsigned.
> -	 */
> -	while (err && (i-- > 0))
> -		if (irq[i] >= 0)
> -			free_irq(irq[i], s);
> -
>  	return err;
>  }
>
> @@ -1596,18 +1579,12 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> -	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!r) {
> -		ret = -ENXIO;
> +	s->port.membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
> +	if (IS_ERR(s->port.membase)) {
> +		ret = PTR_ERR(s->port.membase);
>  		goto out_disable_clk;
>  	}
> -
>  	s->port.mapbase = r->start;
> -	s->port.membase = ioremap(r->start, resource_size(r));
> -	if (!s->port.membase) {
> -		ret = -ENOMEM;
> -		goto out_disable_clk;
> -	}
>  	s->port.ops = &mxs_auart_ops;
>  	s->port.iotype = UPIO_MEM;
>  	s->port.fifosize = MXS_AUART_FIFO_SIZE;
> @@ -1622,21 +1599,21 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		ret = irq;
> -		goto out_iounmap;
> +		goto out_disable_clk;
>  	}
>
>  	s->port.irq = irq;
>  	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
>  			       dev_name(&pdev->dev), s);
>  	if (ret)
> -		goto out_iounmap;
> +		goto out_disable_clk;
>
>  	platform_set_drvdata(pdev, s);
>
>  	ret = mxs_auart_init_gpios(s, &pdev->dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to initialize GPIOs.\n");
> -		goto out_iounmap;
> +		goto out_disable_clk;
>  	}
>
>  	/*
> @@ -1644,7 +1621,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	 */
>  	ret = mxs_auart_request_gpio_irq(s);
>  	if (ret)
> -		goto out_iounmap;
> +		goto out_disable_clk;
>
>  	auart_port[s->port.line] = s;
>
> @@ -1667,11 +1644,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	return 0;
>
>  out_free_qpio_irq:
> -	mxs_auart_free_gpio_irq(s);
> -	auart_port[pdev->id] = NULL;
> -
> -out_iounmap:
> -	iounmap(s->port.membase);
> +	auart_port[s->port.line] = NULL;
>
>  out_disable_clk:
>  	clk_disable_unprepare(s->clk);
> @@ -1683,11 +1656,7 @@ static void mxs_auart_remove(struct platform_device *pdev)
>  	struct mxs_auart_port *s = platform_get_drvdata(pdev);
>
>  	uart_remove_one_port(&auart_driver, &s->port);
> -	auart_port[pdev->id] = NULL;
> -	mxs_auart_free_gpio_irq(s);
> -	iounmap(s->port.membase);
> -	if (is_asm9260_auart(s))
> -		clk_disable_unprepare(s->clk);
> +	auart_port[s->port.line] = NULL;
>  }
>
>  static struct platform_driver mxs_auart_driver = {
> --
> 2.54.0
>
>

Re: [PATCHv2 4/4] serial: mxs-auart: fix IRQ registration ordering and manage console clock

[Frank Li]

On Tue, Jun 09, 2026 at 03:37:17PM -0700, Rosen Penev wrote:
> Move the main UART IRQ registration after uart_add_one_port so that
> s->port.state and s->port.lock are initialized before the interrupt
> handler can run. Mask all UART interrupts before adding the port to
> prevent spurious IRQs left by the bootloader.

Please use seperate patch to this problem only.

Frank

>
> After probe succeeds, disable the module clock for non-console ports
> since startup will re-enable it on port open. For console ports, keep
> the clock prepared so auart_console_write() can safely call
> clk_enable() from atomic context.
>
> Guard the IRQ handler and get_mctrl with clk_enable/clk_disable since
> GPIO IRQs and serial-core status queries can fire while the clock is
> disabled for non-console ports.
>
> In remove, disable the clock for console ports to balance the enable
> done in probe, preventing a clock leak on unbind.
>
> Assisted-by: opencode:big-pickle
> ---
>  drivers/tty/serial/mxs-auart.c | 49 +++++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> index 4499e3206e85..e2b656638ab3 100644
> --- a/drivers/tty/serial/mxs-auart.c
> +++ b/drivers/tty/serial/mxs-auart.c
> @@ -738,9 +738,13 @@ static u32 mxs_auart_modem_status(struct mxs_auart_port *s, u32 mctrl)
>  static u32 mxs_auart_get_mctrl(struct uart_port *u)
>  {
>  	struct mxs_auart_port *s = to_auart_port(u);
> -	u32 stat = mxs_read(s, REG_STAT);
> +	u32 stat;
>  	u32 mctrl = 0;
>
> +	clk_enable(s->clk);
> +	stat = mxs_read(s, REG_STAT);
> +	clk_disable(s->clk);
> +
>  	if (stat & AUART_STAT_CTS)
>  		mctrl |= TIOCM_CTS;
>
> @@ -1079,6 +1083,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>  	struct mxs_auart_port *s = context;
>  	u32 mctrl_temp = s->mctrl_prev;
>
> +	clk_enable(s->clk);
>  	uart_port_lock(&s->port);
>
>  	stat = mxs_read(s, REG_STAT);
> @@ -1118,6 +1123,7 @@ static irqreturn_t mxs_auart_irq_handle(int irq, void *context)
>  	}
>
>  	uart_port_unlock(&s->port);
> +	clk_disable(s->clk);
>
>  	return IRQ_HANDLED;
>  }
> @@ -1603,10 +1609,6 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  	}
>
>  	s->port.irq = irq;
> -	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
> -			       dev_name(&pdev->dev), s);
> -	if (ret)
> -		goto out_disable_clk;
>
>  	platform_set_drvdata(pdev, s);
>
> @@ -1627,9 +1629,28 @@ static int mxs_auart_probe(struct platform_device *pdev)
>
>  	mxs_auart_reset_deassert(s);
>
> +	/* Mask all UART interrupts to prevent spurious IRQs from bootloader */
> +	mxs_write(0, s, REG_INTR);
> +
>  	ret = uart_add_one_port(&auart_driver, &s->port);
> -	if (ret)
> -		goto out_free_qpio_irq;
> +	if (ret) {
> +		auart_port[s->port.line] = NULL;
> +		goto out_disable_clk;
> +	}
> +
> +	/*
> +	 * Request the main IRQ after uart_add_one_port so that
> +	 * s->port.state and s->port.lock are initialized before
> +	 * the handler can run in response to a bootloader-left
> +	 * interrupt.
> +	 */
> +	ret = devm_request_irq(&pdev->dev, irq, mxs_auart_irq_handle, 0,
> +			       dev_name(&pdev->dev), s);
> +	if (ret) {
> +		uart_remove_one_port(&auart_driver, &s->port);
> +		auart_port[s->port.line] = NULL;
> +		goto out_disable_clk;
> +	}
>
>  	/* ASM9260 don't have version reg */
>  	if (is_asm9260_auart(s)) {
> @@ -1641,10 +1662,16 @@ static int mxs_auart_probe(struct platform_device *pdev)
>  			 (version >> 16) & 0xff, version & 0xffff);
>  	}
>
> -	return 0;
> +	/*
> +	 * Disable clock -- startup will re-enable when the port is opened.
> +	 * For the console port the clock must stay prepared so that
> +	 * auart_console_write() can safely call clk_enable() from
> +	 * atomic context.
> +	 */
> +	if (!uart_console(&s->port))
> +		clk_disable_unprepare(s->clk);
>
> -out_free_qpio_irq:
> -	auart_port[s->port.line] = NULL;
> +	return 0;
>
>  out_disable_clk:
>  	clk_disable_unprepare(s->clk);
> @@ -1657,6 +1684,8 @@ static void mxs_auart_remove(struct platform_device *pdev)
>
>  	uart_remove_one_port(&auart_driver, &s->port);
>  	auart_port[s->port.line] = NULL;
> +	if (uart_console(&s->port))
> +		clk_disable_unprepare(s->clk);
>  }
>
>  static struct platform_driver mxs_auart_driver = {
> --
> 2.54.0
>
>

Re: [PATCHv2 1/4] serial: mxs-auart: fix cast type for of_device_get_match_data

[Rosen Penev]

On Tue, Jun 9, 2026 at 6:13 PM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Tue, Jun 09, 2026 at 03:37:14PM -0700, Rosen Penev wrote:
> > of_device_get_match_data returns const void*. Cast to unsigned long to
> > avoid implicit integer truncation warnings. All the data parameters are
> > correct anyway.
>
> It is not worth to fix it by this ways. cast void * to int/long is not good
> at all.
It's just a compilation fix. The same solution is used in many other places.
>
> struct drvdata
> {
>         enum mxs_auart_type type;
> }
>
> static const struct drvata chip_imx28;
>
> &chip_imx28 as drv data.
That's a lot of work just to avoid a cast.
>
> Frank
> >
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/tty/serial/mxs-auart.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
> > index 697318dbb146..de97c0f74e7d 100644
> > --- a/drivers/tty/serial/mxs-auart.c
> > +++ b/drivers/tty/serial/mxs-auart.c
> > @@ -1598,7 +1598,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
> >               return -EINVAL;
> >       }
> >
> > -     s->devtype = (enum mxs_auart_type)of_device_get_match_data(&pdev->dev);
> > +     s->devtype = (unsigned long)of_device_get_match_data(&pdev->dev);
> >
> >       ret = mxs_get_clks(s, pdev);
> >       if (ret)
> > --
> > 2.54.0
> >
> >