diff mbox series

[v6,8/9] iio: add iio backend support to sd modulator

Message ID 20240730084640.1307938-9-olivier.moysan@foss.st.com (mailing list archive)
State Accepted
Headers show
Series iio: adc: dfsdm: add scaling support | expand

Commit Message

Olivier Moysan July 30, 2024, 8:46 a.m. UTC
The legacy sd modulator driver registers the sigma delta modulator as
an IIO channel provider. This implementation is not convenient when the
SD modulator has to be cascaded with another IIO device. The scaling
information is distributed across devices, which makes it difficult to
report consistent scaling data on IIO devices.

The solution is to expose these cascaded IIO devices as an aggregate
device, which report global scaling information.
Add IIO backend support to SD modulator to allow scaling information
management.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 drivers/iio/adc/Kconfig            |  1 +
 drivers/iio/adc/sd_adc_modulator.c | 92 +++++++++++++++++++++++++++++-
 2 files changed, 92 insertions(+), 1 deletion(-)

Comments

Jonathan Cameron Aug. 6, 2024, 4:51 p.m. UTC | #1
On Tue, 30 Jul 2024 10:46:38 +0200
Olivier Moysan <olivier.moysan@foss.st.com> wrote:

> The legacy sd modulator driver registers the sigma delta modulator as
> an IIO channel provider. This implementation is not convenient when the
> SD modulator has to be cascaded with another IIO device. The scaling
> information is distributed across devices, which makes it difficult to
> report consistent scaling data on IIO devices.
> 
> The solution is to expose these cascaded IIO devices as an aggregate
> device, which report global scaling information.
> Add IIO backend support to SD modulator to allow scaling information
> management.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
I've applied this fixup given changes in the backend code that crossed
with this.

diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
index 06f9c5cacd53..654b6a38b650 100644
--- a/drivers/iio/adc/sd_adc_modulator.c
+++ b/drivers/iio/adc/sd_adc_modulator.c
@@ -74,6 +74,11 @@ static const struct iio_backend_ops sd_backend_ops = {
        .read_raw = iio_sd_mod_read,
 };
 
+static const struct iio_backend_info sd_backend_info = {
+       .name = "sd-modulator",
+       .ops = &sd_backend_ops,
+};
+
 static int iio_sd_mod_register(struct platform_device *pdev)
 {
        struct device *dev = &pdev->dev;
@@ -131,7 +136,7 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
                priv->vref_mv = ret / 1000;
        }
 
-       return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
+       return devm_iio_backend_register(&pdev->dev, &sd_backend_info, priv);
 };

Please give it a quick spin. Hopefully I didn't break anything.

Jonathan

> ---
>  drivers/iio/adc/Kconfig            |  1 +
>  drivers/iio/adc/sd_adc_modulator.c | 92 +++++++++++++++++++++++++++++-
>  2 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index bd028d59db63..43ff8182b2ea 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1195,6 +1195,7 @@ config SD_ADC_MODULATOR
>  	tristate "Generic sigma delta modulator"
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
> +	select IIO_BACKEND
>  	help
>  	  Select this option to enables sigma delta modulator. This driver can
>  	  support generic sigma delta modulators.
> diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
> index 327cc2097f6c..06f9c5cacd53 100644
> --- a/drivers/iio/adc/sd_adc_modulator.c
> +++ b/drivers/iio/adc/sd_adc_modulator.c
> @@ -6,11 +6,14 @@
>   * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>   */
>  
> +#include <linux/iio/backend.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
>  
>  static const struct iio_info iio_sd_mod_iio_info;
>  
> @@ -24,7 +27,54 @@ static const struct iio_chan_spec iio_sd_mod_ch = {
>  	},
>  };
>  
> -static int iio_sd_mod_probe(struct platform_device *pdev)
> +struct iio_sd_backend_priv {
> +	struct regulator *vref;
> +	int vref_mv;
> +};
> +
> +static int iio_sd_mod_enable(struct iio_backend *backend)
> +{
> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
> +
> +	if (priv->vref)diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
index 06f9c5cacd53..654b6a38b650 100644
--- a/drivers/iio/adc/sd_adc_modulator.c
+++ b/drivers/iio/adc/sd_adc_modulator.c
@@ -74,6 +74,11 @@ static const struct iio_backend_ops sd_backend_ops = {
        .read_raw = iio_sd_mod_read,
 };
 
+static const struct iio_backend_info sd_backend_info = {
+       .name = "sd-modulator",
+       .ops = &sd_backend_ops,
+};
+
 static int iio_sd_mod_register(struct platform_device *pdev)
 {
        struct device *dev = &pdev->dev;
@@ -131,7 +136,7 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
                priv->vref_mv = ret / 1000;
        }
 
-       return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
+       return devm_iio_backend_register(&pdev->dev, &sd_backend_info, priv);
 };
> +		return regulator_enable(priv->vref);
> +
> +	return 0;
> +};
> +
> +static void iio_sd_mod_disable(struct iio_backend *backend)
> +{
> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
> +
> +	if (priv->vref)
> +		regulator_disable(priv->vref);
> +};
> +
> +static int iio_sd_mod_read(struct iio_backend *backend, struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long mask)
> +{
> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = priv->vref_mv;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 0;
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EOPNOTSUPP;
> +};
> +
> +static const struct iio_backend_ops sd_backend_ops = {
> +	.enable = iio_sd_mod_enable,
> +	.disable = iio_sd_mod_disable,
> +	.read_raw = iio_sd_mod_read,
> +};
> +
> +static int iio_sd_mod_register(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct iio_dev *iio;
> @@ -45,6 +95,45 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
>  	return devm_iio_device_register(&pdev->dev, iio);
>  }
>  
> +static int iio_sd_mod_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct regulator *vref;
> +	struct iio_sd_backend_priv *priv;
> +	int ret;
> +
> +	/* If sd modulator is not defined as an IIO backend device, fallback to legacy */
> +	if (!device_property_present(dev, "#io-backend-cells"))
> +		return iio_sd_mod_register(pdev);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Get regulator reference if any, but don't enable regulator right now.
> +	 * Rely on enable and disable callbacks to manage regulator power.
> +	 */
> +	vref = devm_regulator_get_optional(dev, "vref");
> +	if (IS_ERR(vref)) {
> +		if (PTR_ERR(vref) != -ENODEV)
> +			return dev_err_probe(dev, PTR_ERR(vref), "Failed to get vref\n");
> +	} else {
> +		/*
> +		 * Retrieve voltage right now, as regulator_get_voltage() provides it whatever
> +		 * the state of the regulator.
> +		 */
> +		ret = regulator_get_voltage(vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		priv->vref = vref;
> +		priv->vref_mv = ret / 1000;
> +	}
> +
> +	return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
> +};
> +
>  static const struct of_device_id sd_adc_of_match[] = {
>  	{ .compatible = "sd-modulator" },
>  	{ .compatible = "ads1201" },
> @@ -65,3 +154,4 @@ module_platform_driver(iio_sd_mod_adc);
>  MODULE_DESCRIPTION("Basic sigma delta modulator");
>  MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>  MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(IIO_BACKEND);
Olivier Moysan Aug. 7, 2024, 7:40 a.m. UTC | #2
Hi Jonathan,

On 8/6/24 18:51, Jonathan Cameron wrote:
> On Tue, 30 Jul 2024 10:46:38 +0200
> Olivier Moysan <olivier.moysan@foss.st.com> wrote:
> 
>> The legacy sd modulator driver registers the sigma delta modulator as
>> an IIO channel provider. This implementation is not convenient when the
>> SD modulator has to be cascaded with another IIO device. The scaling
>> information is distributed across devices, which makes it difficult to
>> report consistent scaling data on IIO devices.
>>
>> The solution is to expose these cascaded IIO devices as an aggregate
>> device, which report global scaling information.
>> Add IIO backend support to SD modulator to allow scaling information
>> management.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> I've applied this fixup given changes in the backend code that crossed
> with this.
> 
> diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
> index 06f9c5cacd53..654b6a38b650 100644
> --- a/drivers/iio/adc/sd_adc_modulator.c
> +++ b/drivers/iio/adc/sd_adc_modulator.c
> @@ -74,6 +74,11 @@ static const struct iio_backend_ops sd_backend_ops = {
>          .read_raw = iio_sd_mod_read,
>   };
>   
> +static const struct iio_backend_info sd_backend_info = {
> +       .name = "sd-modulator",
> +       .ops = &sd_backend_ops,
> +};
> +
>   static int iio_sd_mod_register(struct platform_device *pdev)
>   {
>          struct device *dev = &pdev->dev;
> @@ -131,7 +136,7 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
>                  priv->vref_mv = ret / 1000;
>          }
>   
> -       return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
> +       return devm_iio_backend_register(&pdev->dev, &sd_backend_info, priv);
>   };
> 
> Please give it a quick spin. Hopefully I didn't break anything.
> 
> Jonathan

I did a quick check. All looks fine to me.
Thanks for the fixup and the merge.

Regards
Olivier

