Skip to contentSkip to conversation

drivers: iio: meter: ade7753: Replace IIO_DEV_ATTR_SAMP_FREQ attribute with IIO_CHAN_INFO_SAMP_FREQ handlers.

PATCH Superseded Sandhya Bankar 2 participants 3 messages Sep 29, 2016

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers: iio: meter: ade7753: Replace IIO_DEV_ATTR_SAMP_FREQ attribute with IIO_CHAN_INFO_SAMP_FREQ handlers.

Moved functionality from IIO_DEV_ATTR_SAMP_FREQ attribute into IIO_CHAN_INFO_SAMP_FREQ handlers. Added ade7753_read_raw() and ade7753_write_raw() to allow reading/writing the element as well.

---
 drivers/staging/iio/meter/ade7753.c | 155 +++++++++++++++++++++---------------
 1 file changed, 90 insertions(+), 65 deletions(-)
drivers/staging/iio/meter/ade7753.c +90 -65
index 4b5f05f..a2b4a18 100644
@@ -98,6 +98,94 @@ static int ade7753_spi_read_reg_16(struct device *dev,
 	return 0;
 }
 
+static int ade7753_read_raw_samp_freq(struct device *dev, int *val)
+{
+	int ret;
+	u16 t;
+
+	ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &t);
+	if (ret)
+		return ret;
+	t = (t >> 11) & 0x3;
+	*val = 27900 / (1 + t);
+
+	return 0;
+}
+
+static int ade7753_write_raw_samp_freq(struct device *dev, int val)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ade7753_state *st = iio_priv(indio_dev);
+	int ret;
+	u16 reg, t;
+
+	if (!val)
+		return -EINVAL;
+
+	t = 27900 / val;
+	if (t > 0)
+		t--;
+
+	if (t > 1)
+		st->us->max_speed_hz = ADE7753_SPI_SLOW;
+	else
+		st->us->max_speed_hz = ADE7753_SPI_FAST;
+
+	ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &reg);
+	if (ret)
+		goto out;
+
+	reg &= ~(3 << 11);
+	reg |= t << 11;
+
+	ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
+
+out:
+	return ret;
+}
+
+static int ade7753_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val2)
+			return -EINVAL;
+		mutex_lock(&indio_dev->mlock);
+		ret = ade7753_write_raw_samp_freq(&indio_dev->dev, val);
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int ade7753_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+		            int *val2,
+			    long mask)
+{
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		mutex_lock(&indio_dev->mlock);
+		ret = ade7753_read_raw_samp_freq(&indio_dev->dev, val);
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
 static int ade7753_spi_read_reg_24(struct device *dev,
 		u8 reg_address,
 		u32 *val)
@@ -383,82 +471,17 @@ err_ret:
 	return ret;
 }
 
-static ssize_t ade7753_read_frequency(struct device *dev,
-		struct device_attribute *attr,
-		char *buf)
-{
-	int ret;
-	u16 t;
-	int sps;
-
-	ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &t);
-	if (ret)
-		return ret;
-
-	t = (t >> 11) & 0x3;
-	sps = 27900 / (1 + t);
-
-	return sprintf(buf, "%d\n", sps);
-}
-
-static ssize_t ade7753_write_frequency(struct device *dev,
-		struct device_attribute *attr,
-		const char *buf,
-		size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ade7753_state *st = iio_priv(indio_dev);
-	u16 val;
-	int ret;
-	u16 reg, t;
-
-	ret = kstrtou16(buf, 10, &val);
-	if (ret)
-		return ret;
-	if (!val)
-		return -EINVAL;
-
-	mutex_lock(&indio_dev->mlock);
-
-	t = 27900 / val;
-	if (t > 0)
-		t--;
-
-	if (t > 1)
-		st->us->max_speed_hz = ADE7753_SPI_SLOW;
-	else
-		st->us->max_speed_hz = ADE7753_SPI_FAST;
-
-	ret = ade7753_spi_read_reg_16(dev, ADE7753_MODE, &reg);
-	if (ret)
-		goto out;
-
-	reg &= ~(3 << 11);
-	reg |= t << 11;
-
-	ret = ade7753_spi_write_reg_16(dev, ADE7753_MODE, reg);
-
-out:
-	mutex_unlock(&indio_dev->mlock);
-
-	return ret ? ret : len;
-}
 
 static IIO_DEV_ATTR_TEMP_RAW(ade7753_read_8bit);
 static IIO_CONST_ATTR(in_temp_offset, "-25 C");
 static IIO_CONST_ATTR(in_temp_scale, "0.67 C");
 
-static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
-		ade7753_read_frequency,
-		ade7753_write_frequency);
-
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("27900 14000 7000 3500");
 
 static struct attribute *ade7753_attributes[] = {
 	&iio_dev_attr_in_temp_raw.dev_attr.attr,
 	&iio_const_attr_in_temp_offset.dev_attr.attr,
 	&iio_const_attr_in_temp_scale.dev_attr.attr,
-	&iio_dev_attr_sampling_frequency.dev_attr.attr,
 	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_phcal.dev_attr.attr,
 	&iio_dev_attr_cfden.dev_attr.attr,
@@ -496,6 +519,8 @@ static const struct attribute_group ade7753_attribute_group = {
 
 static const struct iio_info ade7753_info = {
 	.attrs = &ade7753_attribute_group,
+	.read_raw = &ade7753_read_raw,
+	.write_raw = &ade7753_write_raw,
 	.driver_module = THIS_MODULE,
 };
 
Signature · 1 line
1.8.3.1
maintainer
#1 lore raw
Quoted 3 lines · Sandhya Bankar

Moved functionality from IIO_DEV_ATTR_SAMP_FREQ attribute into IIO_CHAN_INFO_SAMP_FREQ handlers. Added ade7753_read_raw() and ade7753_write_raw() to allow reading/writing the element as well.

That says what you did, but why would you want to do this?

What benefit is this? Why do this churn at all?

Changelogs should say why you do something, not just what you did. Hopefully what you do should be obvious from the diff itself.

thanks,

greg k-h

#2 lore raw
Quoted 8 lines · gregkh
Quoted 3 lines · Sandhya Bankar

Moved functionality from IIO_DEV_ATTR_SAMP_FREQ attribute into IIO_CHAN_INFO_SAMP_FREQ handlers. Added ade7753_read_raw() and ade7753_write_raw() to allow reading/writing the element as well.

That says what you did, but why would you want to do this?

Quoted 9 lines

What benefit is this? Why do this churn at all?

Changelogs should say why you do something, not just what you did. Hopefully what you do should be obvious from the diff itself.

thanks,

greg k-h

Thanks ! got it, I will fix in next revision.

Thanks,
Sandhya