[PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Andy Shevchenko]

The whole series was induced by the Geert's report on one mistake
in the error path. I looked closely and found a room to refactor
the whole driver to use modern style and APIs.

Not tested.

Andy Shevchenko (7):
  auxdisplay: arm-charlcd: convert to use device managed APIs
  auxdisplay: arm-charlcd: Remove unneeded info message
  auxdisplay: arm-charlcd: drop of_match_ptr()
  auxdisplay: arm-charlcd: Don't use "proxy" headers
  auxdisplay: arm-charlcd: Use readl_poll_timeout
  auxdisplay: arm-charlcd: Join string literals back
  auxdisplay: arm-charlcd: Remove redundant ternary operators

 drivers/auxdisplay/arm-charlcd.c | 96 ++++++++++----------------------
 1 file changed, 28 insertions(+), 68 deletions(-)

-- 
2.50.1

[PATCH v1 1/7] auxdisplay: arm-charlcd: convert to use device managed APIs

[Andy Shevchenko]

The current probe function of the driver is implemented by using
plain old APIs. As Geert noted, one of the error paths has a mistake
that may lead to a wrong call. Instead of targeting that particular
issue, convert the driver to use device managed APIs altogether.
It's not needed per se (driver can never be removed once loaded),
but it simplifies coding and error handling.

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lore.kernel.org/r/CAMuHMdVquNEoxRQkcBEH0nC+CDuib6o0H6m3CBk3ZN2267LpQw@mail.gmail.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 49 ++++++--------------------------
 1 file changed, 9 insertions(+), 40 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index 4e22882f57c9..0dab69938352 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -14,7 +14,6 @@
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/io.h>
-#include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <generated/utsrelease.h>
 
@@ -56,8 +55,6 @@
 /**
  * struct charlcd - Private data structure
  * @dev: a pointer back to containing device
- * @phybase: the offset to the controller in physical memory
- * @physize: the size of the physical page
  * @virtbase: the offset to the controller in virtual memory
  * @irq: reserved interrupt number
  * @complete: completion structure for the last LCD command
@@ -65,8 +62,6 @@
  */
 struct charlcd {
 	struct device *dev;
-	u32 phybase;
-	u32 physize;
 	void __iomem *virtbase;
 	int irq;
 	struct completion complete;
@@ -266,44 +261,27 @@ static void charlcd_init_work(struct work_struct *work)
 
 static int __init charlcd_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	int ret;
 	struct charlcd *lcd;
 	struct resource *res;
 
-	lcd = kzalloc(sizeof(*lcd), GFP_KERNEL);
+	lcd = devm_kzalloc(dev, sizeof(*lcd), GFP_KERNEL);
 	if (!lcd)
 		return -ENOMEM;
 
 	lcd->dev = &pdev->dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		ret = -ENOENT;
-		goto out_no_resource;
-	}
-	lcd->phybase = res->start;
-	lcd->physize = resource_size(res);
-
-	if (request_mem_region(lcd->phybase, lcd->physize,
-			       DRIVERNAME) == NULL) {
-		ret = -EBUSY;
-		goto out_no_memregion;
-	}
-
-	lcd->virtbase = ioremap(lcd->phybase, lcd->physize);
-	if (!lcd->virtbase) {
-		ret = -ENOMEM;
-		goto out_no_memregion;
-	}
+	lcd->virtbase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(lcd->virtbase))
+		return PTR_ERR(lcd->virtbase);
 
 	lcd->irq = platform_get_irq(pdev, 0);
 	/* If no IRQ is supplied, we'll survive without it */
 	if (lcd->irq >= 0) {
-		if (request_irq(lcd->irq, charlcd_interrupt, 0,
-				DRIVERNAME, lcd)) {
-			ret = -EIO;
-			goto out_no_irq;
-		}
+		ret = devm_request_irq(dev, lcd->irq, charlcd_interrupt, 0, DRIVERNAME, lcd);
+		if (ret)
+			return ret;
 	}
 
 	platform_set_drvdata(pdev, lcd);
@@ -315,18 +293,9 @@ static int __init charlcd_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&lcd->init_work, charlcd_init_work);
 	schedule_delayed_work(&lcd->init_work, 0);
 
-	dev_info(&pdev->dev, "initialized ARM character LCD at %08x\n",
-		lcd->phybase);
+	dev_info(dev, "initialized ARM character LCD at %pa\n", &res->start);
 
 	return 0;
-
-out_no_irq:
-	iounmap(lcd->virtbase);
-out_no_memregion:
-	release_mem_region(lcd->phybase, lcd->physize);
-out_no_resource:
-	kfree(lcd);
-	return ret;
 }
 
 static int charlcd_suspend(struct device *dev)
-- 
2.50.1

[PATCH v1 2/7] auxdisplay: arm-charlcd: Remove unneeded info message

[Andy Shevchenko]

When probe succeeds we have other means to check if it was successful,
no need to pollute kernel log with the practically duplicated info.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index 0dab69938352..eeabbca7923d 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -264,7 +264,6 @@ static int __init charlcd_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int ret;
 	struct charlcd *lcd;
