[PATCH 0/4] iio: Start conversion to sysfs_emit()

[PATCH 0/4] iio: Start conversion to sysfs_emit()

[Lars-Peter Clausen]

sysfs_emit() and sysfs_emit_at() are new helper functions for output data
for sysfs attributes. They are preferred over raw s*printf() for sysfs
attributes since it knows about the sysfs buffer specifics and has some
built-in sanity checks.

This patch series updates the IIO core to make use these new helper
functions instead of raw s*printf().

In addition all the powerdown callbacks of DAC drivers are also updated to
use sysfs_emit().

There remain many instances of s*printf() in individual drivers for now.
These can be converted in follow up patch series.

But in many cases rather than doing the conversion from s*printf() to
sysfs_emit() it will be better to convert those drivers to implement
read_raw() or read_avail() callbacks and let the IIO core do the
formatting.

Implementing read_raw() or read_avail() instead of directly implementing a
sysfs attribute has the advantage that the data can then also be accessed
through in-kernel APIs and is available to in-kernel consumers.

Lars-Peter Clausen (4):
  iio: core: Use sysfs_emit() (trivial bits)
  iio: iio_enum_available_read(): Convert to sysfs_emit_at()
  iio: __iio_format_value(): Convert to sysfs_emit_at()
  iio: dac: Convert powerdown read callbacks to sysfs_emit()

 drivers/iio/dac/ad5064.c           |  2 +-
 drivers/iio/dac/ad5360.c           |  2 +-
 drivers/iio/dac/ad5380.c           |  2 +-
 drivers/iio/dac/ad5446.c           |  2 +-
 drivers/iio/dac/ad5504.c           |  4 +-
 drivers/iio/dac/ad5624r_spi.c      |  4 +-
 drivers/iio/dac/ad5686.c           |  2 +-
 drivers/iio/dac/ad5755.c           |  4 +-
 drivers/iio/dac/ad5758.c           |  2 +-
 drivers/iio/dac/ad5770r.c          |  2 +-
 drivers/iio/dac/ad5791.c           |  2 +-
 drivers/iio/dac/ad7303.c           |  2 +-
 drivers/iio/dac/ltc2632.c          |  4 +-
 drivers/iio/dac/max5821.c          |  2 +-
 drivers/iio/dac/mcp4725.c          |  2 +-
 drivers/iio/dac/stm32-dac.c        |  2 +-
 drivers/iio/dac/ti-dac082s085.c    |  2 +-
 drivers/iio/dac/ti-dac5571.c       |  2 +-
 drivers/iio/dac/ti-dac7311.c       |  2 +-
 drivers/iio/industrialio-buffer.c  | 20 ++++-----
 drivers/iio/industrialio-core.c    | 70 +++++++++++++++---------------
 drivers/iio/industrialio-event.c   |  2 +-
 drivers/iio/industrialio-trigger.c |  4 +-
 23 files changed, 71 insertions(+), 71 deletions(-)

-- 
2.20.1

[PATCH 1/4] iio: core: Use sysfs_emit() (trivial bits)

[Lars-Peter Clausen]

sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
knows about the sysfs buffer specifics and has some built-in sanity checks.

This patch converts the places in the iio core that follow the pattern of

   return s*printf(...)