> 
>> ---
>>   drivers/iio/adc/Kconfig            |  1 +
>>   drivers/iio/adc/sd_adc_modulator.c | 92 +++++++++++++++++++++++++++++-
>>   2 files changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index bd028d59db63..43ff8182b2ea 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -1195,6 +1195,7 @@ config SD_ADC_MODULATOR
>>   	tristate "Generic sigma delta modulator"
>>   	select IIO_BUFFER
>>   	select IIO_TRIGGERED_BUFFER
>> +	select IIO_BACKEND
>>   	help
>>   	  Select this option to enables sigma delta modulator. This driver can
>>   	  support generic sigma delta modulators.
>> diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
>> index 327cc2097f6c..06f9c5cacd53 100644
>> --- a/drivers/iio/adc/sd_adc_modulator.c
>> +++ b/drivers/iio/adc/sd_adc_modulator.c
>> @@ -6,11 +6,14 @@
>>    * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>>    */
>>   
>> +#include <linux/iio/backend.h>
>>   #include <linux/iio/iio.h>
>>   #include <linux/iio/triggered_buffer.h>
>>   #include <linux/module.h>
>>   #include <linux/mod_devicetable.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/property.h>
>> +#include <linux/regulator/consumer.h>
>>   
>>   static const struct iio_info iio_sd_mod_iio_info;
>>   
>> @@ -24,7 +27,54 @@ static const struct iio_chan_spec iio_sd_mod_ch = {
>>   	},
>>   };
>>   
>> -static int iio_sd_mod_probe(struct platform_device *pdev)
>> +struct iio_sd_backend_priv {
>> +	struct regulator *vref;
>> +	int vref_mv;
>> +};
>> +
>> +static int iio_sd_mod_enable(struct iio_backend *backend)
>> +{
>> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
>> +
>> +	if (priv->vref)diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
> index 06f9c5cacd53..654b6a38b650 100644
> --- a/drivers/iio/adc/sd_adc_modulator.c
> +++ b/drivers/iio/adc/sd_adc_modulator.c
> @@ -74,6 +74,11 @@ static const struct iio_backend_ops sd_backend_ops = {
>          .read_raw = iio_sd_mod_read,
>   };
>   
> +static const struct iio_backend_info sd_backend_info = {
> +       .name = "sd-modulator",
> +       .ops = &sd_backend_ops,
> +};
> +
>   static int iio_sd_mod_register(struct platform_device *pdev)
>   {
>          struct device *dev = &pdev->dev;
> @@ -131,7 +136,7 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
>                  priv->vref_mv = ret / 1000;
>          }
>   
> -       return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
> +       return devm_iio_backend_register(&pdev->dev, &sd_backend_info, priv);
>   };
>> +		return regulator_enable(priv->vref);
>> +
>> +	return 0;
>> +};
>> +
>> +static void iio_sd_mod_disable(struct iio_backend *backend)
>> +{
>> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
>> +
>> +	if (priv->vref)
>> +		regulator_disable(priv->vref);
>> +};
>> +
>> +static int iio_sd_mod_read(struct iio_backend *backend, struct iio_chan_spec const *chan, int *val,
>> +			   int *val2, long mask)
>> +{
>> +	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = priv->vref_mv;
>> +		return IIO_VAL_INT;
>> +
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		*val = 0;
>> +		return IIO_VAL_INT;
>> +	}
>> +
>> +	return -EOPNOTSUPP;
>> +};
>> +
>> +static const struct iio_backend_ops sd_backend_ops = {
>> +	.enable = iio_sd_mod_enable,
>> +	.disable = iio_sd_mod_disable,
>> +	.read_raw = iio_sd_mod_read,
>> +};
>> +
>> +static int iio_sd_mod_register(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct iio_dev *iio;
>> @@ -45,6 +95,45 @@ static int iio_sd_mod_probe(struct platform_device *pdev)
>>   	return devm_iio_device_register(&pdev->dev, iio);
>>   }
>>   
>> +static int iio_sd_mod_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct regulator *vref;
>> +	struct iio_sd_backend_priv *priv;
>> +	int ret;
>> +
>> +	/* If sd modulator is not defined as an IIO backend device, fallback to legacy */
>> +	if (!device_property_present(dev, "#io-backend-cells"))
>> +		return iio_sd_mod_register(pdev);
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * Get regulator reference if any, but don't enable regulator right now.
>> +	 * Rely on enable and disable callbacks to manage regulator power.
>> +	 */
>> +	vref = devm_regulator_get_optional(dev, "vref");
>> +	if (IS_ERR(vref)) {
>> +		if (PTR_ERR(vref) != -ENODEV)
>> +			return dev_err_probe(dev, PTR_ERR(vref), "Failed to get vref\n");
>> +	} else {
>> +		/*
>> +		 * Retrieve voltage right now, as regulator_get_voltage() provides it whatever
>> +		 * the state of the regulator.
>> +		 */
>> +		ret = regulator_get_voltage(vref);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		priv->vref = vref;
>> +		priv->vref_mv = ret / 1000;
>> +	}
>> +
>> +	return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
>> +};
>> +
>>   static const struct of_device_id sd_adc_of_match[] = {
>>   	{ .compatible = "sd-modulator" },
>>   	{ .compatible = "ads1201" },
>> @@ -65,3 +154,4 @@ module_platform_driver(iio_sd_mod_adc);
>>   MODULE_DESCRIPTION("Basic sigma delta modulator");
>>   MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>>   MODULE_LICENSE("GPL v2");
>> +MODULE_IMPORT_NS(IIO_BACKEND);
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index bd028d59db63..43ff8182b2ea 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1195,6 +1195,7 @@  config SD_ADC_MODULATOR
 	tristate "Generic sigma delta modulator"
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select IIO_BACKEND
 	help
 	  Select this option to enables sigma delta modulator. This driver can
 	  support generic sigma delta modulators.
diff --git a/drivers/iio/adc/sd_adc_modulator.c b/drivers/iio/adc/sd_adc_modulator.c
index 327cc2097f6c..06f9c5cacd53 100644
--- a/drivers/iio/adc/sd_adc_modulator.c
+++ b/drivers/iio/adc/sd_adc_modulator.c
@@ -6,11 +6,14 @@ 
  * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
  */
 
+#include <linux/iio/backend.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
 
 static const struct iio_info iio_sd_mod_iio_info;
 
@@ -24,7 +27,54 @@  static const struct iio_chan_spec iio_sd_mod_ch = {
 	},
 };
 
