staging: fbtft: ili9341: use macro FBTFT_REGISTER_SPI_DRIVER

Message ID 20230702080324.120137-1-rgallaispou@gmail.com
State New
Headers
Series staging: fbtft: ili9341: use macro FBTFT_REGISTER_SPI_DRIVER |

Commit Message

Raphael Gallais-Pou July 2, 2023, 8:03 a.m. UTC
  Using FBTFT_REGISTER_DRIVER resolves to a NULL struct spi_device_id. This
ultimately causes the module to an early exit at probe time.
In addition the MODULE_ALIASes can be dropped.

Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
---
 drivers/staging/fbtft/fb_ili9341.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
  

Comments

Greg KH July 2, 2023, 12:02 p.m. UTC | #1
On Sun, Jul 02, 2023 at 10:03:24AM +0200, Raphael Gallais-Pou wrote:
> Using FBTFT_REGISTER_DRIVER resolves to a NULL struct spi_device_id. This
> ultimately causes the module to an early exit at probe time.

So this doesn't work at all today?  Has it ever worked?  What commit
does thi fix?

> In addition the MODULE_ALIASes can be dropped.

Why?  When you say "also" or "in addition", that's a huge hint it should
be a separate patch.

thanks,

greg k-h
  
Raphael Gallais-Pou July 2, 2023, 1:05 p.m. UTC | #2
Hi,

Le 02/07/2023 à 14:02, Greg Kroah-Hartman a écrit :
> On Sun, Jul 02, 2023 at 10:03:24AM +0200, Raphael Gallais-Pou wrote:
>> Using FBTFT_REGISTER_DRIVER resolves to a NULL struct spi_device_id. This
>> ultimately causes the module to an early exit at probe time.
> 
> So this doesn't work at all today?  Has it ever worked?  What commit
> does thi fix?

I tested again with only a tweak in my device-tree. The early exit in 
the driver's code is caused by a missing field. So regarding this 
particular driver the macro works.

It resolves to set spi_driver.id_table = NULL, which yields a warning in 
__spi_register_driver(). So I guess this patch only fixes a warning.

> 
>> In addition the MODULE_ALIASes can be dropped.
> 
> Why?  When you say "also" or "in addition", that's a huge hint it should
> be a separate patch.
I did not find any reference to those aliases in the kernel, which led 
me to remove those.

If you think they are still necessary, I'll split them in an upcoming v2.

Thanks for your insights,

Raphaël
> 
> thanks,
> 
> greg k-h
  
Greg KH July 3, 2023, 1:25 p.m. UTC | #3
On Sun, Jul 02, 2023 at 03:05:25PM +0200, Raphaël Gallais-Pou wrote:
> Hi,
> 
> Le 02/07/2023 à 14:02, Greg Kroah-Hartman a écrit :
> > On Sun, Jul 02, 2023 at 10:03:24AM +0200, Raphael Gallais-Pou wrote:
> > > Using FBTFT_REGISTER_DRIVER resolves to a NULL struct spi_device_id. This
> > > ultimately causes the module to an early exit at probe time.
> > 
> > So this doesn't work at all today?  Has it ever worked?  What commit
> > does thi fix?
> 
> I tested again with only a tweak in my device-tree. The early exit in the
> driver's code is caused by a missing field. So regarding this particular
> driver the macro works.
> 
> It resolves to set spi_driver.id_table = NULL, which yields a warning in
> __spi_register_driver(). So I guess this patch only fixes a warning.

Ok, please fix the changelog text when you resend this.

> > > In addition the MODULE_ALIASes can be dropped.
> > 
> > Why?  When you say "also" or "in addition", that's a huge hint it should
> > be a separate patch.
> I did not find any reference to those aliases in the kernel, which led me to
> remove those.

Aliases are used by userspace, not the kernel.

> If you think they are still necessary, I'll split them in an upcoming v2.

Please document why they are not needed in order to be able to be
removed.

thanks,

greg k-h
  

Patch

diff --git a/drivers/staging/fbtft/fb_ili9341.c b/drivers/staging/fbtft/fb_ili9341.c
index 9ccd0823c3ab..9528bf3cf711 100644
--- a/drivers/staging/fbtft/fb_ili9341.c
+++ b/drivers/staging/fbtft/fb_ili9341.c
@@ -145,12 +145,7 @@  static struct fbtft_display display = {
 	},
 };
 
-FBTFT_REGISTER_DRIVER(DRVNAME, "ilitek,ili9341", &display);
-
-MODULE_ALIAS("spi:" DRVNAME);
-MODULE_ALIAS("platform:" DRVNAME);
-MODULE_ALIAS("spi:ili9341");
-MODULE_ALIAS("platform:ili9341");
+FBTFT_REGISTER_SPI_DRIVER(DRVNAME, "ilitek", "ili9341", &display);
 
 MODULE_DESCRIPTION("FB driver for the ILI9341 LCD display controller");
 MODULE_AUTHOR("Christian Vogelgsang");