to

   return sysfs_emit(...)

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-buffer.c  | 20 ++++++++++----------
 drivers/iio/industrialio-core.c    | 16 ++++++++--------
 drivers/iio/industrialio-event.c   |  2 +-
 drivers/iio/industrialio-trigger.c |  4 ++--
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index ee5aab9d4a23..ccc8a8cae604 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -260,7 +260,7 @@ static ssize_t iio_show_scan_index(struct device *dev,
 				   struct device_attribute *attr,
 				   char *buf)
 {
-	return sprintf(buf, "%u\n", to_iio_dev_attr(attr)->c->scan_index);
+	return sysfs_emit(buf, "%u\n", to_iio_dev_attr(attr)->c->scan_index);
 }
 
 static ssize_t iio_show_fixed_type(struct device *dev,
@@ -278,15 +278,15 @@ static ssize_t iio_show_fixed_type(struct device *dev,
 #endif
 	}
 	if (this_attr->c->scan_type.repeat > 1)
-		return sprintf(buf, "%s:%c%d/%dX%d>>%u\n",
+		return sysfs_emit(buf, "%s:%c%d/%dX%d>>%u\n",
 		       iio_endian_prefix[type],
 		       this_attr->c->scan_type.sign,
 		       this_attr->c->scan_type.realbits,
 		       this_attr->c->scan_type.storagebits,
 		       this_attr->c->scan_type.repeat,
 		       this_attr->c->scan_type.shift);
-		else
-			return sprintf(buf, "%s:%c%d/%d>>%u\n",
+	else
+		return sysfs_emit(buf, "%s:%c%d/%d>>%u\n",
 		       iio_endian_prefix[type],
 		       this_attr->c->scan_type.sign,
 		       this_attr->c->scan_type.realbits,
@@ -305,7 +305,7 @@ static ssize_t iio_scan_el_show(struct device *dev,
 	ret = !!test_bit(to_iio_dev_attr(attr)->address,
 		       buffer->scan_mask);
 
-	return sprintf(buf, "%d\n", ret);
+	return sysfs_emit(buf, "%d\n", ret);
 }
 
 /* Note NULL used as error indicator as it doesn't make sense. */
@@ -449,7 +449,7 @@ static ssize_t iio_scan_el_ts_show(struct device *dev,
 {
 	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
 
-	return sprintf(buf, "%d\n", buffer->scan_timestamp);
+	return sysfs_emit(buf, "%d\n", buffer->scan_timestamp);
 }
 
 static ssize_t iio_scan_el_ts_store(struct device *dev,
@@ -541,7 +541,7 @@ static ssize_t iio_buffer_read_length(struct device *dev,
 {
 	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
 
-	return sprintf(buf, "%d\n", buffer->length);
+	return sysfs_emit(buf, "%d\n", buffer->length);
 }
 
 static ssize_t iio_buffer_write_length(struct device *dev,
@@ -583,7 +583,7 @@ static ssize_t iio_buffer_show_enable(struct device *dev,
 {
 	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
 
-	return sprintf(buf, "%d\n", iio_buffer_is_active(buffer));
+	return sysfs_emit(buf, "%d\n", iio_buffer_is_active(buffer));
 }
 
 static unsigned int iio_storage_bytes_for_si(struct iio_dev *indio_dev,
@@ -1227,7 +1227,7 @@ static ssize_t iio_buffer_show_watermark(struct device *dev,
 {
 	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
 
-	return sprintf(buf, "%u\n", buffer->watermark);
+	return sysfs_emit(buf, "%u\n", buffer->watermark);
 }
 
 static ssize_t iio_buffer_store_watermark(struct device *dev,
@@ -1271,7 +1271,7 @@ static ssize_t iio_dma_show_data_available(struct device *dev,
 {
 	struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer;
 
-	return sprintf(buf, "%zu\n", iio_buffer_data_available(buffer));
+	return sysfs_emit(buf, "%zu\n", iio_buffer_data_available(buffer));
 }
 
 static DEVICE_ATTR(length, S_IRUGO | S_IWUSR, iio_buffer_read_length,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index b5750edf935c..058874af1242 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -234,7 +234,7 @@ ssize_t iio_read_const_attr(struct device *dev,
 			    struct device_attribute *attr,
 			    char *buf)
 {
-	return sprintf(buf, "%s\n", to_iio_const_attr(attr)->string);
+	return sysfs_emit(buf, "%s\n", to_iio_const_attr(attr)->string);
 }
 EXPORT_SYMBOL(iio_read_const_attr);
 
@@ -529,7 +529,7 @@ ssize_t iio_enum_read(struct iio_dev *indio_dev,
 	else if (i >= e->num_items || !e->items[i])
 		return -EINVAL;
 
-	return snprintf(buf, PAGE_SIZE, "%s\n", e->items[i]);
+	return sysfs_emit(buf, "%s\n", e->items[i]);
 }
 EXPORT_SYMBOL_GPL(iio_enum_read);
 
@@ -580,10 +580,10 @@ ssize_t iio_show_mount_matrix(struct iio_dev *indio_dev, uintptr_t priv,
 	if (!mtx)
 		mtx = &iio_mount_idmatrix;
 
-	return snprintf(buf, PAGE_SIZE, "%s, %s, %s; %s, %s, %s; %s, %s, %s\n",
-			mtx->rotation[0], mtx->rotation[1], mtx->rotation[2],
-			mtx->rotation[3], mtx->rotation[4], mtx->rotation[5],
-			mtx->rotation[6], mtx->rotation[7], mtx->rotation[8]);
+	return sysfs_emit(buf, "%s, %s, %s; %s, %s, %s; %s, %s, %s\n",
+			  mtx->rotation[0], mtx->rotation[1], mtx->rotation[2],
+			  mtx->rotation[3], mtx->rotation[4], mtx->rotation[5],
+			  mtx->rotation[6], mtx->rotation[7], mtx->rotation[8]);
 }
 EXPORT_SYMBOL_GPL(iio_show_mount_matrix);
 
@@ -1369,7 +1369,7 @@ static ssize_t iio_show_dev_name(struct device *dev,
 				 char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	return snprintf(buf, PAGE_SIZE, "%s\n", indio_dev->name);
+	return sysfs_emit(buf, "%s\n", indio_dev->name);
 }
 
 static DEVICE_ATTR(name, S_IRUGO, iio_show_dev_name, NULL);
@@ -1379,7 +1379,7 @@ static ssize_t iio_show_dev_label(struct device *dev,
 				 char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	return snprintf(buf, PAGE_SIZE, "%s\n", indio_dev->label);
+	return sysfs_emit(buf, "%s\n", indio_dev->label);
 }
 
 static DEVICE_ATTR(label, S_IRUGO, iio_show_dev_label, NULL);
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index a30e289fc362..1b3a15bc75fe 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -297,7 +297,7 @@ static ssize_t iio_ev_state_show(struct device *dev,
 	if (val < 0)
 		return val;
 	else
-		return sprintf(buf, "%d\n", val);
+		return sysfs_emit(buf, "%d\n", val);
 }
 
 static ssize_t iio_ev_value_show(struct device *dev,
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 32ac1bec25e3..efeb5e2eca8a 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -50,7 +50,7 @@ static ssize_t iio_trigger_read_name(struct device *dev,
 				     char *buf)
 {
 	struct iio_trigger *trig = to_iio_trigger(dev);
-	return sprintf(buf, "%s\n", trig->name);
+	return sysfs_emit(buf, "%s\n", trig->name);
 }
 
 static DEVICE_ATTR(name, S_IRUGO, iio_trigger_read_name, NULL);
@@ -375,7 +375,7 @@ static ssize_t iio_trigger_read_current(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 
 	if (indio_dev->trig)
-		return sprintf(buf, "%s\n", indio_dev->trig->name);
+		return sysfs_emit(buf, "%s\n", indio_dev->trig->name);
 	return 0;
 }
 
-- 
2.20.1

[PATCH 2/4] iio: iio_enum_available_read(): Convert to sysfs_emit_at()

[Lars-Peter Clausen]

sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
knows about the sysfs buffer specifics and has some built-in sanity checks.

Convert the iio_enum_available_read() function to use sysfs_emit_at()
instead of scnprintf().

The conversion is straight forward, the only difference is that
sysfs_emit_at() takes the buffers start address and an offset as parameters
and already knows about the buffer's size limit.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 058874af1242..e0fdf9141e09 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -504,7 +504,7 @@ ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
 	for (i = 0; i < e->num_items; ++i) {
 		if (!e->items[i])
 			continue;
-		len += scnprintf(buf + len, PAGE_SIZE - len, "%s ", e->items[i]);
+		len += sysfs_emit_at(buf, len, "%s ", e->items[i]);
 	}
 
 	/* replace last space with a newline */
-- 
2.20.1

[PATCH 3/4] iio: __iio_format_value(): Convert to sysfs_emit_at()

[Lars-Peter Clausen]

sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
knows about the sysfs buffer specifics and has some built-in sanity checks.

Convert __iio_format_value() and related functions to use this new
interface.

This conversion involves changing the signature of __iio_format_value() so
that it similar to sysfs_emit_at() and takes the buffers start address and
an offset where to write within the buffer.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-core.c | 52 ++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e0fdf9141e09..d92c58a94fe4 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -623,7 +623,7 @@ int iio_read_mount_matrix(struct device *dev, const char *propname,
 }
 EXPORT_SYMBOL(iio_read_mount_matrix);
 
-static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
+static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
 				  int size, const int *vals)
 {
 	int tmp0, tmp1;
@@ -632,52 +632,53 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
 
 	switch (type) {
 	case IIO_VAL_INT:
-		return scnprintf(buf, len, "%d", vals[0]);
+		return sysfs_emit_at(buf, offset, "%d", vals[0]);
 	case IIO_VAL_INT_PLUS_MICRO_DB:
 		scale_db = true;
 		fallthrough;
 	case IIO_VAL_INT_PLUS_MICRO:
 		if (vals[1] < 0)
-			return scnprintf(buf, len, "-%d.%06u%s", abs(vals[0]),
-					-vals[1], scale_db ? " dB" : "");
+			return sysfs_emit_at(buf, offset, "-%d.%06u%s",
+					     abs(vals[0]), -vals[1],
+					     scale_db ? " dB" : "");
 		else
-			return scnprintf(buf, len, "%d.%06u%s", vals[0], vals[1],
-					scale_db ? " dB" : "");
+			return sysfs_emit_at(buf, offset, "%d.%06u%s", vals[0],
+					     vals[1], scale_db ? " dB" : "");
 	case IIO_VAL_INT_PLUS_NANO:
 		if (vals[1] < 0)
-			return scnprintf(buf, len, "-%d.%09u", abs(vals[0]),
-					-vals[1]);
+			return sysfs_emit_at(buf, offset, "-%d.%09u",
+					     abs(vals[0]), -vals[1]);
 		else
-			return scnprintf(buf, len, "%d.%09u", vals[0], vals[1]);
+			return sysfs_emit_at(buf, offset, "%d.%09u", vals[0],
+					     vals[1]);
 	case IIO_VAL_FRACTIONAL:
 		tmp2 = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
 		tmp1 = vals[1];
 		tmp0 = (int)div_s64_rem(tmp2, 1000000000, &tmp1);
 		if ((tmp2 < 0) && (tmp0 == 0))
-			return snprintf(buf, len, "-0.%09u", abs(tmp1));
+			return sysfs_emit_at(buf, offset, "-0.%09u", abs(tmp1));
 		else
-			return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
+			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
+					     abs(tmp1));
 	case IIO_VAL_FRACTIONAL_LOG2:
 		tmp2 = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
 		tmp0 = (int)div_s64_rem(tmp2, 1000000000LL, &tmp1);
 		if (tmp0 == 0 && tmp2 < 0)
-			return snprintf(buf, len, "-0.%09u", abs(tmp1));
+			return sysfs_emit_at(buf, offset, "-0.%09u", abs(tmp1));
 		else
-			return scnprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
+			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
+					     abs(tmp1));
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
 		int l = 0;
 
-		for (i = 0; i < size; ++i) {
-			l += scnprintf(&buf[l], len - l, "%d ", vals[i]);
-			if (l >= len)
-				break;
-		}
+		for (i = 0; i < size; ++i)
+			l += sysfs_emit_at(buf, offset + l, "%d ", vals[i]);
 		return l;
 	}
 	case IIO_VAL_CHAR:
-		return scnprintf(buf, len, "%c", (char)vals[0]);
+		return sysfs_emit_at(buf, offset, "%c", (char)vals[0]);
 	default:
 		return 0;
 	}
@@ -701,11 +702,11 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
 {
 	ssize_t len;
 
-	len = __iio_format_value(buf, PAGE_SIZE, type, size, vals);
+	len = __iio_format_value(buf, 0, type, size, vals);
 	if (len >= PAGE_SIZE - 1)
 		return -EFBIG;
 
-	return len + sprintf(buf + len, "\n");
+	return len + sysfs_emit_at(buf, len, "\n");
 }
 EXPORT_SYMBOL_GPL(iio_format_value);
 
@@ -763,22 +764,21 @@ static ssize_t iio_format_list(char *buf, const int *vals, int type, int length,
 		break;
 	}
 
-	len = scnprintf(buf, PAGE_SIZE, prefix);
+	len = sysfs_emit(buf, prefix);
 
 	for (i = 0; i <= length - stride; i += stride) {
 		if (i != 0) {
-			len += scnprintf(buf + len, PAGE_SIZE - len, " ");
+			len += sysfs_emit_at(buf, len, " ");
 			if (len >= PAGE_SIZE)
 				return -EFBIG;
 		}
 
-		len += __iio_format_value(buf + len, PAGE_SIZE - len, type,
-					  stride, &vals[i]);
+		len += __iio_format_value(buf, len, type, stride, &vals[i]);
 		if (len >= PAGE_SIZE)
 			return -EFBIG;
 	}
 
-	len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n", suffix);
+	len += sysfs_emit_at(buf, len, "%s\n", suffix);
 
 	return len;
 }
-- 
2.20.1

[PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Lars-Peter Clausen]

Update DAC drivers powerdown attribute show callback to use the new
sysfs_emit() function.

sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
knows about the sysfs buffer specifics and has some built-in sanity checks.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/dac/ad5064.c        | 2 +-
 drivers/iio/dac/ad5360.c        | 2 +-
 drivers/iio/dac/ad5380.c        | 2 +-
 drivers/iio/dac/ad5446.c        | 2 +-
 drivers/iio/dac/ad5504.c        | 4 ++--
 drivers/iio/dac/ad5624r_spi.c   | 4 ++--
 drivers/iio/dac/ad5686.c        | 2 +-
 drivers/iio/dac/ad5755.c        | 4 ++--
 drivers/iio/dac/ad5758.c        | 2 +-
 drivers/iio/dac/ad5770r.c       | 2 +-
 drivers/iio/dac/ad5791.c        | 2 +-
 drivers/iio/dac/ad7303.c        | 2 +-
 drivers/iio/dac/ltc2632.c       | 4 ++--
 drivers/iio/dac/max5821.c       | 2 +-
 drivers/iio/dac/mcp4725.c       | 2 +-
 drivers/iio/dac/stm32-dac.c     | 2 +-
 drivers/iio/dac/ti-dac082s085.c | 2 +-
 drivers/iio/dac/ti-dac5571.c    | 2 +-
 drivers/iio/dac/ti-dac7311.c    | 2 +-
 19 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index 82abd4d6886c..dff623b65e4f 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -277,7 +277,7 @@ static ssize_t ad5064_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5064_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->pwr_down[chan->channel]);
+	return sysfs_emit(buf, "%d\n", st->pwr_down[chan->channel]);
 }
 
 static ssize_t ad5064_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c
index 602dd2ba61b5..2d3b14c407d8 100644
--- a/drivers/iio/dac/ad5360.c
+++ b/drivers/iio/dac/ad5360.c
@@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad5360_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
+	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
 }
 
 static int ad5360_update_ctrl(struct iio_dev *indio_dev, unsigned int set,
diff --git a/drivers/iio/dac/ad5380.c b/drivers/iio/dac/ad5380.c
index 37ef653564b0..53db5b4e4c53 100644
--- a/drivers/iio/dac/ad5380.c
+++ b/drivers/iio/dac/ad5380.c
@@ -85,7 +85,7 @@ static ssize_t ad5380_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5380_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->pwr_down);
+	return sysfs_emit(buf, "%d\n", st->pwr_down);
 }
 
 static ssize_t ad5380_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index d87e21016863..488ec69967d6 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -100,7 +100,7 @@ static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->pwr_down);
+	return sysfs_emit(buf, "%d\n", st->pwr_down);
 }
 
 static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index e9297c25d4ef..c12ae91d8843 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -170,8 +170,8 @@ static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n",
-			!(st->pwr_down_mask & (1 << chan->channel)));
+	return sysfs_emit(buf, "%d\n",
+			  !(st->pwr_down_mask & (1 << chan->channel)));
 }
 
 static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5624r_spi.c b/drivers/iio/dac/ad5624r_spi.c
index 2b2b8edfd258..9bde86982912 100644
--- a/drivers/iio/dac/ad5624r_spi.c
+++ b/drivers/iio/dac/ad5624r_spi.c
@@ -117,8 +117,8 @@ static ssize_t ad5624r_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5624r_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n",
-			!!(st->pwr_down_mask & (1 << chan->channel)));
+	return sysfs_emit(buf, "%d\n",
+			  !!(st->pwr_down_mask & (1 << chan->channel)));
 }
 
 static ssize_t ad5624r_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 99a95282ac57..fcb64f20ff64 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -57,7 +57,7 @@ static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5686_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", !!(st->pwr_down_mask &
+	return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask &
 				       (0x3 << (chan->channel * 2))));
 }
 
diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index 0df28acf074a..cabc38d54085 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -399,8 +399,8 @@ static ssize_t ad5755_read_powerdown(struct iio_dev *indio_dev, uintptr_t priv,
 {
 	struct ad5755_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n",
-		       (bool)(st->pwr_down & (1 << chan->channel)));
+	return sysfs_emit(buf, "%d\n",
+			  (bool)(st->pwr_down & (1 << chan->channel)));
 }
 
 static ssize_t ad5755_write_powerdown(struct iio_dev *indio_dev, uintptr_t priv,
diff --git a/drivers/iio/dac/ad5758.c b/drivers/iio/dac/ad5758.c
index bd9ac8359d98..0572ef518101 100644
--- a/drivers/iio/dac/ad5758.c
+++ b/drivers/iio/dac/ad5758.c
@@ -574,7 +574,7 @@ static ssize_t ad5758_read_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5758_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->pwr_down);
+	return sysfs_emit(buf, "%d\n", st->pwr_down);
 }
 
 static ssize_t ad5758_write_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5770r.c b/drivers/iio/dac/ad5770r.c
index 84dcf149261f..5e901fe48023 100644
--- a/drivers/iio/dac/ad5770r.c
+++ b/drivers/iio/dac/ad5770r.c
@@ -433,7 +433,7 @@ static ssize_t ad5770r_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5770r_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->ch_pwr_down[chan->channel]);
+	return sysfs_emit(buf, "%d\n", st->ch_pwr_down[chan->channel]);
 }
 
 static ssize_t ad5770r_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index 615d72cd59bc..a0923b76e8b6 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -177,7 +177,7 @@ static ssize_t ad5791_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5791_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->pwr_down);
