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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Michael Riesch <michael.riesch@wolfvision.net>
Cc: devicetree@vger.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Sebastian Reichel <sebastian.reichel@collabora.com>,
	dri-devel@lists.freedesktop.org, Rob Herring <robh+dt@kernel.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Maxime Ripard <mripard@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>
Subject: Re: [PATCH 1/7] drm/panel: sitronix-st7789v: Prevent core spi warnings
Date: Tue, 13 Jun 2023 08:56:30 +0200	[thread overview]
Message-ID: <20230613085630.4a48fa8b@xps-13> (raw)
In-Reply-To: <b4ae6e9c-b548-b1e3-42f9-e24aacaf2b38@wolfvision.net>

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