[PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Andy Shevchenko]

There is one OF call in the driver that limits its area of use.
Replace it to generic device_get_match_data() and get rid of OF dependency.

While here, cast SPI driver data to certain enumerator type.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/tiny/repaper.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 76d179200775..fd9e95ce3201 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -17,7 +17,7 @@
 #include <linux/dma-buf.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 #include <linux/sched/clock.h>
 #include <linux/spi/spi.h>
 #include <linux/thermal.h>
@@ -40,6 +40,7 @@
 #define REPAPER_RID_G2_COG_ID	0x12
 
 enum repaper_model {
+	EXXXXCSXXX = 0,
 	E1144CS021 = 1,
 	E1190CS021,
 	E2200CS021,
@@ -995,21 +996,21 @@ static int repaper_probe(struct spi_device *spi)
 {
 	const struct drm_display_mode *mode;
 	const struct spi_device_id *spi_id;
-	const struct of_device_id *match;
 	struct device *dev = &spi->dev;
 	enum repaper_model model;
 	const char *thermal_zone;
 	struct repaper_epd *epd;
 	size_t line_buffer_size;
 	struct drm_device *drm;
+	const void *match;
 	int ret;
 
-	match = of_match_device(repaper_of_match, dev);
+	match = device_get_match_data(dev);
 	if (match) {
-		model = (enum repaper_model)match->data;
+		model = (enum repaper_model)match;
 	} else {
 		spi_id = spi_get_device_id(spi);
-		model = spi_id->driver_data;
+		model = (enum repaper_model)spi_id->driver_data;
 	}
 
 	/* The SPI device is used to allocate dma memory */
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 2/4] drm/tiny/repaper: No need to set ->owner for spi_register_driver()

[Andy Shevchenko]

The spi_register_driver() will set the ->owner member to THIS_MODULE.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/tiny/repaper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index fd9e95ce3201..4a497203923f 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -1198,7 +1198,6 @@ static void repaper_shutdown(struct spi_device *spi)
 static struct spi_driver repaper_spi_driver = {
 	.driver = {
 		.name = "repaper",
-		.owner = THIS_MODULE,
 		.of_match_table = repaper_of_match,
 	},
 	.id_table = repaper_id,
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 3/4] drm/tiny/st7735r: Make driver OF-independent

[Andy Shevchenko]

There is one OF call in the driver that limits its area of use.
Replace it to generic device_get_match_data() and get rid of OF dependency.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/tiny/st7735r.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
index 32574f1b6071..a844cde6d14a 100644
--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -12,7 +12,6 @@
 #include <linux/dma-buf.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
 #include <linux/property.h>
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
@@ -192,7 +191,7 @@ static int st7735r_probe(struct spi_device *spi)
 	u32 rotation = 0;
 	int ret;
 
-	cfg = of_device_get_match_data(&spi->dev);
+	cfg = device_get_match_data(&spi->dev);
 	if (!cfg)
 		cfg = (void *)spi_get_device_id(spi)->driver_data;
 
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1 4/4] drm/tiny/st7735r: No need to set ->owner for spi_register_driver()

[Andy Shevchenko]

The spi_register_driver() will set the ->owner member to THIS_MODULE.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/tiny/st7735r.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
index a844cde6d14a..3cd9b8d9888d 100644
--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -276,7 +276,6 @@ static void st7735r_shutdown(struct spi_device *spi)
 static struct spi_driver st7735r_spi_driver = {
 	.driver = {
 		.name = "st7735r",
-		.owner = THIS_MODULE,
 		.of_match_table = st7735r_of_match,
 	},
 	.id_table = st7735r_id,
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 4/4] drm/tiny/st7735r: No need to set ->owner for spi_register_driver()

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:
> The spi_register_driver() will set the ->owner member to THIS_MODULE.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Reviewed-by: David Lechner <david@lechnology.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 3/4] drm/tiny/st7735r: Make driver OF-independent

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:
> There is one OF call in the driver that limits its area of use.
> Replace it to generic device_get_match_data() and get rid of OF dependency.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Acked-by: David Lechner <david@lechnology.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/4] drm/tiny/repaper: No need to set ->owner for spi_register_driver()

[David Lechner]

On 1/22/20 4:54 AM, Andy Shevchenko wrote:
> The spi_register_driver() will set the ->owner member to THIS_MODULE.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
Reviewed-by: David Lechner <david@lechnology.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Sam Ravnborg]

Hi Andy.