+	return sysfs_emit(buf, "%d\n", st->pwr_down);
 }
 
 static ssize_t ad5791_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ad7303.c b/drivers/iio/dac/ad7303.c
index dbb4645ab6b1..e1b6a92df12f 100644
--- a/drivers/iio/dac/ad7303.c
+++ b/drivers/iio/dac/ad7303.c
@@ -65,7 +65,7 @@ static ssize_t ad7303_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad7303_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", (bool)(st->config &
+	return sysfs_emit(buf, "%d\n", (bool)(st->config &
 		AD7303_CFG_POWER_DOWN(chan->channel)));
 }
 
diff --git a/drivers/iio/dac/ltc2632.c b/drivers/iio/dac/ltc2632.c
index 4002ed0868be..53e4b887d372 100644
--- a/drivers/iio/dac/ltc2632.c
+++ b/drivers/iio/dac/ltc2632.c
@@ -135,8 +135,8 @@ static ssize_t ltc2632_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ltc2632_state *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n",
-		       !!(st->powerdown_cache_mask & (1 << chan->channel)));
+	return sysfs_emit(buf, "%d\n",
+			  !!(st->powerdown_cache_mask & (1 << chan->channel)));
 }
 
 static ssize_t ltc2632_write_dac_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d6bb24db49c4..bd6e75699a63 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -84,7 +84,7 @@ static ssize_t max5821_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct max5821_data *st = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", st->powerdown[chan->channel]);
