Re: [PATCH v4] staging: iio: cdc: ad7152: Implement IIO_CHAN_INFO_SAMP_FREQ attribute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 9 October 2016 19:26:52 BST, sayli karnik <karniksayli1995@xxxxxxxxx> wrote:
>Attributes that were once privately defined become standard with time
>and
>hence a special global define is used. Hence update driver ad7152 to
>use
>IIO_CHAN_INFO_SAMP_FREQ which is a global define instead of
>IIO_DEV_ATTR_SAMP_FREQ.
>Move functionality from IIO_DEV_ATTR_SAMP_FREQ attribute into
>IIO_CHAN_INFO_SAMP_FREQ to implement the sampling_frequency attribute.
>Modify ad7152_read_raw() and ad7152_write_raw() to allow reading and
>writing the element as well. Also add a lock in the driver's private
>data.
>
>Signed-off-by: sayli karnik <karniksayli1995@xxxxxxxxx>

Sorry Sayli,

I was not clear enough. The block to local lock change would make a good extra patch.

It is a separate change so should be a separate patch. I have already applied v3 of the patch
 without the mlock change.

https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/log/?h=testing

Such an mlock replacing patch needs to replace all uses of mlock in the driver. Also make sure
 you initialise the mutex appropriately.

Jonathan


>---
>Changes in v4:
>Replaced mlock with a local lock since mlock is intended to protect
>'only'
>switches between modes. Given this driver doesn't support more than one
>mode
>(sysfs polled reads only)
>
>Changes in v3:
>Drop the unlock in ad7152_write_raw_samp_freq() and use the one at the
>exit point
>instead
>Return ret immediately instead of using a goto statement in
>ad7152_write_raw_samp_freq()
>
>Changes in v2:
>Give a more detailed explanation
>Remove null check for val in ad7152_write_raw_samp_freq()
>Merged two stucture variable initialisations into one statement in
>ad7152_read_raw_samp_freq()
>Set ret to IIO_VAL_INT in ad7152_read_raw() when mask equals
>IIO_CHAN_INFO_SAMP_FREQ and ret>=0
>drivers/staging/iio/cdc/ad7152.c | 117
>++++++++++++++++++++++-----------------
> 1 file changed, 66 insertions(+), 51 deletions(-)
>
>diff --git a/drivers/staging/iio/cdc/ad7152.c
>b/drivers/staging/iio/cdc/ad7152.c
>index 485d0a5..0ce3849 100644
>--- a/drivers/staging/iio/cdc/ad7152.c
>+++ b/drivers/staging/iio/cdc/ad7152.c
>@@ -89,6 +89,7 @@ struct ad7152_chip_info {
> 	 */
> 	u8	filter_rate_setup;
> 	u8	setup[2];
>+	struct mutex state_lock;	/* protect hardware state */
> };
> 
> static inline ssize_t ad7152_start_calib(struct device *dev,
>@@ -165,63 +166,12 @@ static const unsigned char
>ad7152_filter_rate_table[][2] = {
> 	{200, 5 + 1}, {50, 20 + 1}, {20, 50 + 1}, {17, 60 + 1},
> };
> 
>-static ssize_t ad7152_show_filter_rate_setup(struct device *dev,
>-		struct device_attribute *attr,
>-		char *buf)
>-{
>-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>-	struct ad7152_chip_info *chip = iio_priv(indio_dev);
>-
>-	return sprintf(buf, "%d\n",
>-		       ad7152_filter_rate_table[chip->filter_rate_setup][0]);
>-}
>-
>-static ssize_t ad7152_store_filter_rate_setup(struct device *dev,
>-		struct device_attribute *attr,
>-		const char *buf,
>-		size_t len)
>-{
>-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>-	struct ad7152_chip_info *chip = iio_priv(indio_dev);
>-	u8 data;
>-	int ret, i;
>-
>-	ret = kstrtou8(buf, 10, &data);
>-	if (ret < 0)
>-		return ret;
>-
>-	for (i = 0; i < ARRAY_SIZE(ad7152_filter_rate_table); i++)
>-		if (data >= ad7152_filter_rate_table[i][0])
>-			break;
>-
>-	if (i >= ARRAY_SIZE(ad7152_filter_rate_table))
>-		i = ARRAY_SIZE(ad7152_filter_rate_table) - 1;
>-
>-	mutex_lock(&indio_dev->mlock);
>-	ret = i2c_smbus_write_byte_data(chip->client,
>-			AD7152_REG_CFG2, AD7152_CFG2_OSR(i));
>-	if (ret < 0) {
>-		mutex_unlock(&indio_dev->mlock);
>-		return ret;
>-	}
>-
>-	chip->filter_rate_setup = i;
>-	mutex_unlock(&indio_dev->mlock);
>-
>-	return len;
>-}
>-
>-static IIO_DEV_ATTR_SAMP_FREQ(S_IRUGO | S_IWUSR,
>-		ad7152_show_filter_rate_setup,
>-		ad7152_store_filter_rate_setup);
>-
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("200 50 20 17");
> 
> static IIO_CONST_ATTR(in_capacitance_scale_available,
> 		      "0.000061050 0.000030525 0.000015263 0.000007631");
> 
> static struct attribute *ad7152_attributes[] = {
>-	&iio_dev_attr_sampling_frequency.dev_attr.attr,
> 	&iio_dev_attr_in_capacitance0_calibbias_calibration.dev_attr.attr,
> 	&iio_dev_attr_in_capacitance1_calibbias_calibration.dev_attr.attr,
> 	&iio_dev_attr_in_capacitance0_calibscale_calibration.dev_attr.attr,
>@@ -247,6 +197,50 @@ static const int ad7152_scale_table[] = {
> 	30525, 7631, 15263, 61050
> };
> 
>+/**
>+ * read_raw handler for IIO_CHAN_INFO_SAMP_FREQ
>+ *
>+ * lock must be held
>+ **/
>+static int ad7152_read_raw_samp_freq(struct device *dev, int *val)
>+{
>+	struct ad7152_chip_info *chip = iio_priv(dev_to_iio_dev(dev));
>+
>+	*val = ad7152_filter_rate_table[chip->filter_rate_setup][0];
>+
>+	return 0;
>+}
>+
>+/**
>+ * write_raw handler for IIO_CHAN_INFO_SAMP_FREQ
>+ *
>+ * lock must be held
>+ **/
>+static int ad7152_write_raw_samp_freq(struct device *dev, int val)
>+{
>+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>+	struct ad7152_chip_info *chip = iio_priv(indio_dev);
>+	u8 data;
>+	int ret, i;
>+
>+	for (i = 0; i < ARRAY_SIZE(ad7152_filter_rate_table); i++)
>+		if (data >= ad7152_filter_rate_table[i][0])
>+			break;
>+
>+	if (i >= ARRAY_SIZE(ad7152_filter_rate_table))
>+		i = ARRAY_SIZE(ad7152_filter_rate_table) - 1;
>+
>+	mutex_lock(&chip->state_lock);
>+	ret = i2c_smbus_write_byte_data(chip->client,
>+					AD7152_REG_CFG2, AD7152_CFG2_OSR(i));
>+	if (ret < 0)
>+		return ret;
>+
>+	chip->filter_rate_setup = i;
>+	mutex_unlock(&chip->state_lock);
>+
>+	return ret;
>+}
> static int ad7152_write_raw(struct iio_dev *indio_dev,
> 			    struct iio_chan_spec const *chan,
> 			    int val,
>@@ -309,6 +303,16 @@ static int ad7152_write_raw(struct iio_dev
>*indio_dev,
> 
> 		ret = 0;
> 		break;
>+	case IIO_CHAN_INFO_SAMP_FREQ:
>+		if (val2)
>+			return -EINVAL;
>+
>+		ret = ad7152_write_raw_samp_freq(&indio_dev->dev, val);
>+		if (ret < 0)
>+			goto out;
>+
>+		ret = 0;
>+		break;
> 	default:
> 		ret = -EINVAL;
> 	}
>@@ -403,6 +407,13 @@ static int ad7152_read_raw(struct iio_dev
>*indio_dev,
> 
> 		ret = IIO_VAL_INT_PLUS_NANO;
> 		break;
>+	case IIO_CHAN_INFO_SAMP_FREQ:
>+		ret = ad7152_read_raw_samp_freq(&indio_dev->dev, val);
>+		if (ret < 0)
>+			goto out;
>+
>+		ret = IIO_VAL_INT;
>+		break;
> 	default:
> 		ret = -EINVAL;
> 	}
>@@ -440,6 +451,7 @@ static const struct iio_chan_spec ad7152_channels[]
>= {
> 		BIT(IIO_CHAN_INFO_CALIBSCALE) |
> 		BIT(IIO_CHAN_INFO_CALIBBIAS) |
> 		BIT(IIO_CHAN_INFO_SCALE),
>+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> 	}, {
> 		.type = IIO_CAPACITANCE,
> 		.differential = 1,
>@@ -450,6 +462,7 @@ static const struct iio_chan_spec ad7152_channels[]
>= {
> 		BIT(IIO_CHAN_INFO_CALIBSCALE) |
> 		BIT(IIO_CHAN_INFO_CALIBBIAS) |
> 		BIT(IIO_CHAN_INFO_SCALE),
>+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> 	}, {
> 		.type = IIO_CAPACITANCE,
> 		.indexed = 1,
>@@ -458,6 +471,7 @@ static const struct iio_chan_spec ad7152_channels[]
>= {
> 		BIT(IIO_CHAN_INFO_CALIBSCALE) |
> 		BIT(IIO_CHAN_INFO_CALIBBIAS) |
> 		BIT(IIO_CHAN_INFO_SCALE),
>+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> 	}, {
> 		.type = IIO_CAPACITANCE,
> 		.differential = 1,
>@@ -468,6 +482,7 @@ static const struct iio_chan_spec ad7152_channels[]
>= {
> 		BIT(IIO_CHAN_INFO_CALIBSCALE) |
> 		BIT(IIO_CHAN_INFO_CALIBBIAS) |
> 		BIT(IIO_CHAN_INFO_SCALE),
>+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> 	}
> };
> /*

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux