[PATCH 0/9] iio: adc: ad9467: add debugFS test mode support

[PATCH 0/9] iio: adc: ad9467: add debugFS test mode support

[Nuno Sa]

This series adds support for testing the digital interface on ad9467
using different test patterns/modes. For doing it, debugFS support for
backends is also added. Notably:

* backend direct reg access which is not directly needed for the ad9467
  new interface but it's something useful to have (the same as IIO
  direct_reg_access for "normal" IIO devices) and something that I would
  eventually try to add anyways.
* printing channel status which is useful for more verbose information
  about some possible failure in the test pattern. 

The first couple of patches are unrelated but simple enough that
I placed them in this patchset.

---
Nuno Sa (9):
      iio: backend: remove unused parameter
      iio: backend: print message in case op is not implemented
      iio: backend: add debugFs interface
      iio: backend: add a modified prbs23 support
      iio: adc: adi-axi-adc: support modified prbs23
      iio: adc: adi-axi-adc: split axi_adc_chan_status()
      iio: adc: adi-axi-adc: implement backend debugfs interface
      iio: adc: ad9467: add backend test mode helpers
      iio: adc: ad9467: add digital interface test to debugfs

 drivers/iio/adc/ad9467.c           | 255 +++++++++++++++++++++++++++++++++----
 drivers/iio/adc/adi-axi-adc.c      |  64 +++++++++-
 drivers/iio/dac/ad9739a.c          |   3 +-
 drivers/iio/industrialio-backend.c | 123 +++++++++++++++++-
 include/linux/iio/backend.h        |  21 ++-
 5 files changed, 428 insertions(+), 38 deletions(-)
---
base-commit: 986da024b99a72e64f6bdb3f3f0e52af024b1f50
change-id: 20240709-dev-iio-backend-add-debugfs-ef7cd2007073
--

Thanks!
- Nuno Sá

[PATCH 1/9] iio: backend: remove unused parameter

[Nuno Sa]

Indio_dev was not being used in iio_backend_extend_chan_spec() so remove
it.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/dac/ad9739a.c          | 3 +--
 drivers/iio/industrialio-backend.c | 4 +---
 include/linux/iio/backend.h        | 3 +--
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/dac/ad9739a.c b/drivers/iio/dac/ad9739a.c
index f56eabe53723..9b1570e788b6 100644
--- a/drivers/iio/dac/ad9739a.c
+++ b/drivers/iio/dac/ad9739a.c
@@ -413,8 +413,7 @@ static int ad9739a_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = iio_backend_extend_chan_spec(indio_dev, st->back,
-					   &ad9739a_channels[0]);
+	ret = iio_backend_extend_chan_spec(st->back, &ad9739a_channels[0]);
 	if (ret)
 		return ret;
 
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index efe05be284b6..65a42944d090 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -451,7 +451,6 @@ EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, IIO_BACKEND);
 
 /**
  * iio_backend_extend_chan_spec - Extend an IIO channel
- * @indio_dev: IIO device
  * @back: Backend device
  * @chan: IIO channel
  *
@@ -461,8 +460,7 @@ EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, IIO_BACKEND);
  * RETURNS:
  * 0 on success, negative error number on failure.
  */
-int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
-				 struct iio_backend *back,
+int iio_backend_extend_chan_spec(struct iio_backend *back,
 				 struct iio_chan_spec *chan)
 {
 	const struct iio_chan_spec_ext_info *frontend_ext_info = chan->ext_info;
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 8099759d7242..4e81931703ab 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -142,8 +142,7 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 				 const struct iio_chan_spec *chan, char *buf);
 
-int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
-				 struct iio_backend *back,
+int iio_backend_extend_chan_spec(struct iio_backend *back,
 				 struct iio_chan_spec *chan);
 void *iio_backend_get_priv(const struct iio_backend *conv);
 struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name);

-- 
2.45.2

[PATCH 2/9] iio: backend: print message in case op is not implemented

[Nuno Sa]

For APIs that have a return value, -EOPNOTSUPP is returned in case the
backend does not support the functionality. However, for APIs that do
not have a return value we are left in silence. Hence, at least print a
debug message in case the callback is not implemented by the backend.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index 65a42944d090..f9da635cdfea 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -40,6 +40,7 @@
 #include <linux/mutex.h>
 #include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/stringify.h>
 #include <linux/types.h>
 
 #include <linux/iio/backend.h>