-static int iio_sd_mod_probe(struct platform_device *pdev)
+struct iio_sd_backend_priv {
+	struct regulator *vref;
+	int vref_mv;
+};
+
+static int iio_sd_mod_enable(struct iio_backend *backend)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	if (priv->vref)
+		return regulator_enable(priv->vref);
+
+	return 0;
+};
+
+static void iio_sd_mod_disable(struct iio_backend *backend)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	if (priv->vref)
+		regulator_disable(priv->vref);
+};
+
+static int iio_sd_mod_read(struct iio_backend *backend, struct iio_chan_spec const *chan, int *val,
+			   int *val2, long mask)
+{
+	struct iio_sd_backend_priv *priv = iio_backend_get_priv(backend);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*val = priv->vref_mv;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 0;
+		return IIO_VAL_INT;
+	}
+
+	return -EOPNOTSUPP;
+};
+
+static const struct iio_backend_ops sd_backend_ops = {
+	.enable = iio_sd_mod_enable,
+	.disable = iio_sd_mod_disable,
+	.read_raw = iio_sd_mod_read,
+};
+
+static int iio_sd_mod_register(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct iio_dev *iio;
@@ -45,6 +95,45 @@  static int iio_sd_mod_probe(struct platform_device *pdev)
 	return devm_iio_device_register(&pdev->dev, iio);
 }
 
+static int iio_sd_mod_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regulator *vref;
+	struct iio_sd_backend_priv *priv;
+	int ret;
+
+	/* If sd modulator is not defined as an IIO backend device, fallback to legacy */
+	if (!device_property_present(dev, "#io-backend-cells"))
+		return iio_sd_mod_register(pdev);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	/*
+	 * Get regulator reference if any, but don't enable regulator right now.
+	 * Rely on enable and disable callbacks to manage regulator power.
+	 */
+	vref = devm_regulator_get_optional(dev, "vref");
+	if (IS_ERR(vref)) {
+		if (PTR_ERR(vref) != -ENODEV)
+			return dev_err_probe(dev, PTR_ERR(vref), "Failed to get vref\n");
+	} else {
+		/*
+		 * Retrieve voltage right now, as regulator_get_voltage() provides it whatever
+		 * the state of the regulator.
+		 */
+		ret = regulator_get_voltage(vref);
+		if (ret < 0)
+			return ret;
+
+		priv->vref = vref;
+		priv->vref_mv = ret / 1000;
+	}
+
+	return devm_iio_backend_register(&pdev->dev, &sd_backend_ops, priv);
+};
+
 static const struct of_device_id sd_adc_of_match[] = {
 	{ .compatible = "sd-modulator" },
 	{ .compatible = "ads1201" },
@@ -65,3 +154,4 @@  module_platform_driver(iio_sd_mod_adc);
 MODULE_DESCRIPTION("Basic sigma delta modulator");
 MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(IIO_BACKEND);