-	struct resource *res;
 
 	lcd = devm_kzalloc(dev, sizeof(*lcd), GFP_KERNEL);
 	if (!lcd)
@@ -272,7 +271,7 @@ static int __init charlcd_probe(struct platform_device *pdev)
 
 	lcd->dev = &pdev->dev;
 
-	lcd->virtbase = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	lcd->virtbase = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(lcd->virtbase))
 		return PTR_ERR(lcd->virtbase);
 
@@ -293,8 +292,6 @@ static int __init charlcd_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&lcd->init_work, charlcd_init_work);
 	schedule_delayed_work(&lcd->init_work, 0);
 
-	dev_info(dev, "initialized ARM character LCD at %pa\n", &res->start);
-
 	return 0;
 }
 
-- 
2.50.1

[PATCH v1 3/7] auxdisplay: arm-charlcd: drop of_match_ptr()

[Andy Shevchenko]

The general recommendation is to not use of_match_ptr(). Drop it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index eeabbca7923d..6a83eacb458c 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -9,8 +9,8 @@
  */
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
-#include <linux/of.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -328,7 +328,7 @@ static struct platform_driver charlcd_driver = {
 		.name = DRIVERNAME,
 		.pm = &charlcd_pm_ops,
 		.suppress_bind_attrs = true,
-		.of_match_table = of_match_ptr(charlcd_match),
+		.of_match_table = charlcd_match,
 	},
 };
 builtin_platform_driver_probe(charlcd_driver, charlcd_probe);
-- 
2.50.1

[PATCH v1 4/7] auxdisplay: arm-charlcd: Don't use "proxy" headers

[Andy Shevchenko]

Update header inclusions to follow IWYU (Include What You Use) principle.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index 6a83eacb458c..ac8604d3b9a0 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -7,13 +7,17 @@
  *
  * Author: Linus Walleij <triad@df.lth.se>
  */
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/err.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
-#include <linux/completion.h>
-#include <linux/delay.h>
-#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/types.h>
 #include <linux/workqueue.h>
 #include <generated/utsrelease.h>
 
-- 
2.50.1

[PATCH v1 5/7] auxdisplay: arm-charlcd: Use readl_poll_timeout

[Andy Shevchenko]

Use readl_poll_timeout_atomic() from <iopoll.h> instead of using
custom poll loops.

The timeout settings are different, but that shouldn't be much of a
problem. Instead of polling 10 times in a close loop, it polls for
one millisecond.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index ac8604d3b9a0..a537126f5d6a 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -13,7 +13,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
-#include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
@@ -115,20 +115,14 @@ static u8 charlcd_4bit_read_char(struct charlcd *lcd)
 {
 	u8 data;
 	u32 val;
-	int i;
 
 	/* If we can, use an IRQ to wait for the data, else poll */
 	if (lcd->irq >= 0)
 		charlcd_wait_complete_irq(lcd);
 	else {
-		i = 0;
-		val = 0;
-		while (!(val & CHAR_RAW_VALID) && i < 10) {
-			udelay(100);
-			val = readl(lcd->virtbase + CHAR_RAW);
-			i++;
-		}
-
+		udelay(100);
+		readl_poll_timeout_atomic(lcd->virtbase + CHAR_RAW, val,
+					  val & CHAR_RAW_VALID, 100, 1000);
 		writel(CHAR_RAW_CLEAR, lcd->virtbase + CHAR_RAW);
 	}
 	msleep(1);
@@ -140,13 +134,9 @@ static u8 charlcd_4bit_read_char(struct charlcd *lcd)
 	 * The second read for the low bits does not trigger an IRQ
 	 * so in this case we have to poll for the 4 lower bits
 	 */
-	i = 0;
-	val = 0;
-	while (!(val & CHAR_RAW_VALID) && i < 10) {
-		udelay(100);
-		val = readl(lcd->virtbase + CHAR_RAW);
-		i++;
-	}
+	udelay(100);
+	readl_poll_timeout_atomic(lcd->virtbase + CHAR_RAW, val,
+				  val & CHAR_RAW_VALID, 100, 1000);
 	writel(CHAR_RAW_CLEAR, lcd->virtbase + CHAR_RAW);
 	msleep(1);
 
-- 
2.50.1

[PATCH v1 6/7] auxdisplay: arm-charlcd: Join string literals back

[Andy Shevchenko]

For easy grepping on debug purposes join string literals back in
the messages.

No functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index a537126f5d6a..7395a6dd6dc9 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -99,14 +99,13 @@ static void charlcd_wait_complete_irq(struct charlcd *lcd)
 
 	if (ret < 0) {
 		dev_err(lcd->dev,
-			"wait_for_completion_interruptible_timeout() "
-			"returned %d waiting for ready\n", ret);
+			"wait_for_completion_interruptible_timeout() returned %d waiting for ready\n",
+			ret);
 		return;
 	}
 
 	if (ret == 0) {
-		dev_err(lcd->dev, "charlcd controller timed out "
-			"waiting for ready\n");
+		dev_err(lcd->dev, "charlcd controller timed out waiting for ready\n");
 		return;
 	}
 }
-- 
2.50.1

