All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Brian Masney <masneyb@onstation.org>
Cc: linux-iio@vger.kernel.org, gregkh@linuxfoundation.org,
	devel@driverdev.osuosl.org, knaack.h@gmx.de, lars@metafoo.de,
	pmeerw@pmeerw.net, linux-kernel@vger.kernel.org,
	Jon.Brenner@ams.com
Subject: Re: [PATCH 2/3] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec
Date: Sun, 1 Oct 2017 11:10:54 +0100	[thread overview]
Message-ID: <20171001111054.5eff7239@archlinux> (raw)
In-Reply-To: <20170930010921.15447-3-masneyb@onstation.org>

On Fri, 29 Sep 2017 21:09:20 -0400
Brian Masney <masneyb@onstation.org> wrote:

> The driver explicitly creates the in_illuminance0_integration_time sysfs
> attribute outside the IIO core. This attribute is available in the IIO
> core so this patches migrates the attribute to be created by
> the iio_chan_spec.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>

One comment inline, but a general observation rather than about this
patch so applied to the togreg branch of iio.git and pushed out as
testing.

Thanks,

Jonathan
> ---
> Changes since v1 (Jul 9 2017):
> - Use MIN_ITIME instead of hardcoded 3 in tsl2x7x_write_raw()
> 
>  drivers/staging/iio/light/tsl2x7x.c | 65 ++++++++++---------------------------
>  1 file changed, 17 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index e340ea624e5c..e6a71f5fc9cb 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -898,45 +898,6 @@ static ssize_t in_proximity0_calibscale_available_show(struct device *dev,
>  		return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
>  }
>  
> -static ssize_t in_illuminance0_integration_time_show(struct device *dev,
> -				     struct device_attribute *attr,
> -				     char *buf)
> -{
> -	struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
> -	int y, z;
> -
> -	y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->settings.als_time) + 1;
> -	z = y * TSL2X7X_MIN_ITIME;
> -	y /= 1000;
> -	z %= 1000;
> -
> -	return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
> -}
> -
> -static ssize_t in_illuminance0_integration_time_store(struct device *dev,
> -				      struct device_attribute *attr,
> -				      const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> -	struct tsl2x7x_parse_result result;
> -	int ret;
> -
> -	ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
> -	if (ret)
> -		return ret;
> -
> -	result.fract /= 3;
> -	chip->settings.als_time = TSL2X7X_MAX_TIMER_CNT - (u8)result.fract;
> -
> -	dev_info(&chip->client->dev, "%s: als time = %d",
> -		 __func__, chip->settings.als_time);
> -
> -	tsl2x7x_invoke_change(indio_dev);
> -
> -	return IIO_VAL_INT_PLUS_MICRO;
> -}
> -
>  static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
>  		".00272 - .696");
>  
> @@ -1377,7 +1338,11 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
>  		*val = chip->settings.als_gain_trim;
>  		ret = IIO_VAL_INT;
>  		break;
> -
> +	case IIO_CHAN_INFO_INT_TIME:
> +		*val = (TSL2X7X_MAX_TIMER_CNT - chip->settings.als_time) + 1;
> +		*val2 = ((*val * TSL2X7X_MIN_ITIME) % 1000) / 1000;
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -1453,7 +1418,13 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_CALIBBIAS:
>  		chip->settings.als_gain_trim = val;
>  		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		chip->settings.als_time =
> +			TSL2X7X_MAX_TIMER_CNT - (val2 / TSL2X7X_MIN_ITIME);
>  
> +		dev_info(&chip->client->dev, "%s: als time = %d",
> +			 __func__, chip->settings.als_time);
> +		break;

I'm not sure this dev_info is terribly useful for anything - however
not an issue with this patch as you have simply moved it..

>  	default:
>  		return -EINVAL;
>  	}
> @@ -1465,8 +1436,6 @@ static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>  
>  static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
>  
> -static DEVICE_ATTR_RW(in_illuminance0_integration_time);
> -
>  static DEVICE_ATTR_RW(in_illuminance0_target_input);
>  
>  static DEVICE_ATTR_WO(in_illuminance0_calibrate);
> @@ -1546,7 +1515,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1561,7 +1529,6 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1578,7 +1545,6 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&dev_attr_in_illuminance0_integration_time.attr,
>  	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
> @@ -1711,7 +1677,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,
> @@ -1750,7 +1717,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,
> @@ -1798,7 +1766,8 @@ static const struct tsl2x7x_chip_info tsl2x7x_chip_info_tbl[] = {
>  			.type = IIO_LIGHT,
>  			.indexed = 1,
>  			.channel = 0,
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +			.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +					      BIT(IIO_CHAN_INFO_INT_TIME),
>  			}, {
>  			.type = IIO_INTENSITY,
>  			.indexed = 1,

  reply	other threads:[~2017-10-01 10:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-30  1:09 [PATCH 0/3] staging: iio: tsl2x7x: staging cleanups Brian Masney
2017-09-30  1:09 ` [PATCH 1/3] staging: iio: tsl2x7x: rename tsl2x7x_settings variable to settings Brian Masney
2017-10-01 10:08   ` Jonathan Cameron
2017-09-30  1:09 ` [PATCH 2/3] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec Brian Masney
2017-10-01 10:10   ` Jonathan Cameron [this message]
2017-09-30  1:09 ` [PATCH 3/3] staging: iio: tsl2x7x: migrate *_thresh_period sysfs attributes to iio_event_spec Brian Masney
2017-10-01 10:14   ` Jonathan Cameron
2017-10-01 10:58     ` Brian Masney

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=20171001111054.5eff7239@archlinux \
    --to=jic23@kernel.org \
    --cc=Jon.Brenner@ams.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masneyb@onstation.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.