@@ -111,6 +112,9 @@ static DEFINE_MUTEX(iio_back_lock);
 	__ret = iio_backend_check_op(__back, op);		\
 	if (!__ret)						\
 		__back->ops->op(__back, ##args);		\
+	else							\
+		dev_dbg(__back->dev, "Op(%s) not implemented\n",\
+			__stringify(op));			\
 }
 
 /**

-- 
2.45.2

[PATCH 3/9] iio: backend: add debugFs interface

[Nuno Sa]

This adds a basic debugfs interface for backends. Two new ops are being
added:

 * debugfs_reg_access: Analogous to the core IIO one but for backend
   devices.
 * debugfs_print_chan_status: One useful usecase for this one is for
   testing test tones in a digital interface and "ask" the backend to
   dump more details on why a test tone might have errors.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/industrialio-backend.c | 115 +++++++++++++++++++++++++++++++++++++
 include/linux/iio/backend.h        |  16 +++++-
 2 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index f9da635cdfea..52cbde0d5885 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -32,6 +32,7 @@
 #define dev_fmt(fmt) "iio-backend: " fmt
 
 #include <linux/cleanup.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/errno.h>
@@ -46,6 +47,8 @@
 #include <linux/iio/backend.h>
 #include <linux/iio/iio.h>
 
+#define IIO_BACKEND_DEFAULT_NAME	"backend"
+
 struct iio_backend {
 	struct list_head entry;
 	const struct iio_backend_ops *ops;
@@ -53,6 +56,8 @@ struct iio_backend {
 	struct device *dev;
 	struct module *owner;
 	void *priv;
+	const char *name;
+	unsigned int cached_reg_addr;
 };
 
 /*
@@ -117,6 +122,111 @@ static DEFINE_MUTEX(iio_back_lock);
 			__stringify(op));			\
 }
 
+static ssize_t iio_backend_debugfs_read_reg(struct file *file,
+					    char __user *userbuf,
+					    size_t count, loff_t *ppos)
+{
+	struct iio_backend *back = file->private_data;
+	char read_buf[20];
+	unsigned int val;
+	int ret, len;
+
+	ret = iio_backend_op_call(back, debugfs_reg_access,
+				  back->cached_reg_addr, 0, &val);
+	if (ret)
+		return ret;
+
+	len = scnprintf(read_buf, sizeof(read_buf), "0x%X\n", val);
+
+	return simple_read_from_buffer(userbuf, count, ppos, read_buf, len);
+}
+
+static ssize_t iio_backend_debugfs_write_reg(struct file *file,
+					     const char __user *userbuf,
+					     size_t count, loff_t *ppos)
+{
+	struct iio_backend *back = file->private_data;
+	unsigned int val;
+	char buf[80];
+	ssize_t rc;
+	int ret;
+
+	rc = simple_write_to_buffer(buf, sizeof(buf), ppos, userbuf, count);
+	if (rc < 0)
+		return rc;
+
+	ret = sscanf(buf, "%i %i", &back->cached_reg_addr, &val);
+
+	switch (ret) {
+	case 1:
+		return count;
+	case 2:
+		ret = iio_backend_op_call(back, debugfs_reg_access,
+					  back->cached_reg_addr, val, NULL);
+		if (ret)
+			return ret;
+		return count;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct file_operations iio_backend_debugfs_reg_fops = {
+	.open = simple_open,
+	.read = iio_backend_debugfs_read_reg,
+	.write = iio_backend_debugfs_write_reg,
+};
+
+/**
+ * iio_backend_debugfs_add - Add debugfs interfaces for Backends
+ * @back: Backend device
+ * @indio_dev: IIO device
+ */
+void iio_backend_debugfs_add(struct iio_backend *back,
+			     struct iio_dev *indio_dev)
+{
+	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
+	char attr_name[128];
+
+	if (!IS_ENABLED(CONFIG_DEBUG_FS))
+		return;
+	if (!back->ops->debugfs_reg_access || !d)
+		return;
+
+	snprintf(attr_name, sizeof(attr_name), "%s_direct_reg_access",
+		 back->name);
+
+	debugfs_create_file(attr_name, 0644, d, back,
+			    &iio_backend_debugfs_reg_fops);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_debugfs_add, IIO_BACKEND);
+
+/**
+ * iio_backend_debugfs_print_chan_status - Print channel status
+ * @back: Backend device
+ * @chan: Channel number
+ * @buf: Buffer where to print the status
+ * @len: Available space
+ *
+ * One usecase where this is useful is for testing test tones in a digital
+ * interface and "ask" the backend to dump more details on why a test tone might
+ * have errors.
+ *
+ * RETURNS:
+ * Number of copied bytes on success, negative error code on failure.
+ */
+ssize_t iio_backend_debugfs_print_chan_status(struct iio_backend *back,
+					      unsigned int chan, char *buf,
+					      size_t len)
+{
+	if (!IS_ENABLED(CONFIG_DEBUG_FS))
+		return -ENODEV;
+
+	return iio_backend_op_call(back, debugfs_print_chan_status, chan, buf,
+				   len);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_debugfs_print_chan_status, IIO_BACKEND);
+
 /**
  * iio_backend_chan_enable - Enable a backend channel
  * @back: Backend device
@@ -577,6 +687,11 @@ struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
 		if (ret)
 			return ERR_PTR(ret);
 
+		if (name)
+			back->name = name;
+		else
+			back->name = IIO_BACKEND_DEFAULT_NAME;
+
 		return back;
 	}
 
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 4e81931703ab..a643d86c7487 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -9,6 +9,7 @@ struct fwnode_handle;
 struct iio_backend;
 struct device;
 struct iio_dev;
+struct dentry *d;
 
 enum iio_backend_data_type {
 	IIO_BACKEND_TWOS_COMPLEMENT,
@@ -22,6 +23,8 @@ enum iio_backend_data_source {
 	IIO_BACKEND_DATA_SOURCE_MAX
 };
 
+#define iio_backend_debugfs_ptr(ptr)	PTR_IF(IS_ENABLED(CONFIG_DEBUG_FS), ptr)
+
 /**
  * IIO_BACKEND_EX_INFO - Helper for an IIO extended channel attribute
  * @_name: Attribute name
@@ -81,6 +84,8 @@ enum iio_backend_sample_trigger {
  * @extend_chan_spec: Extend an IIO channel.
  * @ext_info_set: Extended info setter.
  * @ext_info_get: Extended info getter.
+ * @debugfs_print_chan_status: Print channel status into a buffer.
+ * @debugfs_reg_access: Read or write register value of backend.
  **/
 struct iio_backend_ops {
 	int (*enable)(struct iio_backend *back);
@@ -113,6 +118,11 @@ struct iio_backend_ops {
 			    const char *buf, size_t len);
 	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
 			    const struct iio_chan_spec *chan, char *buf);
+	int (*debugfs_print_chan_status)(struct iio_backend *back,
+					 unsigned int chan, char *buf,
+					 size_t len);
+	int (*debugfs_reg_access)(struct iio_backend *back, unsigned int reg,
+				  unsigned int writeval, unsigned int *readval);
 };
 
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
@@ -152,5 +162,9 @@ __devm_iio_backend_get_from_fwnode_lookup(struct device *dev,
 
 int devm_iio_backend_register(struct device *dev,
 			      const struct iio_backend_ops *ops, void *priv);
-
+ssize_t iio_backend_debugfs_print_chan_status(struct iio_backend *back,
+					      unsigned int chan, char *buf,
+					      size_t len);
+void iio_backend_debugfs_add(struct iio_backend *back,
+			     struct iio_dev *indio_dev);
 #endif

-- 
2.45.2

[PATCH 4/9] iio: backend: add a modified prbs23 support

[Nuno Sa]

Support ADI specific prb23 sequence that can be used both for
calibrating or debugging digital interfaces.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 include/linux/iio/backend.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index a643d86c7487..7cdcab6ded66 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -57,6 +57,8 @@ enum iio_backend_test_pattern {
 	IIO_BACKEND_NO_TEST_PATTERN,
 	/* modified prbs9 */
 	IIO_BACKEND_ADI_PRBS_9A = 32,
+	/* modified prbs23 */
+	IIO_BACKEND_ADI_PRBS_23A,
 	IIO_BACKEND_TEST_PATTERN_MAX
 };
 

-- 
2.45.2

[PATCH 5/9] iio: adc: adi-axi-adc: support modified prbs23

[Nuno Sa]

Add support for configuring the prbs23 sequence.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 21ce7564e83d..54499653d888 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -199,6 +199,10 @@ static int axi_adc_test_pattern_set(struct iio_backend *back,
 		return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CHAN_CTRL_3(chan),
 					  ADI_AXI_ADC_CHAN_PN_SEL_MASK,
 					  FIELD_PREP(ADI_AXI_ADC_CHAN_PN_SEL_MASK, 0));
+	case IIO_BACKEND_ADI_PRBS_23A:
+		return regmap_update_bits(st->regmap, ADI_AXI_ADC_REG_CHAN_CTRL_3(chan),
+					  ADI_AXI_ADC_CHAN_PN_SEL_MASK,
+					  FIELD_PREP(ADI_AXI_ADC_CHAN_PN_SEL_MASK, 1));
 	default:
 		return -EINVAL;
 	}

-- 
2.45.2

[PATCH 6/9] iio: adc: adi-axi-adc: split axi_adc_chan_status()

[Nuno Sa]

Add a new axi_adc_read_chan_status() helper so we get the raw register
value out of it.

This is in preparation of a future change where we really want to look
into dedicated bits of the register.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 54499653d888..12e0e6350a38 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -208,12 +208,10 @@ static int axi_adc_test_pattern_set(struct iio_backend *back,
 	}
 }
 
