[PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[Andy Shevchenko]

A few ad-hoc cleanups and one patch to make driver OF-independent.

Chagelog v2:
- renamed init to init_fn and typedef accordingly (Daniel)
- added tags (Daniel, Javier)

Andy Shevchenko (4):
  backlight: hx8357: Make use of device properties
  backlight: hx8357: Move OF table closer to its consumer
  backlight: hx8357: Make use of dev_err_probe()
  backlight: hx8357: Utilise temporary variable for struct device

 drivers/video/backlight/hx8357.c | 57 +++++++++++++++-----------------
 1 file changed, 27 insertions(+), 30 deletions(-)

-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 1/4] backlight: hx8357: Make use of device properties

[Andy Shevchenko]

Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Include mod_devicetable.h explicitly to replace the dropped of.h
which included mod_devicetable.h indirectly.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/video/backlight/hx8357.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index bf18337ff0c2..ac65609e5d84 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -8,9 +8,9 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/lcd.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 
 #define HX8357_NUM_IM_PINS	3
@@ -564,6 +564,8 @@ static struct lcd_ops hx8357_ops = {
 	.get_power	= hx8357_get_power,
 };
 
+typedef int (*hx8357_init_fn)(struct lcd_device *);
+
 static const struct of_device_id hx8357_dt_ids[] = {
 	{
 		.compatible = "himax,hx8357",
@@ -582,7 +584,7 @@ static int hx8357_probe(struct spi_device *spi)
 	struct device *dev = &spi->dev;
 	struct lcd_device *lcdev;
 	struct hx8357_data *lcd;
-	const struct of_device_id *match;
+	hx8357_init_fn init_fn;
 	int i, ret;
 
 	lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
@@ -597,8 +599,8 @@ static int hx8357_probe(struct spi_device *spi)
 
 	lcd->spi = spi;
 
-	match = of_match_device(hx8357_dt_ids, &spi->dev);
-	if (!match || !match->data)
+	init_fn = device_get_match_data(dev);
+	if (!init_fn)
 		return -EINVAL;
 
 	lcd->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
@@ -627,7 +629,7 @@ static int hx8357_probe(struct spi_device *spi)
 
 	hx8357_lcd_reset(lcdev);
 
-	ret = ((int (*)(struct lcd_device *))match->data)(lcdev);
+	ret = init_fn(lcdev);
 	if (ret) {
 		dev_err(&spi->dev, "Couldn't initialize panel\n");
 		return ret;
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v2 1/4] backlight: hx8357: Make use of device properties

[Daniel Thompson]

On Thu, Feb 01, 2024 at 04:47:42PM +0200, Andy Shevchenko wrote:
> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.
>
> Include mod_devicetable.h explicitly to replace the dropped of.h
> which included mod_devicetable.h indirectly.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.

[PATCH v2 2/4] backlight: hx8357: Move OF table closer to its consumer

[Andy Shevchenko]

Move OF table near to the user.

While at it, drop comma at terminator entry.

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/video/backlight/hx8357.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index ac65609e5d84..81d0984e9d8b 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -566,19 +566,6 @@ static struct lcd_ops hx8357_ops = {
 
 typedef int (*hx8357_init_fn)(struct lcd_device *);
 
-static const struct of_device_id hx8357_dt_ids[] = {
-	{
-		.compatible = "himax,hx8357",
-		.data = hx8357_lcd_init,
-	},
-	{
-		.compatible = "himax,hx8369",
-		.data = hx8369_lcd_init,
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
-
 static int hx8357_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -640,6 +627,19 @@ static int hx8357_probe(struct spi_device *spi)
 	return 0;
 }
 
+static const struct of_device_id hx8357_dt_ids[] = {
+	{
+		.compatible = "himax,hx8357",
+		.data = hx8357_lcd_init,
+	},
+	{
+		.compatible = "himax,hx8369",
+		.data = hx8369_lcd_init,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, hx8357_dt_ids);
+
 static struct spi_driver hx8357_driver = {
 	.probe  = hx8357_probe,
 	.driver = {
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 3/4] backlight: hx8357: Make use of dev_err_probe()

[Andy Shevchenko]

Simplify the error handling in probe function by switching from
dev_err() to dev_err_probe().

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/video/backlight/hx8357.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 81d0984e9d8b..70a62755805a 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -579,10 +579,8 @@ static int hx8357_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	ret = spi_setup(spi);
-	if (ret < 0) {
-		dev_err(&spi->dev, "SPI setup failed.\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "SPI setup failed.\n");
 
 	lcd->spi = spi;
 
@@ -617,10 +615,8 @@ static int hx8357_probe(struct spi_device *spi)
 	hx8357_lcd_reset(lcdev);
 
 	ret = init_fn(lcdev);
-	if (ret) {
-		dev_err(&spi->dev, "Couldn't initialize panel\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't initialize panel\n");
 
 	dev_info(&spi->dev, "Panel probed\n");
 
-- 
2.43.0.rc1.1.gbec44491f096

[PATCH v2 4/4] backlight: hx8357: Utilise temporary variable for struct device

[Andy Shevchenko]

We have a temporary variable to keep pointer to struct device.
Utilise it inside the ->probe() implementation.

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/video/backlight/hx8357.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 70a62755805a..339d9128fbde 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -574,7 +574,7 @@ static int hx8357_probe(struct spi_device *spi)
 	hx8357_init_fn init_fn;
 	int i, ret;
 
-	lcd = devm_kzalloc(&spi->dev, sizeof(*lcd), GFP_KERNEL);
+	lcd = devm_kzalloc(dev, sizeof(*lcd), GFP_KERNEL);
 	if (!lcd)
 		return -ENOMEM;
 
@@ -604,8 +604,7 @@ static int hx8357_probe(struct spi_device *spi)
 			gpiod_set_consumer_name(lcd->im_pins->desc[i], "im_pins");
 	}
 
-	lcdev = devm_lcd_device_register(&spi->dev, "mxsfb", &spi->dev, lcd,
-					&hx8357_ops);
+	lcdev = devm_lcd_device_register(dev, "mxsfb", dev, lcd, &hx8357_ops);
 	if (IS_ERR(lcdev)) {
 		ret = PTR_ERR(lcdev);
 		return ret;
@@ -618,7 +617,7 @@ static int hx8357_probe(struct spi_device *spi)
 	if (ret)
 		return dev_err_probe(dev, ret, "Couldn't initialize panel\n");
 
-	dev_info(&spi->dev, "Panel probed\n");
+	dev_info(dev, "Panel probed\n");
 
 	return 0;
 }
-- 
2.43.0.rc1.1.gbec44491f096

Re: [PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[Lee Jones]

On Thu, 01 Feb 2024 16:47:41 +0200, Andy Shevchenko wrote:
> A few ad-hoc cleanups and one patch to make driver OF-independent.
> 
> Chagelog v2:
> - renamed init to init_fn and typedef accordingly (Daniel)
> - added tags (Daniel, Javier)
> 
> Andy Shevchenko (4):
>   backlight: hx8357: Make use of device properties
>   backlight: hx8357: Move OF table closer to its consumer
>   backlight: hx8357: Make use of dev_err_probe()
>   backlight: hx8357: Utilise temporary variable for struct device
> 
> [...]

Applied, thanks!

[1/4] backlight: hx8357: Make use of device properties
      commit: d965a5ee7c95ce9414259181cbdccb1d2f1c1247
[2/4] backlight: hx8357: Move OF table closer to its consumer
      commit: 3d226ecdfd83c0d89c1d4a430706e8228022685d
[3/4] backlight: hx8357: Make use of dev_err_probe()
      commit: f0ed1589885ae933e2b2f9c63e16f5be3fb0324d
[4/4] backlight: hx8357: Utilise temporary variable for struct device
      commit: 27a4701c92250ae0aecb2edea1109f89cf344ba1

--
Lee Jones [李琼斯]

Re: [PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[Lee Jones]

On Thu, 01 Feb 2024, Andy Shevchenko wrote:

> A few ad-hoc cleanups and one patch to make driver OF-independent.
> 
> Chagelog v2:
> - renamed init to init_fn and typedef accordingly (Daniel)
> - added tags (Daniel, Javier)
> 
> Andy Shevchenko (4):
>   backlight: hx8357: Make use of device properties
>   backlight: hx8357: Move OF table closer to its consumer
>   backlight: hx8357: Make use of dev_err_probe()
>   backlight: hx8357: Utilise temporary variable for struct device
> 
>  drivers/video/backlight/hx8357.c | 57 +++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 30 deletions(-)

Someone may wish to address this:

WARNING: DT compatible string "himax,hx8369" appears un-documented -- check ./Documentation/devicetree/bindings/
#58: FILE: drivers/video/backlight/hx8357.c:636:
+		.compatible = "himax,hx8369",

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[Andy Shevchenko]

On Thu, Feb 08, 2024 at 10:53:04AM +0000, Lee Jones wrote:
> On Thu, 01 Feb 2024, Andy Shevchenko wrote:

...

> Someone may wish to address this:
> 
> WARNING: DT compatible string "himax,hx8369" appears un-documented -- check ./Documentation/devicetree/bindings/
> #58: FILE: drivers/video/backlight/hx8357.c:636:
> +		.compatible = "himax,hx8369",

I can do it if and when have more time. But apparently it was before this
series, right?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/4] backlight: hx8357: Clean up and make OF-independent

[Lee Jones]

On Thu, 08 Feb 2024, Andy Shevchenko wrote:

> On Thu, Feb 08, 2024 at 10:53:04AM +0000, Lee Jones wrote:
> > On Thu, 01 Feb 2024, Andy Shevchenko wrote:
> 
> ...
> 
> > Someone may wish to address this:
> > 
> > WARNING: DT compatible string "himax,hx8369" appears un-documented -- check ./Documentation/devicetree/bindings/
> > #58: FILE: drivers/video/backlight/hx8357.c:636:
> > +		.compatible = "himax,hx8369",
> 
> I can do it if and when have more time. But apparently it was before this
> series, right?

I'm not sure it's ever been documented.

It doesn't affect your series in any way.

-- 
Lee Jones [李琼斯]