+	return sysfs_emit(buf, "%d\n", st->powerdown[chan->channel]);
 }
 
 static int max5821_sync_powerdown_mode(struct max5821_data *data,
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index beb9a15b7c74..34b14aafb630 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -167,7 +167,7 @@ static ssize_t mcp4725_read_powerdown(struct iio_dev *indio_dev,
 {
 	struct mcp4725_data *data = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", data->powerdown);
+	return sysfs_emit(buf, "%d\n", data->powerdown);
 }
 
 static ssize_t mcp4725_write_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
index 12dec68c16f7..a5b0a52bf86e 100644
--- a/drivers/iio/dac/stm32-dac.c
+++ b/drivers/iio/dac/stm32-dac.c
@@ -210,7 +210,7 @@ static ssize_t stm32_dac_read_powerdown(struct iio_dev *indio_dev,
 	if (ret < 0)
 		return ret;
 
-	return sprintf(buf, "%d\n", ret ? 0 : 1);
+	return sysfs_emit(buf, "%d\n", ret ? 0 : 1);
 }
 
 static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ti-dac082s085.c b/drivers/iio/dac/ti-dac082s085.c
index de33c1fc6e0b..5c14bfb16521 100644
--- a/drivers/iio/dac/ti-dac082s085.c
+++ b/drivers/iio/dac/ti-dac082s085.c
@@ -121,7 +121,7 @@ static ssize_t ti_dac_read_powerdown(struct iio_dev *indio_dev,
 {
 	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", ti_dac->powerdown);
+	return sysfs_emit(buf, "%d\n", ti_dac->powerdown);
 }
 
 static ssize_t ti_dac_write_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index d3295767a079..2a5ba1b08a1d 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -166,7 +166,7 @@ static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
 {
 	struct dac5571_data *data = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", data->powerdown[chan->channel]);
+	return sysfs_emit(buf, "%d\n", data->powerdown[chan->channel]);
 }
 
 static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
diff --git a/drivers/iio/dac/ti-dac7311.c b/drivers/iio/dac/ti-dac7311.c
index 63171e42f987..9d0b253be841 100644
--- a/drivers/iio/dac/ti-dac7311.c
+++ b/drivers/iio/dac/ti-dac7311.c
@@ -110,7 +110,7 @@ static ssize_t ti_dac_read_powerdown(struct iio_dev *indio_dev,
 {
 	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", ti_dac->powerdown);
+	return sysfs_emit(buf, "%d\n", ti_dac->powerdown);
 }
 
 static ssize_t ti_dac_write_powerdown(struct iio_dev *indio_dev,
-- 
2.20.1

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Joe Perches]

On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:
> Update DAC drivers powerdown attribute show callback to use the new
> sysfs_emit() function.
> 
> sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
> knows about the sysfs buffer specifics and has some built-in sanity checks.

Thanks.

unrelated trivia:

> diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c
[]
> @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad5360_state *st = iio_priv(indio_dev);
>  
> -	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> +	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));

rather than cast to bool, perhaps standardize to use !!(val & test)

> diff --git a/drivers/iio/dac/ad5624r_spi.c b/drivers/iio/dac/ad5624r_spi.c
[]
> @@ -117,8 +117,8 @@ static ssize_t ad5624r_read_dac_powerdown(struct iio_dev *indio_dev,
>  {
>  	struct ad5624r_state *st = iio_priv(indio_dev);
>  
> -	return sprintf(buf, "%d\n",
> -			!!(st->pwr_down_mask & (1 << chan->channel)));
> +	return sysfs_emit(buf, "%d\n",
> +			  !!(st->pwr_down_mask & (1 << chan->channel)));

like this and below...

> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
[]
> @@ -57,7 +57,7 @@ static ssize_t ad5686_read_dac_powerdown(struct iio_dev *indio_dev,
>  {
>  	struct ad5686_state *st = iio_priv(indio_dev);
> 
> -	return sprintf(buf, "%d\n", !!(st->pwr_down_mask &
> +	return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask &
>  				       (0x3 << (chan->channel * 2))));
>  }

etc...

and it might be nicer to rewrap alignments like

	return sysfs_emit(buf, "%d\n",
			  !!(st->pwr_down_mask & (0x3 << (chan->channel * 2))));

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Lars-Peter Clausen]

On 3/20/21 12:01 PM, Joe Perches wrote:
> On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:
>> Update DAC drivers powerdown attribute show callback to use the new
>> sysfs_emit() function.
>>
>> sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
>> knows about the sysfs buffer specifics and has some built-in sanity checks.
> Thanks.
>
> unrelated trivia:
>
>> diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c
> []
>> @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
>>   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>   	struct ad5360_state *st = iio_priv(indio_dev);
>>   
>> -	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
>> +	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> rather than cast to bool, perhaps standardize to use !!(val & test)
I very much prefer the cast to bool since it semantically stronger. You 
don't have to know that the !! idiom is used to cast an int to bool.

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Joe Perches]

On Sat, 2021-03-20 at 13:52 +0100, Lars-Peter Clausen wrote:
> On 3/20/21 12:01 PM, Joe Perches wrote:
> > On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:
> > > Update DAC drivers powerdown attribute show callback to use the new
> > > sysfs_emit() function.
> > > 
> > > sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
> > > knows about the sysfs buffer specifics and has some built-in sanity checks.
> > Thanks.
> > 
> > unrelated trivia:
> > 
> > > diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c
> > []
> > > @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
> > >   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > >   	struct ad5360_state *st = iio_priv(indio_dev);
> > >   
> > > 
> > > -	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > > +	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > rather than cast to bool, perhaps standardize to use !!(val & test)
> I very much prefer the cast to bool since it semantically stronger. You 
> don't have to know that the !! idiom is used to cast an int to bool.

Using !! does not cast to bool, it's an int.

casting to bool and using %d in a printf equivalent ends up with an
integer promotion/implicit type conversion from bool to int.

Anyway, it's not my code so it's author's choice, but similar
code using different styles is, at a minimum, inconsistent.

Re: [PATCH 0/4] iio: Start conversion to sysfs_emit()

[Jonathan Cameron]

On Sat, 20 Mar 2021 08:14:01 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> sysfs_emit() and sysfs_emit_at() are new helper functions for output data
> for sysfs attributes. They are preferred over raw s*printf() for sysfs
> attributes since it knows about the sysfs buffer specifics and has some
> built-in sanity checks.
> 
> This patch series updates the IIO core to make use these new helper
> functions instead of raw s*printf().
> 
> In addition all the powerdown callbacks of DAC drivers are also updated to
> use sysfs_emit().
> 
> There remain many instances of s*printf() in individual drivers for now.
> These can be converted in follow up patch series.
> 
> But in many cases rather than doing the conversion from s*printf() to
> sysfs_emit() it will be better to convert those drivers to implement
> read_raw() or read_avail() callbacks and let the IIO core do the
> formatting.
> 
> Implementing read_raw() or read_avail() instead of directly implementing a
> sysfs attribute has the advantage that the data can then also be accessed
> through in-kernel APIs and is available to in-kernel consumers.

Looks good to me. I'll let this sit on the list for a little while
though as obviously touch a lot of drivers + core code so others
may well want to check we haven't missed anything!

Jonathan

> 
> Lars-Peter Clausen (4):
>   iio: core: Use sysfs_emit() (trivial bits)
>   iio: iio_enum_available_read(): Convert to sysfs_emit_at()
>   iio: __iio_format_value(): Convert to sysfs_emit_at()
>   iio: dac: Convert powerdown read callbacks to sysfs_emit()
> 
>  drivers/iio/dac/ad5064.c           |  2 +-
>  drivers/iio/dac/ad5360.c           |  2 +-
>  drivers/iio/dac/ad5380.c           |  2 +-
>  drivers/iio/dac/ad5446.c           |  2 +-
>  drivers/iio/dac/ad5504.c           |  4 +-
>  drivers/iio/dac/ad5624r_spi.c      |  4 +-
>  drivers/iio/dac/ad5686.c           |  2 +-
>  drivers/iio/dac/ad5755.c           |  4 +-
>  drivers/iio/dac/ad5758.c           |  2 +-
>  drivers/iio/dac/ad5770r.c          |  2 +-
>  drivers/iio/dac/ad5791.c           |  2 +-
>  drivers/iio/dac/ad7303.c           |  2 +-
>  drivers/iio/dac/ltc2632.c          |  4 +-
>  drivers/iio/dac/max5821.c          |  2 +-
>  drivers/iio/dac/mcp4725.c          |  2 +-
>  drivers/iio/dac/stm32-dac.c        |  2 +-
>  drivers/iio/dac/ti-dac082s085.c    |  2 +-
>  drivers/iio/dac/ti-dac5571.c       |  2 +-
>  drivers/iio/dac/ti-dac7311.c       |  2 +-
>  drivers/iio/industrialio-buffer.c  | 20 ++++-----
>  drivers/iio/industrialio-core.c    | 70 +++++++++++++++---------------
>  drivers/iio/industrialio-event.c   |  2 +-
>  drivers/iio/industrialio-trigger.c |  4 +-
>  23 files changed, 71 insertions(+), 71 deletions(-)
> 

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Jonathan Cameron]

On Sat, 20 Mar 2021 08:13:55 -0700
Joe Perches <joe@perches.com> wrote:

> On Sat, 2021-03-20 at 13:52 +0100, Lars-Peter Clausen wrote:
> > On 3/20/21 12:01 PM, Joe Perches wrote:  
> > > On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:  
> > > > Update DAC drivers powerdown attribute show callback to use the new
> > > > sysfs_emit() function.
> > > > 
> > > > sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
> > > > knows about the sysfs buffer specifics and has some built-in sanity checks.  
> > > Thanks.
> > > 
> > > unrelated trivia:
> > >   
> > > > diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c  
> > > []  
> > > > @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
> > > >   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > >   	struct ad5360_state *st = iio_priv(indio_dev);
> > > >   
> > > > 
> > > > -	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > > > +	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));  
> > > rather than cast to bool, perhaps standardize to use !!(val & test)  
> > I very much prefer the cast to bool since it semantically stronger. You 
> > don't have to know that the !! idiom is used to cast an int to bool.  
> 
> Using !! does not cast to bool, it's an int.
> 
> casting to bool and using %d in a printf equivalent ends up with an
> integer promotion/implicit type conversion from bool to int.
> 
> Anyway, it's not my code so it's author's choice, but similar
> code using different styles is, at a minimum, inconsistent.
> 
I'm certainly not against cleaning this up at somepoint, but it's not strictly
part of what this particular patch set is doing, so I'd rather do
it separately anyway.

Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.