-static int axi_adc_chan_status(struct iio_backend *back, unsigned int chan,
-			       bool *error)
+static int axi_adc_read_chan_status(struct adi_axi_adc_state *st, unsigned int chan,
+				    unsigned int *status)
 {
-	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
 	int ret;
-	u32 val;
 
 	guard(mutex)(&st->lock);
 	/* reset test bits by setting them */
@@ -225,7 +223,18 @@ static int axi_adc_chan_status(struct iio_backend *back, unsigned int chan,
 	/* let's give enough time to validate or erroring the incoming pattern */
 	fsleep(1000);
 
-	ret = regmap_read(st->regmap, ADI_AXI_ADC_REG_CHAN_STATUS(chan), &val);
+	return regmap_read(st->regmap, ADI_AXI_ADC_REG_CHAN_STATUS(chan),
+			   status);
+}
+
+static int axi_adc_chan_status(struct iio_backend *back, unsigned int chan,
+			       bool *error)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	u32 val;
+	int ret;
+
+	ret = axi_adc_read_chan_status(st, chan, &val);
 	if (ret)
 		return ret;
 

-- 
2.45.2

[PATCH 7/9] iio: adc: adi-axi-adc: implement backend debugfs interface

[Nuno Sa]

Implement debugfs options to read/write registers and print the channel
status into a buffer (so we may know better the cause for errors) .

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 12e0e6350a38..7cba347cf8eb 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -61,6 +61,10 @@
 
 #define ADI_AXI_ADC_REG_CHAN_STATUS(c)		(0x0404 + (c) * 0x40)
 #define   ADI_AXI_ADC_CHAN_STAT_PN_MASK		GENMASK(2, 1)
+/* out of sync */
+#define   ADI_AXI_ADC_CHAN_STAT_PN_OOS		BIT(1)
+/* spurious out of sync */
+#define   ADI_AXI_ADC_CHAN_STAT_PN_ERR		BIT(2)
 
 #define ADI_AXI_ADC_REG_CHAN_CTRL_3(c)		(0x0418 + (c) * 0x40)
 #define   ADI_AXI_ADC_CHAN_PN_SEL_MASK		GENMASK(19, 16)
@@ -246,6 +250,30 @@ static int axi_adc_chan_status(struct iio_backend *back, unsigned int chan,
 	return 0;
 }
 
+static int axi_adc_debugfs_print_chan_status(struct iio_backend *back,
+					     unsigned int chan, char *buf,
+					     size_t len)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+	u32 val;
+	int ret;
+
+	ret = axi_adc_read_chan_status(st, chan, &val);
+	if (ret)
+		return ret;
+
+	/*
+	 * PN_ERR is cleared in case out of sync is set. Hence, no point in
+	 * checking both bits.
+	 */
+	if (val & ADI_AXI_ADC_CHAN_STAT_PN_OOS)
+		return scnprintf(buf, len, "CH%u: Out of Sync.\n", chan);
+	if (val & ADI_AXI_ADC_CHAN_STAT_PN_ERR)
+		return scnprintf(buf, len, "CH%u: Spurious Out of Sync.\n", chan);
+
+	return scnprintf(buf, len, "CH%u: OK.\n", chan);
+}
+
 static int axi_adc_chan_enable(struct iio_backend *back, unsigned int chan)
 {
 	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
@@ -280,6 +308,17 @@ static void axi_adc_free_buffer(struct iio_backend *back,
 	iio_dmaengine_buffer_free(buffer);
 }
 
+static int axi_adc_reg_access(struct iio_backend *back, unsigned int reg,
+			      unsigned int writeval, unsigned int *readval)
+{
+	struct adi_axi_adc_state *st = iio_backend_get_priv(back);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
 static const struct regmap_config axi_adc_regmap_config = {
 	.val_bits = 32,
 	.reg_bits = 32,
@@ -298,6 +337,8 @@ static const struct iio_backend_ops adi_axi_adc_generic = {
 	.iodelay_set = axi_adc_iodelays_set,
 	.test_pattern_set = axi_adc_test_pattern_set,
 	.chan_status = axi_adc_chan_status,
+	.debugfs_reg_access = iio_backend_debugfs_ptr(axi_adc_reg_access),
+	.debugfs_print_chan_status = iio_backend_debugfs_ptr(axi_adc_debugfs_print_chan_status),
 };
 
 static int adi_axi_adc_probe(struct platform_device *pdev)

-- 
2.45.2

[PATCH 8/9] iio: adc: ad9467: add backend test mode helpers

[Nuno Sa]

Group the backend configurations to be done in preparing and stopping
calibration in two new helpers analogous to ad9467_testmode_set(). This
is in preparation for adding support for debugFS test_mode where
we need similar configurations as in the calibration process.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 67 ++++++++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 557d98ca2f25..2f4bbbd5611c 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -494,11 +494,49 @@ static int ad9467_testmode_set(struct ad9467_state *st, unsigned int chan,
 				AN877_ADC_TRANSFER_SYNC);
 }
 
-static int ad9647_calibrate_prepare(struct ad9467_state *st)
+static int ad9467_backend_testmode_on(struct ad9467_state *st,
+				      unsigned int chan,
+				      enum iio_backend_test_pattern pattern)
 {
 	struct iio_backend_data_fmt data = {
 		.enable = false,
 	};
+	int ret;
+
+	ret = iio_backend_data_format_set(st->back, chan, &data);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_test_pattern_set(st->back, chan, pattern);
+	if (ret)
+		return ret;
+
+	return iio_backend_chan_enable(st->back, chan);
+}
+
+static int ad9467_backend_testmode_off(struct ad9467_state *st,
+				       unsigned int chan)
+{
+	struct iio_backend_data_fmt data = {
+		.enable = true,
+		.sign_extend = true,
+	};
+	int ret;
+
+	ret = iio_backend_chan_disable(st->back, chan);
+	if (ret)
+		return ret;
+
+	ret = iio_backend_test_pattern_set(st->back, chan,
+					   IIO_BACKEND_NO_TEST_PATTERN);
+	if (ret)
+		return ret;
+
+	return iio_backend_data_format_set(st->back, chan, &data);
+}
+
+static int ad9647_calibrate_prepare(struct ad9467_state *st)
+{
 	unsigned int c;
 	int ret;
 
@@ -511,16 +549,8 @@ static int ad9647_calibrate_prepare(struct ad9467_state *st)
 		if (ret)
 			return ret;
 
-		ret = iio_backend_data_format_set(st->back, c, &data);
-		if (ret)
-			return ret;
-
-		ret = iio_backend_test_pattern_set(st->back, c,
-						   IIO_BACKEND_ADI_PRBS_9A);
-		if (ret)
-			return ret;
-
-		ret = iio_backend_chan_enable(st->back, c);
+		ret = ad9467_backend_testmode_on(st, c,
+						 IIO_BACKEND_ADI_PRBS_9A);
 		if (ret)
 			return ret;
 	}
@@ -601,24 +631,11 @@ static int ad9467_calibrate_apply(struct ad9467_state *st, unsigned int val)
 
 static int ad9647_calibrate_stop(struct ad9467_state *st)
 {
-	struct iio_backend_data_fmt data = {
-		.sign_extend = true,
-		.enable = true,
-	};
 	unsigned int c, mode;
 	int ret;
 
 	for (c = 0; c < st->info->num_channels; c++) {
-		ret = iio_backend_chan_disable(st->back, c);
-		if (ret)
-			return ret;
-
-		ret = iio_backend_test_pattern_set(st->back, c,
-						   IIO_BACKEND_NO_TEST_PATTERN);
-		if (ret)
-			return ret;
-
-		ret = iio_backend_data_format_set(st->back, c, &data);
+		ret = ad9467_backend_testmode_off(st, c);
 		if (ret)
 			return ret;
 

-- 
2.45.2

[PATCH 9/9] iio: adc: ad9467: add digital interface test to debugfs

[Nuno Sa]

One useful thing to do (in case of problems) in this high speed devices
with digital interfaces is to try different test patterns to see if the
interface is working properly (and properly calibrated). Hence add this
to debugfs.

On top of this, for some test patterns, the backend may have a matching
validator block which can be helpful in identifying possible issues. For
the other patterns some test equipment must be used so one can look into
the signal and see how it looks like.

Hence, we also add the backend debugfs interface with
iio_backend_debugfs_add().

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 2f4bbbd5611c..ce0bae94aa3a 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <linux/seq_file.h>
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
@@ -136,6 +137,8 @@ struct ad9467_chip_info {
 	unsigned int num_channels;
 	const unsigned int (*scale_table)[2];
 	int num_scales;
+	unsigned long test_mask;
+	unsigned int test_mask_len;
 	unsigned long max_rate;
 	unsigned int default_output_mode;
 	unsigned int vref_mask;
@@ -147,11 +150,19 @@ struct ad9467_chip_info {
 	bool has_dco_invert;
 };
 
+struct ad9467_chan_test_mode {
+	struct ad9467_state *st;
+	unsigned int idx;
+	u8 mode;
+};
+
 struct ad9467_state {
 	const struct ad9467_chip_info *info;
 	struct iio_backend *back;
 	struct spi_device *spi;
 	struct clk *clk;
+	/* used for debugfs */
+	struct ad9467_chan_test_mode *chan_test;
 	unsigned int output_mode;
 	unsigned int (*scales)[2];
 	/*
@@ -308,6 +319,23 @@ static const struct iio_chan_spec ad9652_channels[] = {
 	AD9467_CHAN(1, BIT(IIO_CHAN_INFO_SCALE), 1, 16, 's'),
 };
 
+static const char * const ad9467_test_modes[] = {
+	[AN877_ADC_TESTMODE_OFF] = "off",
+	[AN877_ADC_TESTMODE_MIDSCALE_SHORT] = "midscale_short",
+	[AN877_ADC_TESTMODE_POS_FULLSCALE] = "pos_fullscale",
+	[AN877_ADC_TESTMODE_NEG_FULLSCALE] = "neg_fullscale",
+	[AN877_ADC_TESTMODE_ALT_CHECKERBOARD] = "checkerboard",
+	[AN877_ADC_TESTMODE_PN23_SEQ] = "prbs23",
+	[AN877_ADC_TESTMODE_PN9_SEQ] = "prbs9",
+	[AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE] = "one_zero_toggle",
+	[AN877_ADC_TESTMODE_USER] = "user",
+	[AN877_ADC_TESTMODE_BIT_TOGGLE] = "bit_toggle",
+	[AN877_ADC_TESTMODE_SYNC] = "sync",
+	[AN877_ADC_TESTMODE_ONE_BIT_HIGH] = "one_bit_high",
+	[AN877_ADC_TESTMODE_MIXED_BIT_FREQUENCY] = "mixed_bit_frequency",
+	[AN877_ADC_TESTMODE_RAMP] = "ramp",
+};
+
 static const struct ad9467_chip_info ad9467_chip_tbl = {
 	.name = "ad9467",
 	.id = CHIPID_AD9467,
@@ -317,6 +345,9 @@ static const struct ad9467_chip_info ad9467_chip_tbl = {
 	.channels = ad9467_channels,
 	.num_channels = ARRAY_SIZE(ad9467_channels),
 	.test_points = AD9647_MAX_TEST_POINTS,
+	.test_mask = GENMASK(AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE,
+			     AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE + 1,
 	.default_output_mode = AD9467_DEF_OUTPUT_MODE,
 	.vref_mask = AD9467_REG_VREF_MASK,
 	.num_lanes = 8,
@@ -331,6 +362,8 @@ static const struct ad9467_chip_info ad9434_chip_tbl = {
 	.channels = ad9434_channels,
 	.num_channels = ARRAY_SIZE(ad9434_channels),
 	.test_points = AD9647_MAX_TEST_POINTS,
+	.test_mask = GENMASK(AN877_ADC_TESTMODE_USER, AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_USER + 1,
 	.default_output_mode = AD9434_DEF_OUTPUT_MODE,
 	.vref_mask = AD9434_REG_VREF_MASK,
 	.num_lanes = 6,
@@ -345,6 +378,9 @@ static const struct ad9467_chip_info ad9265_chip_tbl = {
 	.channels = ad9467_channels,
 	.num_channels = ARRAY_SIZE(ad9467_channels),
 	.test_points = AD9647_MAX_TEST_POINTS,
+	.test_mask = GENMASK(AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE,
+			     AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE + 1,
 	.default_output_mode = AD9265_DEF_OUTPUT_MODE,
 	.vref_mask = AD9265_REG_VREF_MASK,
 	.has_dco = true,
@@ -360,6 +396,9 @@ static const struct ad9467_chip_info ad9643_chip_tbl = {
 	.channels = ad9643_channels,
 	.num_channels = ARRAY_SIZE(ad9643_channels),
 	.test_points = AD9647_MAX_TEST_POINTS,
+	.test_mask = BIT(AN877_ADC_TESTMODE_RAMP) |
+		GENMASK(AN877_ADC_TESTMODE_MIXED_BIT_FREQUENCY, AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_RAMP + 1,
 	.vref_mask = AD9643_REG_VREF_MASK,
 	.has_dco = true,
 	.has_dco_invert = true,
@@ -375,6 +414,9 @@ static const struct ad9467_chip_info ad9649_chip_tbl = {
 	.channels = ad9649_channels,
 	.num_channels = ARRAY_SIZE(ad9649_channels),
 	.test_points = AD9649_TEST_POINTS,
+	.test_mask = GENMASK(AN877_ADC_TESTMODE_MIXED_BIT_FREQUENCY,
+			     AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_MIXED_BIT_FREQUENCY + 1,
 	.has_dco = true,
 	.has_dco_invert = true,
 	.dco_en = AN877_ADC_DCO_DELAY_ENABLE,
@@ -389,6 +431,9 @@ static const struct ad9467_chip_info ad9652_chip_tbl = {
 	.channels = ad9652_channels,
 	.num_channels = ARRAY_SIZE(ad9652_channels),
 	.test_points = AD9647_MAX_TEST_POINTS,
+	.test_mask = GENMASK(AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE,
+			     AN877_ADC_TESTMODE_OFF),
+	.test_mask_len = AN877_ADC_TESTMODE_ONE_ZERO_TOGGLE + 1,
 	.vref_mask = AD9652_REG_VREF_MASK,
 	.has_dco = true,
 };
@@ -933,6 +978,128 @@ static int ad9467_iio_backend_get(struct ad9467_state *st)
 	return -ENODEV;
 }
 
+static int ad9467_test_mode_available_show(struct seq_file *s, void *ignored)
+{
+	struct ad9467_state *st = s->private;
+	unsigned int bit;
+
+	for_each_set_bit(bit, &st->info->test_mask, st->info->test_mask_len)
+		seq_printf(s, "%s\n", ad9467_test_modes[bit]);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(ad9467_test_mode_available);
+
+static ssize_t ad9467_chan_test_mode_read(struct file *file,
+					  char __user *userbuf, size_t count,
+					  loff_t *ppos)
+{
+	struct ad9467_chan_test_mode *chan = file->private_data;
+	struct ad9467_state *st = chan->st;
+	char buf[128] = {0};
+	size_t len;
+	int ret;
+
+	if (chan->mode == AN877_ADC_TESTMODE_PN9_SEQ ||
+	    chan->mode == AN877_ADC_TESTMODE_PN23_SEQ) {
+		len = scnprintf(buf, sizeof(buf), "Running \"%s\" Test:\n\t",
+				ad9467_test_modes[chan->mode]);
+
+		ret = iio_backend_debugfs_print_chan_status(st->back, chan->idx,
+							    buf + len,
+							    sizeof(buf) - len);
+		if (ret < 0)
+			return ret;
+		len += ret;
+	} else if (chan->mode == AN877_ADC_TESTMODE_OFF) {
+		len = scnprintf(buf, sizeof(buf), "No test Running...\n");
+	} else {
+		len = scnprintf(buf, sizeof(buf), "Running \"%s\" Test on CH:%u\n",
+				ad9467_test_modes[chan->mode], chan->idx);
+	}
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, len);
+}
+
+static ssize_t ad9467_chan_test_mode_write(struct file *file,
+					   const char __user *userbuf,
+					   size_t count, loff_t *ppos)
+{
+	struct ad9467_chan_test_mode *chan = file->private_data;
+	struct ad9467_state *st = chan->st;
+	char test_mode[32] = {0};
+	unsigned int mode;
+	int ret;
+
+	ret = simple_write_to_buffer(test_mode, sizeof(test_mode) - 1, ppos,
+				     userbuf, count);
+	if (ret < 0)
+		return ret;
+
+	for_each_set_bit(mode, &st->info->test_mask, st->info->test_mask_len) {
+		if (sysfs_streq(test_mode, ad9467_test_modes[mode]))
+			break;
+	}
+
+	if (mode == st->info->test_mask_len)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+
+	if (mode == AN877_ADC_TESTMODE_OFF) {
+		unsigned int out_mode;
+
+		if (chan->mode == AN877_ADC_TESTMODE_PN9_SEQ ||
+		    chan->mode == AN877_ADC_TESTMODE_PN23_SEQ) {
+			ret = ad9467_backend_testmode_off(st, chan->idx);
+			if (ret)
+				return ret;
+		}
+
+		ret = ad9467_testmode_set(st, chan->idx, mode);
+		if (ret)
+			return ret;
+
+		out_mode = st->info->default_output_mode | AN877_ADC_OUTPUT_MODE_TWOS_COMPLEMENT;
+		ret = ad9467_outputmode_set(st, out_mode);
+		if (ret)
+			return ret;
+	} else {
+		ret = ad9467_outputmode_set(st, st->info->default_output_mode);
+		if (ret)
+			return ret;
+
+		ret = ad9467_testmode_set(st, chan->idx, mode);
+		if (ret)
+			return ret;
+
+		/*  some patterns have a backend matching monitoring block */
+		if (mode == AN877_ADC_TESTMODE_PN9_SEQ) {
+			ret = ad9467_backend_testmode_on(st, chan->idx,
+							 IIO_BACKEND_ADI_PRBS_9A);
+			if (ret)
+				return ret;
+		} else if (mode == AN877_ADC_TESTMODE_PN23_SEQ) {
+			ret = ad9467_backend_testmode_on(st, chan->idx,
+							 IIO_BACKEND_ADI_PRBS_23A);
+			if (ret)
+				return ret;
+		}
+	}
+
+	chan->mode = mode;
+
+	return count;
+}
+
+static const struct file_operations ad9467_chan_test_mode_fops = {
+	.open = simple_open,
+	.read = ad9467_chan_test_mode_read,
+	.write = ad9467_chan_test_mode_write,
+	.llseek = default_llseek,
+	.owner = THIS_MODULE,
+};
+
 static ssize_t ad9467_dump_calib_table(struct file *file,
 				       char __user *userbuf,
 				       size_t count, loff_t *ppos)
@@ -971,12 +1138,33 @@ static void ad9467_debugfs_init(struct iio_dev *indio_dev)
 {
 	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
 	struct ad9467_state *st = iio_priv(indio_dev);
+	char attr_name[32];
+	unsigned int chan;
 
 	if (!IS_ENABLED(CONFIG_DEBUG_FS))
 		return;
 
+	st->chan_test = devm_kcalloc(&st->spi->dev, st->info->num_channels,
+				     sizeof(*st->chan_test), GFP_KERNEL);
+	if (!st->chan_test)
+		return;
+
 	debugfs_create_file("calibration_table_dump", 0400, d, st,
 			    &ad9467_calib_table_fops);
+
+	for (chan = 0; chan < st->info->num_channels; chan++) {
+		snprintf(attr_name, sizeof(attr_name), "in_voltage%u_test_mode",
+			 chan);
+		st->chan_test[chan].idx = chan;
+		st->chan_test[chan].st = st;
+		debugfs_create_file(attr_name, 0600, d, &st->chan_test[chan],
+				    &ad9467_chan_test_mode_fops);
+	}
+
+	debugfs_create_file("in_voltage_test_mode_available", 0400, d, st,
+			    &ad9467_test_mode_available_fops);
+
+	iio_backend_debugfs_add(st->back, indio_dev);
 }
 
 static int ad9467_probe(struct spi_device *spi)

-- 
2.45.2

Re: [PATCH 1/9] iio: backend: remove unused parameter

[Jonathan Cameron]

On Tue, 9 Jul 2024 13:14:28 +0200
Nuno Sa <nuno.sa@analog.com> wrote:

> Indio_dev was not being used in iio_backend_extend_chan_spec() so remove
> it.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
I'll pick this one up even if not the rest (not looked yet!)
as it seems to be obviously correct and makes sense to get
rid of that parameter now.

Jonathan

Re: [PATCH 2/9] iio: backend: print message in case op is not implemented

[Jonathan Cameron]

On Tue, 9 Jul 2024 13:14:29 +0200
Nuno Sa <nuno.sa@analog.com> wrote:

> For APIs that have a return value, -EOPNOTSUPP is returned in case the
> backend does not support the functionality. However, for APIs that do
> not have a return value we are left in silence. Hence, at least print a
> debug message in case the callback is not implemented by the backend.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Applied.

Re: [PATCH 3/9] iio: backend: add debugFs interface

[Jonathan Cameron]

On Tue, 9 Jul 2024 13:14:30 +0200
Nuno Sa <nuno.sa@analog.com> wrote:

> This adds a basic debugfs interface for backends. Two new ops are being
> added:
> 
>  * debugfs_reg_access: Analogous to the core IIO one but for backend
>    devices.
>  * debugfs_print_chan_status: One useful usecase for this one is for
>    testing test tones in a digital interface and "ask" the backend to
>    dump more details on why a test tone might have errors.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Debugfs deserved docs as well as sysfs.
Same place in Documentation/ABI/

Obviously we've neglected this in the past, but nice to do it right
nor new stuff.

one trivial comment below.

> +
> +/**
> + * iio_backend_debugfs_add - Add debugfs interfaces for Backends
> + * @back: Backend device
> + * @indio_dev: IIO device
> + */
> +void iio_backend_debugfs_add(struct iio_backend *back,
> +			     struct iio_dev *indio_dev)
> +{
> +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> +	char attr_name[128];
> +
> +	if (!IS_ENABLED(CONFIG_DEBUG_FS))
> +		return;
If this happens, d will be null anyway...  Maybe it's worth keeping
as a form of local docs though.

> +	if (!back->ops->debugfs_reg_access || !d)
> +		return;
> +
> +	snprintf(attr_name, sizeof(attr_name), "%s_direct_reg_access",
> +		 back->name);
> +
> +	debugfs_create_file(attr_name, 0644, d, back,
> +			    &iio_backend_debugfs_reg_fops);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_debugfs_add, IIO_BACKEND);
>

Re: [PATCH 3/9] iio: backend: add debugFs interface

[Nuno Sá]

On Tue, 2024-07-16 at 19:14 +0100, Jonathan Cameron wrote:
> On Tue, 9 Jul 2024 13:14:30 +0200
> Nuno Sa <nuno.sa@analog.com> wrote:
> 
> > This adds a basic debugfs interface for backends. Two new ops are being
> > added:
> > 
> >  * debugfs_reg_access: Analogous to the core IIO one but for backend
> >    devices.
> >  * debugfs_print_chan_status: One useful usecase for this one is for
> >    testing test tones in a digital interface and "ask" the backend to
> >    dump more details on why a test tone might have errors.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> Debugfs deserved docs as well as sysfs.
> Same place in Documentation/ABI/
> 
> Obviously we've neglected this in the past, but nice to do it right
> nor new stuff.
> 

I see. So you mean adding debugfs-iio?

There's one thing I'm not sure though... I'm contemplating the case where one device
may have multiple backends in which case I'm doing:

back->name = name;

where name comes from FW (DT usually). That obviously means the interface won't be
always consistent which I guess it's not a real problem for debugfs?

How would the interface look in the file? Something like?

/sys/kernel/debug/iio/iio:deviceX/<backend_name>_direct_reg_access

Or should we think in a more reliable naming? One option that came to mind is

/sys/kernel/debug/iio/iio:deviceX/backendY_direct_reg_access

where Y would be the corresponding index in io-backend-names.

One thing not optimal with the above would be identifying the actual backend device.
It would then maybe make sense having a 'backend_name' interface which I think is
likely too much just for this?

- Nuno Sá

Re: [PATCH 3/9] iio: backend: add debugFs interface

[Jonathan Cameron]

On Thu, 18 Jul 2024 16:32:33 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Tue, 2024-07-16 at 19:14 +0100, Jonathan Cameron wrote:
> > On Tue, 9 Jul 2024 13:14:30 +0200
> > Nuno Sa <nuno.sa@analog.com> wrote:
> >   
> > > This adds a basic debugfs interface for backends. Two new ops are being
> > > added:
> > > 
> > >  * debugfs_reg_access: Analogous to the core IIO one but for backend
> > >    devices.
> > >  * debugfs_print_chan_status: One useful usecase for this one is for
> > >    testing test tones in a digital interface and "ask" the backend to
> > >    dump more details on why a test tone might have errors.
> > > 
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>  
> > Debugfs deserved docs as well as sysfs.
> > Same place in Documentation/ABI/
> > 
> > Obviously we've neglected this in the past, but nice to do it right
> > nor new stuff.
> >   
> 
> I see. So you mean adding debugfs-iio?

Probably debugfs-iio-backend for this stuff, though we should have
a more general doc as well.

> 
> There's one thing I'm not sure though... I'm contemplating the case where one device
> may have multiple backends in which case I'm doing:
> 
> back->name = name;
> 
> where name comes from FW (DT usually). That obviously means the interface won't be
> always consistent which I guess it's not a real problem for debugfs?
> 
> How would the interface look in the file? Something like?
> 
> /sys/kernel/debug/iio/iio:deviceX/<backend_name>_direct_reg_access

That's fine - fairly common sort of thing to see in debugfs.

> 
> Or should we think in a more reliable naming? One option that came to mind is
> 
> /sys/kernel/debug/iio/iio:deviceX/backendY_direct_reg_access
If you were doing this it might be better as a directory.
e.g. backendY/direct_reg_access
> 
> where Y would be the corresponding index in io-backend-names.
> 
> One thing not optimal with the above would be identifying the actual backend device.
> It would then maybe make sense having a 'backend_name' interface which I think is
> likely too much just for this?
It kind of depends on your expected usecase.  These are in debugfs so there
is an assumption they aren't a 'normal operation' thing.  So if they
are going to typically be poked by a user, then complex file names are fine.
If it's going to be scripted, then stable names something like
backendY/name
backendY/direct_reg_access etc
would be easier to use.

I'm not bothered as much about consistency of this debug interface as I would
be about sysfs, so up to you (or other reviewers) for which you prefer.

Jonathan

> 
> - Nuno Sá
> 
> 

Re: [PATCH 9/9] iio: adc: ad9467: add digital interface test to debugfs

[Jonathan Cameron]

On Tue, 9 Jul 2024 13:14:36 +0200
Nuno Sa <nuno.sa@analog.com> wrote:

> One useful thing to do (in case of problems) in this high speed devices
> with digital interfaces is to try different test patterns to see if the
> interface is working properly (and properly calibrated). Hence add this
> to debugfs.
> 
> On top of this, for some test patterns, the backend may have a matching
> validator block which can be helpful in identifying possible issues. For
> the other patterns some test equipment must be used so one can look into
> the signal and see how it looks like.
> 
> Hence, we also add the backend debugfs interface with
> iio_backend_debugfs_add().
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Rest of this series looks good to me.

So just that discussion on naming / directory stuff of debugfs to resolve
and document.

Re: [PATCH 3/9] iio: backend: add debugFs interface

[Nuno Sá]

On Sat, 2024-07-20 at 10:43 +0100, Jonathan Cameron wrote:
> On Thu, 18 Jul 2024 16:32:33 +0200
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Tue, 2024-07-16 at 19:14 +0100, Jonathan Cameron wrote:
> > > On Tue, 9 Jul 2024 13:14:30 +0200
> > > Nuno Sa <nuno.sa@analog.com> wrote:
> > >   
> > > > This adds a basic debugfs interface for backends. Two new ops are being
> > > > added:
> > > > 
> > > >  * debugfs_reg_access: Analogous to the core IIO one but for backend
> > > >    devices.
> > > >  * debugfs_print_chan_status: One useful usecase for this one is for
> > > >    testing test tones in a digital interface and "ask" the backend to
> > > >    dump more details on why a test tone might have errors.
> > > > 
> > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>  
> > > Debugfs deserved docs as well as sysfs.
> > > Same place in Documentation/ABI/
> > > 
> > > Obviously we've neglected this in the past, but nice to do it right
> > > nor new stuff.
> > >   
> > 
> > I see. So you mean adding debugfs-iio?
> 
> Probably debugfs-iio-backend for this stuff, though we should have
> a more general doc as well.
> 
> > 
> > There's one thing I'm not sure though... I'm contemplating the case where
> > one device
> > may have multiple backends in which case I'm doing:
> > 
> > back->name = name;
> > 
> > where name comes from FW (DT usually). That obviously means the interface
> > won't be
> > always consistent which I guess it's not a real problem for debugfs?
> > 
> > How would the interface look in the file? Something like?
> > 
> > /sys/kernel/debug/iio/iio:deviceX/<backend_name>_direct_reg_access
> 
> That's fine - fairly common sort of thing to see in debugfs.
> 
> > 
> > Or should we think in a more reliable naming? One option that came to mind
> > is
> > 
> > /sys/kernel/debug/iio/iio:deviceX/backendY_direct_reg_access
> If you were doing this it might be better as a directory.
> e.g. backendY/direct_reg_access
> > 
> > where Y would be the corresponding index in io-backend-names.
> > 
> > One thing not optimal with the above would be identifying the actual backend
> > device.
> > It would then maybe make sense having a 'backend_name' interface which I
> > think is
> > likely too much just for this?
> It kind of depends on your expected usecase.  These are in debugfs so there
> is an assumption they aren't a 'normal operation' thing.  So if they
> are going to typically be poked by a user, then complex file names are fine.
> If it's going to be scripted, then stable names something like
> backendY/name
> backendY/direct_reg_access etc
> would be easier to use.
> 

Yeah, I think the main usage (the one I do at least) is for the user to directly
play and poke with this. However I don't think that the scripting usecase  to be
that crazy (or unlikely) and I do like stable things :). Also liked your
suggestion about grouping the interfaces in a backendY directory. We'll likely
need an iio_backend_info struct interface for backends to pass in when
registering. Maybe too much for a debug interface but this kind of 'info'
structure may very well be something we'll need in the future. Anyways, I think
I'll give this a try for v2 so we can see how it looks like in practise.

> I'm not bothered as much about consistency of this debug interface as I would
> be about sysfs, so up to you (or other reviewers) for which you prefer.
> 

Yeah, I was kind of expecting that (no one should blindly rely on debugFS) :)

- Nuno Sá