On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> There is one OF call in the driver that limits its area of use.
> Replace it to generic device_get_match_data() and get rid of OF dependency.
> 
> While here, cast SPI driver data to certain enumerator type.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpu/drm/tiny/repaper.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index 76d179200775..fd9e95ce3201 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -17,7 +17,7 @@
>  #include <linux/dma-buf.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/module.h>
> -#include <linux/of_device.h>
> +#include <linux/property.h>
>  #include <linux/sched/clock.h>
>  #include <linux/spi/spi.h>
>  #include <linux/thermal.h>
> @@ -40,6 +40,7 @@
>  #define REPAPER_RID_G2_COG_ID	0x12
>  
>  enum repaper_model {
> +	EXXXXCSXXX = 0,
>  	E1144CS021 = 1,
>  	E1190CS021,
>  	E2200CS021,
The new enum value is not used in the following - is it necessary?

	Sam


> @@ -995,21 +996,21 @@ static int repaper_probe(struct spi_device *spi)
>  {
>  	const struct drm_display_mode *mode;
>  	const struct spi_device_id *spi_id;
> -	const struct of_device_id *match;
>  	struct device *dev = &spi->dev;
>  	enum repaper_model model;
>  	const char *thermal_zone;
>  	struct repaper_epd *epd;
>  	size_t line_buffer_size;
>  	struct drm_device *drm;
> +	const void *match;
>  	int ret;
>  
> -	match = of_match_device(repaper_of_match, dev);
> +	match = device_get_match_data(dev);
>  	if (match) {
> -		model = (enum repaper_model)match->data;
> +		model = (enum repaper_model)match;
>  	} else {
>  		spi_id = spi_get_device_id(spi);
> -		model = spi_id->driver_data;
> +		model = (enum repaper_model)spi_id->driver_data;
>  	}
>  
>  	/* The SPI device is used to allocate dma memory */
> -- 
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/4] drm/tiny/repaper: No need to set ->owner for spi_register_driver()

[Sam Ravnborg]

Hi Andy.

On Wed, Jan 22, 2020 at 12:54:01PM +0200, Andy Shevchenko wrote:
> The spi_register_driver() will set the ->owner member to THIS_MODULE.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Any chance you will update the remaining 3 drivers in drm/tiny/
that has the same unessesary assignment?

	Sam

> ---
>  drivers/gpu/drm/tiny/repaper.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index fd9e95ce3201..4a497203923f 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -1198,7 +1198,6 @@ static void repaper_shutdown(struct spi_device *spi)
>  static struct spi_driver repaper_spi_driver = {
>  	.driver = {
>  		.name = "repaper",
> -		.owner = THIS_MODULE,
>  		.of_match_table = repaper_of_match,
>  	},
>  	.id_table = repaper_id,
> -- 
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Andy Shevchenko]

On Fri, Jan 24, 2020 at 05:42:33PM +0100, Sam Ravnborg wrote:
> On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> > There is one OF call in the driver that limits its area of use.
> > Replace it to generic device_get_match_data() and get rid of OF dependency.
> > 
> > While here, cast SPI driver data to certain enumerator type.

> >  enum repaper_model {
> > +	EXXXXCSXXX = 0,
> >  	E1144CS021 = 1,
> >  	E1190CS021,
> >  	E2200CS021,
> The new enum value is not used in the following - is it necessary?

Yes. It explicitly prevents to use 0 for real device.

This is due to device_get_match_data() returns content of data pointer and thus
we may not distinguish 0 from NULL pointer.

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/4] drm/tiny/repaper: No need to set ->owner for spi_register_driver()

[Andy Shevchenko]

On Fri, Jan 24, 2020 at 06:06:37PM +0100, Sam Ravnborg wrote:
> Hi Andy.
> 
> On Wed, Jan 22, 2020 at 12:54:01PM +0200, Andy Shevchenko wrote:
> > The spi_register_driver() will set the ->owner member to THIS_MODULE.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Thanks.

> Any chance you will update the remaining 3 drivers in drm/tiny/
> that has the same unessesary assignment?

Maybe.

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Sam Ravnborg]

Hi Andy.

On Fri, Jan 24, 2020 at 07:31:34PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 24, 2020 at 05:42:33PM +0100, Sam Ravnborg wrote:
> > On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> > > There is one OF call in the driver that limits its area of use.
> > > Replace it to generic device_get_match_data() and get rid of OF dependency.
> > > 
> > > While here, cast SPI driver data to certain enumerator type.
> 
> > >  enum repaper_model {
> > > +	EXXXXCSXXX = 0,
> > >  	E1144CS021 = 1,
> > >  	E1190CS021,
> > >  	E2200CS021,
> > The new enum value is not used in the following - is it necessary?
> 
> Yes. It explicitly prevents to use 0 for real device.
> 
> This is due to device_get_match_data() returns content of data pointer and thus
> we may not distinguish 0 from NULL pointer.
A name that told this was not a valid name would be descriptive.
As it is now it looks like a wildcard that matches everythign else.

With a more descriptive name:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 4/4] drm/tiny/st7735r: No need to set ->owner for spi_register_driver()

[Daniel Vetter]

On Wed, Jan 22, 2020 at 10:00:58AM -0600, David Lechner wrote:
> On 1/22/20 4:54 AM, Andy Shevchenko wrote:
> > The spi_register_driver() will set the ->owner member to THIS_MODULE.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> Reviewed-by: David Lechner <david@lechnology.com>