[PATCH v1 7/7] auxdisplay: arm-charlcd: Remove redundant ternary operators

[Andy Shevchenko]

For ternary operators in the form of a ? true : false, if a itself returns
a boolean result, the ternary operator can be omitted. Remove redundant
ternary operators to clean up the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/auxdisplay/arm-charlcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/auxdisplay/arm-charlcd.c b/drivers/auxdisplay/arm-charlcd.c
index 7395a6dd6dc9..30fd2341c628 100644
--- a/drivers/auxdisplay/arm-charlcd.c
+++ b/drivers/auxdisplay/arm-charlcd.c
@@ -157,7 +157,8 @@ static bool charlcd_4bit_read_bf(struct charlcd *lcd)
 		writel(0x01, lcd->virtbase + CHAR_MASK);
 	}
 	readl(lcd->virtbase + CHAR_COM);
-	return charlcd_4bit_read_char(lcd) & HD_BUSY_FLAG ? true : false;
+
+	return charlcd_4bit_read_char(lcd) & HD_BUSY_FLAG;
 }
 
 static void charlcd_4bit_wait_busy(struct charlcd *lcd)
-- 
2.50.1

Re: [PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Andy Shevchenko]

On Mon, Dec 29, 2025 at 02:52:33PM +0100, Andy Shevchenko wrote:
> The whole series was induced by the Geert's report on one mistake
> in the error path. I looked closely and found a room to refactor
> the whole driver to use modern style and APIs.

For so long I have got no CI reports and no comments, I'm going to push this
series tomorrow to the repository. Feel free to tell if there is any problem
with that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Andy Shevchenko]

On Wed, Jan 07, 2026 at 11:06:56PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 29, 2025 at 02:52:33PM +0100, Andy Shevchenko wrote:
> > The whole series was induced by the Geert's report on one mistake
> > in the error path. I looked closely and found a room to refactor
> > the whole driver to use modern style and APIs.
> 
> For so long I have got no CI reports and no comments, I'm going to push this
> series tomorrow to the repository. Feel free to tell if there is any problem
> with that.

If I'm not mistaken, Linus might want to have a look... Cc'ing him.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Andy Shevchenko]

On Wed, Jan 07, 2026 at 11:08:20PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 07, 2026 at 11:06:56PM +0200, Andy Shevchenko wrote:
> > On Mon, Dec 29, 2025 at 02:52:33PM +0100, Andy Shevchenko wrote:
> > > The whole series was induced by the Geert's report on one mistake
> > > in the error path. I looked closely and found a room to refactor
> > > the whole driver to use modern style and APIs.
> > 
> > For so long I have got no CI reports and no comments, I'm going to push this
> > series tomorrow to the repository. Feel free to tell if there is any problem
> > with that.
> 
> If I'm not mistaken, Linus might want to have a look... Cc'ing him.

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Linus Walleij]

On Wed, Jan 7, 2026 at 10:08 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, Jan 07, 2026 at 11:06:56PM +0200, Andy Shevchenko wrote:
> > On Mon, Dec 29, 2025 at 02:52:33PM +0100, Andy Shevchenko wrote:
> > > The whole series was induced by the Geert's report on one mistake
> > > in the error path. I looked closely and found a room to refactor
> > > the whole driver to use modern style and APIs.
> >
> > For so long I have got no CI reports and no comments, I'm going to push this
> > series tomorrow to the repository. Feel free to tell if there is any problem
> > with that.
>
> If I'm not mistaken, Linus might want to have a look... Cc'ing him.

I haven't had time to look at it because it's not in my inbox, sorry.

On the other hand, I trust you more than most developers so I'm pretty
sure I would just ACK it anyways.

Yours,
Linus Walleij

Re: [PATCH v1 0/7] auxdisplay: arm-charlcd: refactor with modern APIs

[Andy Shevchenko]

On Fri, Jan 9, 2026 at 4:30 PM Linus Walleij <linusw@kernel.org> wrote:
> On Wed, Jan 7, 2026 at 10:08 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Jan 07, 2026 at 11:06:56PM +0200, Andy Shevchenko wrote:
> > > On Mon, Dec 29, 2025 at 02:52:33PM +0100, Andy Shevchenko wrote:
> > > > The whole series was induced by the Geert's report on one mistake
> > > > in the error path. I looked closely and found a room to refactor
> > > > the whole driver to use modern style and APIs.
> > >
> > > For so long I have got no CI reports and no comments, I'm going to push this
> > > series tomorrow to the repository. Feel free to tell if there is any problem
> > > with that.
> >
> > If I'm not mistaken, Linus might want to have a look... Cc'ing him.
>
> I haven't had time to look at it because it's not in my inbox, sorry.

No, it's me who should say "sorry". I have told you about the old
address (you are the author, if I'm not mistaken), but I forgot to Cc
you with a new one. Of course, you can use `b4 mbox $MSG_ID` to
retrieve this whole thread, but I admit a bit of inconvenience.

> On the other hand, I trust you more than most developers so I'm pretty
> sure I would just ACK it anyways.

Thanks!

-- 
With Best Regards,
Andy Shevchenko