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

Quoted 5 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.

Signed-off-by Sandhya Bankar <[email protected]>

I'd forgotten this driver existed. There are a lot of similar elements in here that can be cleaned up if you want to take this further.

Step 1. Convert all the attrs that are actually standard IIO abi. Step 2. Start looking at the custom ABI in here and consider what to do about it. (might be a job for someone familiar with the hardware!)

Anyhow, this doesn't actually work.

Take a look at what calls the read_raw and write_raw functions and how the relevant sysfs attributes are instantiated.

Lars, I think any major ABI work on this is going to need some input from Analog (or someone who actually has one of these!)...

The energy meter drivers are somewhat of an open space when it comes to ABI definitions (there aren't really any yet!)

Jonathan

Quoted 199 lines

---
Changes in v2:
* Adding "Staging" keyword in subject line.

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

Quoted 20 lines · Jonathan Cameron

Quoted 5 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.

Signed-off-by Sandhya Bankar <[email protected]>

I'd forgotten this driver existed. There are a lot of similar elements in here that can be cleaned up if you want to take this further.

Step 1. Convert all the attrs that are actually standard IIO abi. Step 2. Start looking at the custom ABI in here and consider what to do about it. (might be a job for someone familiar with the hardware!)

Anyhow, this doesn't actually work.

Take a look at what calls the read_raw and write_raw functions and how the relevant sysfs attributes are instantiated.

Quoted 6 lines

Lars, I think any major ABI work on this is going to need some input from Analog (or someone who actually has one of these!)...

The energy meter drivers are somewhat of an open space when it comes to ABI definitions (there aren't really any yet!)

Hello,
It doesn't work because I have not defined the attribute
IIO_CHAN_INFO* for sysfs. So, the attribute won't get exposed as a sysfs file. Right ?
We typically use it as a channel attribute.
So, in this case, Should I create a channel, or is there another possible way to do it ?

I am just curious to know that why the channel for this driver is not defined ? Please suggest on same.

Thanks,
Sandhya

Quoted 192 lines

Jonathan

Quoted 191 lines

---
Changes in v2:
* Adding "Staging" keyword in subject line.

 drivers/staging/iio/meter/ade7753.c | 155 +++++++++++++++++++++---------------
 1 file changed, 90 insertions(+), 65 deletions(-)

drivers/staging/iio/meter/ade7753.c +88 -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 = {
 

Quoted 9 lines

drivers/staging/iio/meter/ade7753.c static const struct attribute_group ade7753_attribute_group = { · line 526

Quoted 8 lines

drivers/staging/iio/meter/ade7753.c +2 -0

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

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html