[PATCH 0/2] Use OF API to get the driver data

[PATCH 0/2] Use OF API to get the driver data

[Slawomir Stepien]

This patch series will change how the driver finds out the device data. Now it
will first try to use OF API and then, if needed, fallback to the spi framework
capabilities.

Slawomir Stepien (2):
  iio: potentiometer: mcp4131: use of_device_get_match_data()
  iio: potentiometer mcp4131: use spi_get_device_id() only when needed

 drivers/iio/potentiometer/mcp4131.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.19.1

[PATCH 1/2] iio: potentiometer: mcp4131: use of_device_get_match_data()

[Slawomir Stepien]

Try to get the device's data using OF API. In case of failure, fallback
to the spi_get_device_id() method.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 drivers/iio/potentiometer/mcp4131.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
index b3e30db246cc..55f526a59215 100644
--- a/drivers/iio/potentiometer/mcp4131.c
+++ b/drivers/iio/potentiometer/mcp4131.c
@@ -42,6 +42,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/spi/spi.h>
 
 #define MCP4131_WRITE		(0x00 << 2)
@@ -254,7 +255,9 @@ static int mcp4131_probe(struct spi_device *spi)
 	data = iio_priv(indio_dev);
 	spi_set_drvdata(spi, indio_dev);
 	data->spi = spi;
-	data->cfg = &mcp4131_cfg[devid];
+	data->cfg = of_device_get_match_data(&spi->dev);
+	if (!data->cfg)
+		data->cfg = &mcp4131_cfg[devid];
 
 	mutex_init(&data->lock);
 
@@ -273,7 +276,6 @@ static int mcp4131_probe(struct spi_device *spi)
 	return 0;
 }
 