Btw to avoid confusion: Since your committer (and work in this area)
everyone else will assume you're going to push this. If not, then you have
to be explicit about that (or the patch will simply get lost). But best is
you just go ahead and push it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Andy Shevchenko]

On Fri, Jan 24, 2020 at 07:18:12PM +0100, Sam Ravnborg wrote:
> On Fri, Jan 24, 2020 at 07:31:34PM +0200, Andy Shevchenko wrote:
> > On Fri, Jan 24, 2020 at 05:42:33PM +0100, Sam Ravnborg wrote:
> > > On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> > > > There is one OF call in the driver that limits its area of use.
> > > > Replace it to generic device_get_match_data() and get rid of OF dependency.
> > > > 
> > > > While here, cast SPI driver data to certain enumerator type.
> > 
> > > >  enum repaper_model {
> > > > +	EXXXXCSXXX = 0,
> > > >  	E1144CS021 = 1,
> > > >  	E1190CS021,
> > > >  	E2200CS021,
> > > The new enum value is not used in the following - is it necessary?
> > 
> > Yes. It explicitly prevents to use 0 for real device.
> > 
> > This is due to device_get_match_data() returns content of data pointer and thus
> > we may not distinguish 0 from NULL pointer.
> A name that told this was not a valid name would be descriptive.
> As it is now it looks like a wildcard that matches everythign else.

Can you be more precise what you would like to see?
Perhaps simple comment will help?

> With a more descriptive name:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Sam Ravnborg]

Hi Andy.

On Mon, Jan 27, 2020 at 11:39:39AM +0200, Andy Shevchenko wrote:
> On Fri, Jan 24, 2020 at 07:18:12PM +0100, Sam Ravnborg wrote:
> > On Fri, Jan 24, 2020 at 07:31:34PM +0200, Andy Shevchenko wrote:
> > > On Fri, Jan 24, 2020 at 05:42:33PM +0100, Sam Ravnborg wrote:
> > > > On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> > > > > There is one OF call in the driver that limits its area of use.
> > > > > Replace it to generic device_get_match_data() and get rid of OF dependency.
> > > > > 
> > > > > While here, cast SPI driver data to certain enumerator type.
> > > 
> > > > >  enum repaper_model {
> > > > > +	EXXXXCSXXX = 0,
> > > > >  	E1144CS021 = 1,
> > > > >  	E1190CS021,
> > > > >  	E2200CS021,
> > > > The new enum value is not used in the following - is it necessary?
> > > 
> > > Yes. It explicitly prevents to use 0 for real device.
> > > 
> > > This is due to device_get_match_data() returns content of data pointer and thus
> > > we may not distinguish 0 from NULL pointer.
> > A name that told this was not a valid name would be descriptive.
> > As it is now it looks like a wildcard that matches everythign else.
> 
> Can you be more precise what you would like to see?
> Perhaps simple comment will help?

Maybe just add something like:
/* 0 is reserved to avoid clashing with NULL */

And then skip the, at least to my eyes, cryptic EXXXXCSXXX.
Up to you.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/4] drm/tiny/repaper: Make driver OF-independent

[Andy Shevchenko]

On Wed, Jan 29, 2020 at 09:29:14PM +0100, Sam Ravnborg wrote:
> On Mon, Jan 27, 2020 at 11:39:39AM +0200, Andy Shevchenko wrote:
> > On Fri, Jan 24, 2020 at 07:18:12PM +0100, Sam Ravnborg wrote:
> > > On Fri, Jan 24, 2020 at 07:31:34PM +0200, Andy Shevchenko wrote:
> > > > On Fri, Jan 24, 2020 at 05:42:33PM +0100, Sam Ravnborg wrote:
> > > > > On Wed, Jan 22, 2020 at 12:54:00PM +0200, Andy Shevchenko wrote:
> > > > > > There is one OF call in the driver that limits its area of use.
> > > > > > Replace it to generic device_get_match_data() and get rid of OF dependency.
> > > > > > 
> > > > > > While here, cast SPI driver data to certain enumerator type.
> > > > 
> > > > > >  enum repaper_model {
> > > > > > +	EXXXXCSXXX = 0,
> > > > > >  	E1144CS021 = 1,
> > > > > >  	E1190CS021,
> > > > > >  	E2200CS021,
> > > > > The new enum value is not used in the following - is it necessary?
> > > > 
> > > > Yes. It explicitly prevents to use 0 for real device.
> > > > 
> > > > This is due to device_get_match_data() returns content of data pointer and thus
> > > > we may not distinguish 0 from NULL pointer.
> > > A name that told this was not a valid name would be descriptive.
> > > As it is now it looks like a wildcard that matches everythign else.
> > 
> > Can you be more precise what you would like to see?
> > Perhaps simple comment will help?
> 
> Maybe just add something like:
> /* 0 is reserved to avoid clashing with NULL */
> 
> And then skip the, at least to my eyes, cryptic EXXXXCSXXX.
> Up to you.

Fine with me, I'll update accordingly.
Thanks!

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel