All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-iio@vger.kernel.org, Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Subject: Re: [PATCH v1 2/2] iio: adc: ti-ads1015: Make use of device property API
Date: Sat, 7 Dec 2019 10:51:55 +0000	[thread overview]
Message-ID: <20191207105155.7a5f80f5@archlinux> (raw)
In-Reply-To: <20191203111753.55146-2-andriy.shevchenko@linux.intel.com>

On Tue,  3 Dec 2019 13:17:53 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Make use of device property API in this driver so that both OF based
> system and ACPI based system can use this driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Really minor comment...

Using the construction around i = -1, then inc to 0 on first channel
is a bit of a convoluted way of setting what is at heart a boolean flag
to say 1 or more channels had description.

Otherwise seems fine to me and that's minor enough lets just ignore it.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan


> ---
>  drivers/iio/adc/ti-ads1015.c | 59 +++++++++++++++++-------------------
>  1 file changed, 27 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 3b123b4f0b99..a128236ec3cc 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -12,10 +12,10 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/of_device.h>
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/i2c.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/mutex.h>
> @@ -77,6 +77,7 @@
>  #define ADS1015_DEFAULT_CHAN		0
>  
>  enum chip_ids {
> +	ADSXXXX = 0,
>  	ADS1015,
>  	ADS1115,
>  };
> @@ -843,65 +844,58 @@ static const struct iio_info ads1115_info = {
>  	.attrs          = &ads1115_attribute_group,
>  };
>  
> -#ifdef CONFIG_OF
> -static int ads1015_get_channels_config_of(struct i2c_client *client)
> +static int ads1015_client_get_channels_config(struct i2c_client *client)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>  	struct ads1015_data *data = iio_priv(indio_dev);
> -	struct device_node *node;
> +	struct device *dev = &client->dev;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev), *child;
> +	int i = -1;
>  
> -	if (!client->dev.of_node ||
> -	    !of_get_next_child(client->dev.of_node, NULL))
> -		return -EINVAL;
> -
> -	for_each_child_of_node(client->dev.of_node, node) {
> +	fwnode_for_each_child_node(fwnode, child) {
>  		u32 pval;
>  		unsigned int channel;
>  		unsigned int pga = ADS1015_DEFAULT_PGA;
>  		unsigned int data_rate = ADS1015_DEFAULT_DATA_RATE;
>  
> -		if (of_property_read_u32(node, "reg", &pval)) {
> -			dev_err(&client->dev, "invalid reg on %pOF\n",
> -				node);
> +		if (fwnode_property_read_u32(child, "reg", &pval)) {
> +			dev_err(dev, "invalid reg on %pfw\n", child);
>  			continue;
>  		}
>  
>  		channel = pval;
>  		if (channel >= ADS1015_CHANNELS) {
> -			dev_err(&client->dev,
> -				"invalid channel index %d on %pOF\n",
> -				channel, node);
> +			dev_err(dev, "invalid channel index %d on %pfw\n",
> +				channel, child);
>  			continue;
>  		}
>  
> -		if (!of_property_read_u32(node, "ti,gain", &pval)) {
> +		if (!fwnode_property_read_u32(child, "ti,gain", &pval)) {
>  			pga = pval;
>  			if (pga > 6) {
> -				dev_err(&client->dev, "invalid gain on %pOF\n",
> -					node);
> -				of_node_put(node);
> +				dev_err(dev, "invalid gain on %pfw\n", child);
> +				fwnode_handle_put(child);
>  				return -EINVAL;
>  			}
>  		}
>  
> -		if (!of_property_read_u32(node, "ti,datarate", &pval)) {
> +		if (!fwnode_property_read_u32(child, "ti,datarate", &pval)) {
>  			data_rate = pval;
>  			if (data_rate > 7) {
> -				dev_err(&client->dev,
> -					"invalid data_rate on %pOF\n",
> -					node);
> -				of_node_put(node);
> +				dev_err(dev, "invalid data_rate on %pfw\n", child);
> +				fwnode_handle_put(child);
>  				return -EINVAL;
>  			}
>  		}
>  
>  		data->channel_data[channel].pga = pga;
>  		data->channel_data[channel].data_rate = data_rate;
> +
> +		i++;
>  	}
>  
> -	return 0;
> +	return i < 0 ? -EINVAL : 0;
>  }
> -#endif
>  
>  static void ads1015_get_channels_config(struct i2c_client *client)
>  {
> @@ -910,10 +904,9 @@ static void ads1015_get_channels_config(struct i2c_client *client)
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>  	struct ads1015_data *data = iio_priv(indio_dev);
>  
> -#ifdef CONFIG_OF
> -	if (!ads1015_get_channels_config_of(client))
> +	if (!ads1015_client_get_channels_config(client))
>  		return;
> -#endif
> +
>  	/* fallback on default configuration */
>  	for (k = 0; k < ADS1015_CHANNELS; ++k) {
>  		data->channel_data[k].pga = ADS1015_DEFAULT_PGA;
> @@ -951,9 +944,8 @@ static int ads1015_probe(struct i2c_client *client,
>  	indio_dev->name = ADS1015_DRV_NAME;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	if (client->dev.of_node)
> -		chip = (enum chip_ids)of_device_get_match_data(&client->dev);
> -	else
> +	chip = (enum chip_ids)device_get_match_data(&client->dev);
> +	if (chip == ADSXXXX)
>  		chip = id->driver_data;
>  	switch (chip) {
>  	case ADS1015:
> @@ -968,6 +960,9 @@ static int ads1015_probe(struct i2c_client *client,
>  		indio_dev->info = &ads1115_info;
>  		data->data_rate = (unsigned int *) &ads1115_data_rate;
>  		break;
> +	default:
> +		dev_err(&client->dev, "Unknown chip %d\n", chip);
> +		return -EINVAL;
>  	}
>  
>  	data->event_channel = ADS1015_CHANNELS;


  reply	other threads:[~2019-12-07 10:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03 11:17 [PATCH v1 1/2] iio: adc: ti-ads1025: Get rid of legacy platform data Andy Shevchenko
2019-12-03 11:17 ` [PATCH v1 2/2] iio: adc: ti-ads1015: Make use of device property API Andy Shevchenko
2019-12-07 10:51   ` Jonathan Cameron [this message]
2019-12-07 10:50 ` [PATCH v1 1/2] iio: adc: ti-ads1025: Get rid of legacy platform data Jonathan Cameron

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=20191207105155.7a5f80f5@archlinux \
    --to=jic23@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    /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.