* [PATCH] backlight: hx8357: Add SPI device ID table
@ 2021-09-22 15:10 Mark Brown
2021-09-27 9:42 ` Daniel Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-09-22 15:10 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han; +Cc: dri-devel, linux-fbdev, Mark Brown
Currently autoloading for SPI devices does not use the DT ID table, it uses
SPI modalises. Supporting OF modalises is going to be difficult if not
impractical, an attempt was made but has been reverted, so ensure that
module autoloading works for this driver by adding a SPI device ID table.
Fixes: 96c8395e2166 ("spi: Revert modalias changes")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/video/backlight/hx8357.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
index 9b50bc96e00f..c64b1fbe027f 100644
--- a/drivers/video/backlight/hx8357.c
+++ b/drivers/video/backlight/hx8357.c
@@ -565,6 +565,19 @@ static struct lcd_ops hx8357_ops = {
.get_power = hx8357_get_power,
};
+static const struct spi_device_id hx8357_spi_ids[] = {
+ {
+ .name = "hx8357",
+ .driver_data = (kernel_ulong_t)hx8357_lcd_init,
+ },
+ {
+ .name = "hx8369",
+ .driver_data = (kernel_ulong_t)hx8369_lcd_init,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(spi, hx8357_spi_ids);
+
static const struct of_device_id hx8357_dt_ids[] = {
{
.compatible = "himax,hx8357",
@@ -672,6 +685,7 @@ static struct spi_driver hx8357_driver = {
.name = "hx8357",
.of_match_table = hx8357_dt_ids,
},
+ .id_table = hx8357_spi_ids,
};
module_spi_driver(hx8357_driver);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: hx8357: Add SPI device ID table
2021-09-22 15:10 [PATCH] backlight: hx8357: Add SPI device ID table Mark Brown
@ 2021-09-27 9:42 ` Daniel Thompson
2021-09-27 11:47 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Thompson @ 2021-09-27 9:42 UTC (permalink / raw)
To: Mark Brown; +Cc: Lee Jones, Jingoo Han, dri-devel, linux-fbdev
On Wed, Sep 22, 2021 at 04:10:14PM +0100, Mark Brown wrote:
> Currently autoloading for SPI devices does not use the DT ID table, it uses
> SPI modalises. Supporting OF modalises is going to be difficult if not
> impractical, an attempt was made but has been reverted, so ensure that
> module autoloading works for this driver by adding a SPI device ID table.
>
> Fixes: 96c8395e2166 ("spi: Revert modalias changes")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> drivers/video/backlight/hx8357.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> index 9b50bc96e00f..c64b1fbe027f 100644
> --- a/drivers/video/backlight/hx8357.c
> +++ b/drivers/video/backlight/hx8357.c
> @@ -565,6 +565,19 @@ static struct lcd_ops hx8357_ops = {
> .get_power = hx8357_get_power,
> };
>
> +static const struct spi_device_id hx8357_spi_ids[] = {
> + {
> + .name = "hx8357",
> + .driver_data = (kernel_ulong_t)hx8357_lcd_init,
Based on this I had expected to find spi_get_device_id() and a ->driver_data
somewhere in this patch.
Where will this .driver_data be consumed?
Daniel.
> + },
> + {
> + .name = "hx8369",
> + .driver_data = (kernel_ulong_t)hx8369_lcd_init,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(spi, hx8357_spi_ids);
> +
> static const struct of_device_id hx8357_dt_ids[] = {
> {
> .compatible = "himax,hx8357",
> @@ -672,6 +685,7 @@ static struct spi_driver hx8357_driver = {
> .name = "hx8357",
> .of_match_table = hx8357_dt_ids,
> },
> + .id_table = hx8357_spi_ids,
> };
>
> module_spi_driver(hx8357_driver);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: hx8357: Add SPI device ID table
2021-09-27 9:42 ` Daniel Thompson
@ 2021-09-27 11:47 ` Mark Brown
2021-09-27 13:24 ` Daniel Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-09-27 11:47 UTC (permalink / raw)
To: Daniel Thompson; +Cc: Lee Jones, Jingoo Han, dri-devel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 393 bytes --]
On Mon, Sep 27, 2021 at 10:42:00AM +0100, Daniel Thompson wrote:
> Based on this I had expected to find spi_get_device_id() and a ->driver_data
> somewhere in this patch.
> Where will this .driver_data be consumed?
It will never be consumed unless someone writes a board file - the
runtime match will still happen based on the DT compatible, this is only
there for the modalias.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: hx8357: Add SPI device ID table
2021-09-27 11:47 ` Mark Brown
@ 2021-09-27 13:24 ` Daniel Thompson
2021-09-27 13:30 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Thompson @ 2021-09-27 13:24 UTC (permalink / raw)
To: Mark Brown; +Cc: Lee Jones, Jingoo Han, dri-devel, linux-fbdev
On Mon, Sep 27, 2021 at 12:47:27PM +0100, Mark Brown wrote:
> On Mon, Sep 27, 2021 at 10:42:00AM +0100, Daniel Thompson wrote:
>
> > Based on this I had expected to find spi_get_device_id() and a ->driver_data
> > somewhere in this patch.
>
> > Where will this .driver_data be consumed?
>
> It will never be consumed unless someone writes a board file - the
> runtime match will still happen based on the DT compatible, this is only
> there for the modalias.
Ok... so I'm not going mad.
In that case what is the point of including unconsumed driver data? Most
DT centric drivers that included this for modalias reasons leave it
NULL.
I reviewed quite a few drivers this morning and I haven't seen a single
one that includes unreachable driver data in this manner. Unless there's
a good reason I'd prefer backlight to follow the prior art!
Daniel.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: hx8357: Add SPI device ID table
2021-09-27 13:24 ` Daniel Thompson
@ 2021-09-27 13:30 ` Mark Brown
2021-10-01 14:20 ` Daniel Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2021-09-27 13:30 UTC (permalink / raw)
To: Daniel Thompson; +Cc: Lee Jones, Jingoo Han, dri-devel, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 387 bytes --]
On Mon, Sep 27, 2021 at 02:24:17PM +0100, Daniel Thompson wrote:
> In that case what is the point of including unconsumed driver data? Most
> DT centric drivers that included this for modalias reasons leave it
> NULL.
It's mainly there because it's generated fairly mechanically from the OF
ID table - there's no great reason for it to be there while all
instantiation is done via DT.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: hx8357: Add SPI device ID table
2021-09-27 13:30 ` Mark Brown
@ 2021-10-01 14:20 ` Daniel Thompson
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Thompson @ 2021-10-01 14:20 UTC (permalink / raw)
To: Mark Brown; +Cc: Lee Jones, Jingoo Han, dri-devel, linux-fbdev
On Mon, Sep 27, 2021 at 02:30:56PM +0100, Mark Brown wrote:
> On Mon, Sep 27, 2021 at 02:24:17PM +0100, Daniel Thompson wrote:
>
> > In that case what is the point of including unconsumed driver data? Most
> > DT centric drivers that included this for modalias reasons leave it
> > NULL.
>
> It's mainly there because it's generated fairly mechanically from the OF
> ID table - there's no great reason for it to be there while all
> instantiation is done via DT.
Ok. If there's no plan to change the driver to pick it up from the table then
let's remove the redundant values. They just make it look like somebody forgot
something in the probe method (instead of it being a deliberate choice).
Daniel.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-01 14:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 15:10 [PATCH] backlight: hx8357: Add SPI device ID table Mark Brown
2021-09-27 9:42 ` Daniel Thompson
2021-09-27 11:47 ` Mark Brown
2021-09-27 13:24 ` Daniel Thompson
2021-09-27 13:30 ` Mark Brown
2021-10-01 14:20 ` Daniel Thompson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).