-#if defined(CONFIG_OF)
 static const struct of_device_id mcp4131_dt_ids[] = {
 	{ .compatible = "microchip,mcp4131-502",
 		.data = &mcp4131_cfg[MCP413x_502] },
@@ -406,7 +408,6 @@ static const struct of_device_id mcp4131_dt_ids[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(of, mcp4131_dt_ids);
-#endif /* CONFIG_OF */
 
 static const struct spi_device_id mcp4131_id[] = {
 	{ "mcp4131-502", MCP413x_502 },
-- 
2.19.1

[PATCH 2/2] iio: potentiometer mcp4131: use spi_get_device_id() only when needed

[Slawomir Stepien]

There is no need to find the device id up front, since the
of_device_get_match_data() might return the correct data.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 drivers/iio/potentiometer/mcp4131.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
index 55f526a59215..efe035ce010d 100644
--- a/drivers/iio/potentiometer/mcp4131.c
+++ b/drivers/iio/potentiometer/mcp4131.c
@@ -244,7 +244,7 @@ static int mcp4131_probe(struct spi_device *spi)
 {
 	int err;
 	struct device *dev = &spi->dev;
-	unsigned long devid = spi_get_device_id(spi)->driver_data;
+	unsigned long devid;
 	struct mcp4131_data *data;
 	struct iio_dev *indio_dev;
 
@@ -256,8 +256,10 @@ static int mcp4131_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, indio_dev);
 	data->spi = spi;
 	data->cfg = of_device_get_match_data(&spi->dev);
-	if (!data->cfg)
+	if (!data->cfg) {
+		devid = spi_get_device_id(spi)->driver_data;
 		data->cfg = &mcp4131_cfg[devid];
+	}
 
 	mutex_init(&data->lock);
 
-- 
2.19.1

Re: [PATCH 0/2] Use OF API to get the driver data

[Jonathan Cameron]

On Fri, 16 Nov 2018 10:47:20 +0100
Slawomir Stepien <sst@poczta.fm> wrote:

> This patch series will change how the driver finds out the device data. Now it
> will first try to use OF API and then, if needed, fallback to the spi framework
> capabilities.

Hi Slawomir.

A quick process point.  The cover letter should definitely mention the driver
the patch is changing.

iio:potentiometer:mcp4131: Use OF API to get the driver data.

No need to resend for that though, just one for future reference!

Thanks,

Jonathan
> 
> Slawomir Stepien (2):
>   iio: potentiometer: mcp4131: use of_device_get_match_data()
>   iio: potentiometer mcp4131: use spi_get_device_id() only when needed
> 
>  drivers/iio/potentiometer/mcp4131.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

Re: [PATCH 1/2] iio: potentiometer: mcp4131: use of_device_get_match_data()

[Jonathan Cameron]

On Fri, 16 Nov 2018 10:47:21 +0100
Slawomir Stepien <sst@poczta.fm> wrote:

> Try to get the device's data using OF API. In case of failure, fallback
> to the spi_get_device_id() method.

The removing of the OF config guards is a good change, but probably wants
to be mentioned in the description.

One way I think this can be improved inline.

Thanks,

Jonathan

> 
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  drivers/iio/potentiometer/mcp4131.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
> index b3e30db246cc..55f526a59215 100644
> --- a/drivers/iio/potentiometer/mcp4131.c
> +++ b/drivers/iio/potentiometer/mcp4131.c
> @@ -42,6 +42,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/spi/spi.h>
>  
>  #define MCP4131_WRITE		(0x00 << 2)
> @@ -254,7 +255,9 @@ static int mcp4131_probe(struct spi_device *spi)
>  	data = iio_priv(indio_dev);
>  	spi_set_drvdata(spi, indio_dev);
>  	data->spi = spi;
> -	data->cfg = &mcp4131_cfg[devid];
> +	data->cfg = of_device_get_match_data(&spi->dev);
> +	if (!data->cfg)
> +		data->cfg = &mcp4131_cfg[devid];
The devid local variable is only used in this path. I would get rid of the
local variable entirely and just use spi_get_device_id(spi)->driver_data
directly here.

>  
>  	mutex_init(&data->lock);
>  
> @@ -273,7 +276,6 @@ static int mcp4131_probe(struct spi_device *spi)
>  	return 0;
>  }
>  
> -#if defined(CONFIG_OF)
>  static const struct of_device_id mcp4131_dt_ids[] = {
>  	{ .compatible = "microchip,mcp4131-502",
>  		.data = &mcp4131_cfg[MCP413x_502] },
> @@ -406,7 +408,6 @@ static const struct of_device_id mcp4131_dt_ids[] = {
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, mcp4131_dt_ids);
> -#endif /* CONFIG_OF */
>  
>  static const struct spi_device_id mcp4131_id[] = {
>  	{ "mcp4131-502", MCP413x_502 },

Re: [PATCH 2/2] iio: potentiometer mcp4131: use spi_get_device_id() only when needed

[Jonathan Cameron]

On Fri, 16 Nov 2018 10:47:22 +0100
Slawomir Stepien <sst@poczta.fm> wrote:

> There is no need to find the device id up front, since the
> of_device_get_match_data() might return the correct data.
> 
Ah. That explains it.   This should just have been merged with the previous
patch :)

Anyhow, I'll apply them both but merge them whilst doing so.

So applied as one patch to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

As things currently stand it's not worth going through and doing this for
every driver as I don't think the other path is being removed any
time soon.  It's a worthwhile change if people happen to be doing other
work on a particular driver though!

Thanks,

Jonathan

> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  drivers/iio/potentiometer/mcp4131.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
> index 55f526a59215..efe035ce010d 100644
> --- a/drivers/iio/potentiometer/mcp4131.c
> +++ b/drivers/iio/potentiometer/mcp4131.c
> @@ -244,7 +244,7 @@ static int mcp4131_probe(struct spi_device *spi)
>  {
>  	int err;
>  	struct device *dev = &spi->dev;
> -	unsigned long devid = spi_get_device_id(spi)->driver_data;
> +	unsigned long devid;
>  	struct mcp4131_data *data;
>  	struct iio_dev *indio_dev;
>  
> @@ -256,8 +256,10 @@ static int mcp4131_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, indio_dev);
>  	data->spi = spi;
>  	data->cfg = of_device_get_match_data(&spi->dev);
> -	if (!data->cfg)
> +	if (!data->cfg) {
> +		devid = spi_get_device_id(spi)->driver_data;
>  		data->cfg = &mcp4131_cfg[devid];
> +	}
>  
>  	mutex_init(&data->lock);
>  

Re: [PATCH 0/2] Use OF API to get the driver data

[Slawomir Stepien]

On lis 16, 2018 11:46, Jonathan Cameron wrote:
> On Fri, 16 Nov 2018 10:47:20 +0100
> Slawomir Stepien <sst@poczta.fm> wrote:
> 
> > This patch series will change how the driver finds out the device data. Now it
> > will first try to use OF API and then, if needed, fallback to the spi framework
> > capabilities.
> 
> Hi Slawomir.

Hi

> A quick process point.  The cover letter should definitely mention the driver
> the patch is changing.

Oops, sorry for that!

> iio:potentiometer:mcp4131: Use OF API to get the driver data.
> 
> No need to resend for that though, just one for future reference!
> 
> Thanks,
> 
> Jonathan
> > 
> > Slawomir Stepien (2):
> >   iio: potentiometer: mcp4131: use of_device_get_match_data()
> >   iio: potentiometer mcp4131: use spi_get_device_id() only when needed
> > 
> >  drivers/iio/potentiometer/mcp4131.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)

-- 
Slawomir Stepien

Re: [PATCH 2/2] iio: potentiometer mcp4131: use spi_get_device_id() only when needed

[Slawomir Stepien]

On lis 16, 2018 11:53, Jonathan Cameron wrote:
> On Fri, 16 Nov 2018 10:47:22 +0100
> Slawomir Stepien <sst@poczta.fm> wrote:
> 
> > There is no need to find the device id up front, since the
> > of_device_get_match_data() might return the correct data.
> > 
> Ah. That explains it.   This should just have been merged with the previous
> patch :)
> 
> Anyhow, I'll apply them both but merge them whilst doing so.

OK, that's fine for me.

> So applied as one patch to the togreg branch of iio.git and pushed out as
> testing for the autobuilders to play with it.
> 
> As things currently stand it's not worth going through and doing this for
> every driver as I don't think the other path is being removed any
> time soon.  It's a worthwhile change if people happen to be doing other
> work on a particular driver though!

OK.

Thank you for quick review.

-- 
Slawomir Stepien