diff mbox series

[1/7] drm/panel: sitronix-st7789v: Prevent core spi warnings

Message ID 20230609145951.853533-2-miquel.raynal@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/panel: sitronix-st7789v: Support ET028013DMA panel | expand

Commit Message

Miquel Raynal June 9, 2023, 2:59 p.m. UTC 
The spi core warns us about using an of_device_id table without a
spi_device_id table aside for module utilities in orter to not rely on
OF modaliases. Just add this table using the device name without the
vendor prefix (as it is usually done).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Sam Ravnborg June 10, 2023, 8:06 p.m. UTC | #1 
On Fri, Jun 09, 2023 at 04:59:45PM +0200, Miquel Raynal wrote:
> The spi core warns us about using an of_device_id table without a
> spi_device_id table aside for module utilities in orter to not rely on
> OF modaliases. Just add this table using the device name without the
> vendor prefix (as it is usually done).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index bbc4569cbcdc..c67b9adb157f 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -400,9 +400,16 @@ static const struct of_device_id st7789v_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, st7789v_of_match);
>  
> +static const struct spi_device_id st7789v_ids[] = {
> +	{ "st7789v", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> +
>  static struct spi_driver st7789v_driver = {
>  	.probe = st7789v_probe,
>  	.remove = st7789v_remove,
> +	.id_table = st7789v_ids,
>  	.driver = {
>  		.name = "st7789v",
>  		.of_match_table = st7789v_of_match,
> -- 
> 2.34.1
Michael Riesch June 13, 2023, 6:15 a.m. UTC | #2 
Hi Miquel,

On 6/9/23 16:59, Miquel Raynal wrote:
> The spi core warns us about using an of_device_id table without a

s/spi/SPI ?

> spi_device_id table aside for module utilities in orter to not rely on

s/in orter to/in order to ?

> OF modaliases. Just add this table using the device name without the
> vendor prefix (as it is usually done).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> index bbc4569cbcdc..c67b9adb157f 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> @@ -400,9 +400,16 @@ static const struct of_device_id
st7789v_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, st7789v_of_match);
>
> +static const struct spi_device_id st7789v_ids[] = {
> +	{ "st7789v", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> +
>  static struct spi_driver st7789v_driver = {
>  	.probe = st7789v_probe,
>  	.remove = st7789v_remove,
> +	.id_table = st7789v_ids,
>  	.driver = {
>  		.name = "st7789v",
>  		.of_match_table = st7789v_of_match,

May I point to you Sebastian Reichel's series that features a partial
overlap with your work? [0]

For instance, the patch at hand is comparable to [1].

Cc: Sebastian to keep him in the loop.

Best regards,
Michael

[0]
https://lore.kernel.org/dri-devel/20230422205012.2464933-1-sre@kernel.org/
[1]
https://lore.kernel.org/dri-devel/20230422205012.2464933-4-sre@kernel.org/
Miquel Raynal June 13, 2023, 6:56 a.m. UTC | #3 
Hi Michael,

michael.riesch@wolfvision.net wrote on Tue, 13 Jun 2023 08:15:26 +0200:

> Hi Miquel,
> 
> On 6/9/23 16:59, Miquel Raynal wrote:
> > The spi core warns us about using an of_device_id table without a  
> 
> s/spi/SPI ?

Actually both are accepted in the kernel, IIRC it depends whether you
refer to the name of the bus or the Linux subsystem. Same for picking
"a" vs "an" before "spi/SPI". An attempt to use a unique formatting was
actually canceled because both are used equivalently and I believe even
the spi maintainer and the spi-nor maintainer kind of disagreed on the
default :)

> > spi_device_id table aside for module utilities in orter to not rely on  
> 
> s/in orter to/in order to ?

Yes, sorry for this typo as well as the two others you rightly pointed
out in another patch. I'll fix them.

> > OF modaliases. Just add this table using the device name without the
> > vendor prefix (as it is usually done).
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c  
> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > index bbc4569cbcdc..c67b9adb157f 100644
> > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > @@ -400,9 +400,16 @@ static const struct of_device_id  
> st7789v_of_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, st7789v_of_match);
> >
> > +static const struct spi_device_id st7789v_ids[] = {
> > +	{ "st7789v", },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> > +
> >  static struct spi_driver st7789v_driver = {
> >  	.probe = st7789v_probe,
> >  	.remove = st7789v_remove,
> > +	.id_table = st7789v_ids,
> >  	.driver = {
> >  		.name = "st7789v",
> >  		.of_match_table = st7789v_of_match,  
> 
> May I point to you Sebastian Reichel's series that features a partial
> overlap with your work? [0]

Woow. That driver has been untouched for years and now two
contributions at the same time. Sebastian, what is the current state of
your series? Shall I base my work on top of yours? Or is it still too
premature and we shall instead try to merge both and contribute a new
version of the series bringing support for the two panels?

> For instance, the patch at hand is comparable to [1].
> 
> Cc: Sebastian to keep him in the loop.

Thanks a lot for pointing this out.

> Best regards,
> Michael
> 
> [0]
> https://lore.kernel.org/dri-devel/20230422205012.2464933-1-sre@kernel.org/
> [1]
> https://lore.kernel.org/dri-devel/20230422205012.2464933-4-sre@kernel.org/


Thanks,
Miquèl
Sebastian Reichel June 14, 2023, 11:22 p.m. UTC | #4 
Hi,

On Tue, Jun 13, 2023 at 08:56:30AM +0200, Miquel Raynal wrote:
> Hi Michael,
> 
> michael.riesch@wolfvision.net wrote on Tue, 13 Jun 2023 08:15:26 +0200:
> 
> > Hi Miquel,
> > 
> > On 6/9/23 16:59, Miquel Raynal wrote:
> > > The spi core warns us about using an of_device_id table without a  
> > 
> > s/spi/SPI ?
> 
> Actually both are accepted in the kernel, IIRC it depends whether you
> refer to the name of the bus or the Linux subsystem. Same for picking
> "a" vs "an" before "spi/SPI". An attempt to use a unique formatting was
> actually canceled because both are used equivalently and I believe even
> the spi maintainer and the spi-nor maintainer kind of disagreed on the
> default :)
> 
> > > spi_device_id table aside for module utilities in orter to not rely on  
> > 
> > s/in orter to/in order to ?
> 
> Yes, sorry for this typo as well as the two others you rightly pointed
> out in another patch. I'll fix them.
> 
> > > OF modaliases. Just add this table using the device name without the
> > > vendor prefix (as it is usually done).
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c  
> > b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > index bbc4569cbcdc..c67b9adb157f 100644
> > > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > @@ -400,9 +400,16 @@ static const struct of_device_id  
> > st7789v_of_match[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(of, st7789v_of_match);
> > >
> > > +static const struct spi_device_id st7789v_ids[] = {
> > > +	{ "st7789v", },
> > > +	{ /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> > > +
> > >  static struct spi_driver st7789v_driver = {
> > >  	.probe = st7789v_probe,
> > >  	.remove = st7789v_remove,
> > > +	.id_table = st7789v_ids,
> > >  	.driver = {
> > >  		.name = "st7789v",
> > >  		.of_match_table = st7789v_of_match,  
> > 
> > May I point to you Sebastian Reichel's series that features a partial
> > overlap with your work? [0]
> 
> Woow. That driver has been untouched for years and now two
> contributions at the same time.

Three actually. Michael also submitted a series :)

> Sebastian, what is the current state of your series?

The DT changes got Ack'd by Rob and I have the R-B from Michael
(minus a minor comment to make the panel struct 'static const').
It's mainly waiting for a review from Sam.

I was a bit distracted by a boot regression on the devices and
some other projects. The boot regression got solved, so I can
prepare a new version if that makes things easier.

> Shall I base my work on top of yours? Or is it still too
> premature and we shall instead try to merge both and contribute a new
> version of the series bringing support for the two panels?

I suppose whatever is easier for Sam to review.

-- Sebastian
Sam Ravnborg June 15, 2023, 5:43 a.m. UTC | #5 
On Thu, Jun 15, 2023 at 01:22:17AM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Jun 13, 2023 at 08:56:30AM +0200, Miquel Raynal wrote:
> > Hi Michael,
> > 
> > michael.riesch@wolfvision.net wrote on Tue, 13 Jun 2023 08:15:26 +0200:
> > 
> > > Hi Miquel,
> > > 
> > > On 6/9/23 16:59, Miquel Raynal wrote:
> > > > The spi core warns us about using an of_device_id table without a  
> > > 
> > > s/spi/SPI ?
> > 
> > Actually both are accepted in the kernel, IIRC it depends whether you
> > refer to the name of the bus or the Linux subsystem. Same for picking
> > "a" vs "an" before "spi/SPI". An attempt to use a unique formatting was
> > actually canceled because both are used equivalently and I believe even
> > the spi maintainer and the spi-nor maintainer kind of disagreed on the
> > default :)
> > 
> > > > spi_device_id table aside for module utilities in orter to not rely on  
> > > 
> > > s/in orter to/in order to ?
> > 
> > Yes, sorry for this typo as well as the two others you rightly pointed
> > out in another patch. I'll fix them.
> > 
> > > > OF modaliases. Just add this table using the device name without the
> > > > vendor prefix (as it is usually done).
> > > >
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c  
> > > b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > index bbc4569cbcdc..c67b9adb157f 100644
> > > > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > > @@ -400,9 +400,16 @@ static const struct of_device_id  
> > > st7789v_of_match[] = {
> > > >  };
> > > >  MODULE_DEVICE_TABLE(of, st7789v_of_match);
> > > >
> > > > +static const struct spi_device_id st7789v_ids[] = {
> > > > +	{ "st7789v", },
> > > > +	{ /* sentinel */ }
> > > > +};
> > > > +MODULE_DEVICE_TABLE(spi, st7789v_ids);
> > > > +
> > > >  static struct spi_driver st7789v_driver = {
> > > >  	.probe = st7789v_probe,
> > > >  	.remove = st7789v_remove,
> > > > +	.id_table = st7789v_ids,
> > > >  	.driver = {
> > > >  		.name = "st7789v",
> > > >  		.of_match_table = st7789v_of_match,  
> > > 
> > > May I point to you Sebastian Reichel's series that features a partial
> > > overlap with your work? [0]
> > 
> > Woow. That driver has been untouched for years and now two
> > contributions at the same time.
> 
> Three actually. Michael also submitted a series :)
> 
> > Sebastian, what is the current state of your series?
> 
> The DT changes got Ack'd by Rob and I have the R-B from Michael
> (minus a minor comment to make the panel struct 'static const').
> It's mainly waiting for a review from Sam.
> 
> I was a bit distracted by a boot regression on the devices and
> some other projects. The boot regression got solved, so I can
> prepare a new version if that makes things easier.
> 
> > Shall I base my work on top of yours? Or is it still too
> > premature and we shall instead try to merge both and contribute a new
> > version of the series bringing support for the two panels?
> 
> I suppose whatever is easier for Sam to review.

Hi Sebastian.

Too much panel stuff going on, so I miss the most and I am happy to see
other people do a lot of good work here.
Can i get a pointer to lore or so, then I will try to take a look.

	Sam
Sebastian Reichel June 15, 2023, 12:58 p.m. UTC | #6 
Hi Sam,

On Thu, Jun 15, 2023 at 07:43:46AM +0200, Sam Ravnborg wrote:
> On Thu, Jun 15, 2023 at 01:22:17AM +0200, Sebastian Reichel wrote:
> > > > May I point to you Sebastian Reichel's series that features a partial
> > > > overlap with your work? [0]
> > > 
> > > Woow. That driver has been untouched for years and now two
> > > contributions at the same time.
> > 
> > Three actually. Michael also submitted a series :)
> > 
> > > Sebastian, what is the current state of your series?
> > 
> > The DT changes got Ack'd by Rob and I have the R-B from Michael
> > (minus a minor comment to make the panel struct 'static const').
> > It's mainly waiting for a review from Sam.
> > 
> > I was a bit distracted by a boot regression on the devices and
> > some other projects. The boot regression got solved, so I can
> > prepare a new version if that makes things easier.
> > 
> > > Shall I base my work on top of yours? Or is it still too
> > > premature and we shall instead try to merge both and contribute a new
> > > version of the series bringing support for the two panels?
> > 
> > I suppose whatever is easier for Sam to review.
> 
> Hi Sebastian.
> 
> Too much panel stuff going on, so I miss the most and I am happy
> to see other people do a lot of good work here. Can i get a
> pointer to lore or so, then I will try to take a look.

Sure, 

Michael Riesch already referenced it earlier in this thread:

[0] https://lore.kernel.org/dri-devel/20230422205012.2464933-1-sre@kernel.org/

Thanks for taking a look,

-- Sebastian
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
index bbc4569cbcdc..c67b9adb157f 100644
--- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
+++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
@@ -400,9 +400,16 @@  static const struct of_device_id st7789v_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, st7789v_of_match);
 
+static const struct spi_device_id st7789v_ids[] = {
+	{ "st7789v", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, st7789v_ids);
+
 static struct spi_driver st7789v_driver = {
 	.probe = st7789v_probe,
 	.remove = st7789v_remove,
+	.id_table = st7789v_ids,
 	.driver = {
 		.name = "st7789v",
 		.of_match_table = st7789v_of_match,