Thanks,

Jonathan

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Andy Shevchenko]

On Sat, Mar 20, 2021 at 2:54 PM Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 3/20/21 12:01 PM, Joe Perches wrote:
> > On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:
> >> Update DAC drivers powerdown attribute show callback to use the new
> >> sysfs_emit() function.
> >>
> >> sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
> >> knows about the sysfs buffer specifics and has some built-in sanity checks.
> > Thanks.
> >
> > unrelated trivia:
> >
> >> diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c
> > []
> >> @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
> >>      struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >>      struct ad5360_state *st = iio_priv(indio_dev);
> >>
> >> -    return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> >> +    return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > rather than cast to bool, perhaps standardize to use !!(val & test)
> I very much prefer the cast to bool since it semantically stronger.

It's a mistake here. You have no special type for bool and you do
transition int -> bool -> int.
Why? !! is a proper way to deal with this.

> You
> don't have to know that the !! idiom is used to cast an int to bool.

As Joe said.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Andy Shevchenko]

On Mon, Mar 29, 2021 at 2:21 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Mar 20, 2021 at 2:54 PM Lars-Peter Clausen <lars@metafoo.de> wrote:
> > On 3/20/21 12:01 PM, Joe Perches wrote:
> > > On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:

...

> > >> -    return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > >> +    return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > > rather than cast to bool, perhaps standardize to use !!(val & test)
> > I very much prefer the cast to bool since it semantically stronger.
>
> It's a mistake here. You have no special type for bool and you do
> transition int -> bool -> int.
> Why? !! is a proper way to deal with this.

