All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Slawomir Stepien <sst@poczta.fm>
Cc: lars@metafoo.de, knaack.h@gmx.de, pmeerw@pmeerw.net,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH 1/2] iio: potentiometer: mcp4131: use of_device_get_match_data()
Date: Fri, 16 Nov 2018 11:49:18 +0000	[thread overview]
Message-ID: <20181116114918.52b82372@archlinux> (raw)
In-Reply-To: <20181116094722.1070-2-sst@poczta.fm>

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 },

  reply	other threads:[~2018-11-16 22:01 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-16  9:47 [PATCH 0/2] Use OF API to get the driver data Slawomir Stepien
2018-11-16  9:47 ` [PATCH 1/2] iio: potentiometer: mcp4131: use of_device_get_match_data() Slawomir Stepien
2018-11-16 11:49   ` Jonathan Cameron [this message]
2018-11-16  9:47 ` [PATCH 2/2] iio: potentiometer mcp4131: use spi_get_device_id() only when needed Slawomir Stepien
2018-11-16 11:53   ` Jonathan Cameron
2018-11-16 12:19     ` Slawomir Stepien
2018-11-16 11:46 ` [PATCH 0/2] Use OF API to get the driver data Jonathan Cameron
2018-11-16 12:16   ` Slawomir Stepien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181116114918.52b82372@archlinux \
    --to=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --cc=sst@poczta.fm \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.