Re: [PATCH v1 1/4] backlight: hx8357: Make use of device properties
From: Javier Martinez Canillas <javierm@redhat.com>
Date: 2024-01-15 08:20:51
Also in:
dri-devel, lkml
Andy Shevchenko [off-list ref] writes: Hello Andy,
quoted hunk ↗ jump to hunk
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. 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..c7fd10d55c5d 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)(struct lcd_device *); +
This kind of typedef usage is frowned upon in the Linux coding style [0] (per my understanding at least) and indeed in my opinion it makes harder to grep. [0] https://www.kernel.org/doc/Documentation/process/coding-style.rst
quoted hunk ↗ jump to hunk
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 init; 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 = device_get_match_data(dev); + if (!init) 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);
This is what I mean, before it was clear what was stored in match->data. But after you changes, what is returned by the device_get_match_data() function is opaque and you need to look at the typedef hx8357_init to figure that out. No strong opinion though and I see other drivers doing the same (but no other driver in drivers/video/backlight). Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> -- Best regards, Javier Martinez Canillas Core Platforms Red Hat