Just to generalize: casting printf() parameters is a mistake in 99% cases.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Andy Shevchenko]

On Sat, Mar 20, 2021 at 1:26 PM Lars-Peter Clausen <lars@metafoo.de> wrote:

One side note for the future improvements.

> +       return sysfs_emit(buf, "%d\n",
> +                         !(st->pwr_down_mask & (1 << chan->channel)));

> +       return sysfs_emit(buf, "%d\n",
> +                         !!(st->pwr_down_mask & (1 << chan->channel)));

> -       return sprintf(buf, "%d\n", !!(st->pwr_down_mask &
> +       return sysfs_emit(buf, "%d\n", !!(st->pwr_down_mask &
>                                        (0x3 << (chan->channel * 2))));

Converting above to use BIT() / GENMASK() will help to avoid potential
UB when it will try to set the last (31st) bit.

> +       return sysfs_emit(buf, "%d\n",
> +                         (bool)(st->pwr_down & (1 << chan->channel)));

> +       return sysfs_emit(buf, "%d\n",
> +                         !!(st->powerdown_cache_mask & (1 << chan->channel)));

...

> -       return sprintf(buf, "%d\n", ret ? 0 : 1);
> +       return sysfs_emit(buf, "%d\n", ret ? 0 : 1);

Useless ternary, may be ret == 0 or !ret. (Yes, I know that compiler
optimizes this away)

-- 
With Best Regards,
Andy Shevchenko