Skip to contentSkip to conversation

iio: adc: meson-saradc: switch from polling to interrupt mode

PATCH In review Heiner Kallweit 3 participants 5 messages Feb 15, 2017 – Feb 19, 2017

Subsystems (3)
Device tree bindings Arm/Amlogic Meson SoC support IIO
lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

PATCH v2 1/2 iio: adc: meson-saradc: switch from polling to interrupt mode

Switch from polling to interrupt mode.

Successfully tested on a S905GXBB-based Odroid C2.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move enabling / disabling irq to meson_sar_adc_start_sample_engine / meson_sar_adc_stop_sample_engine respectively
- move setting FIFO threshold for irq triggering to meson_sar_adc_hw_enable to avoid a potential issue when uboot should be involved in system resume
- return IRQ_NONE if the interrupt can't be for us
- document interrupts property in DT documentation

---
 .../bindings/iio/adc/amlogic,meson-saradc.txt      |  2 +
 drivers/iio/adc/meson_saradc.c                     | 56 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt +2 -0
index f9e3ff2c..04718919 100644
@@ -7,6 +7,7 @@ Required properties:
 			- "amlogic,meson-gxm-saradc" for GXM
 		along with the generic "amlogic,meson-saradc"
 - reg:		the physical base address and length of the registers
+- interrupts:	the interrupt indicating end of sampling
 - clocks:	phandle and clock identifier (see clock-names)
 - clock-names:	mandatory clocks:
 			- "clkin" for the reference clock (typically XTAL)

@@ -23,6 +24,7 @@ Example:
 		compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
 		#io-channel-cells = <1>;
 		reg = <0x0 0x8680 0x0 0x34>;
+		interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
 		clocks = <&xtal>,
 			 <&clkc CLKID_SAR_ADC>,
 			 <&clkc CLKID_SANA>,
drivers/iio/adc/meson_saradc.c +51 -5

130 lines not on this page · raw message

Signature · 1 line
2.11.1
reviewer Acked-by Tested-by on v2 1/2
#1 lore raw
Quoted 5 lines · Heiner Kallweit

Switch from polling to interrupt mode.

Successfully tested on a S905GXBB-based Odroid C2.

Signed-off-by Heiner Kallweit <[email protected]>
Acked-by Martin Blumenstingl <[email protected]>
Tested-by Martin Blumenstingl <[email protected]>

(tested on Khadas VIM board, which uses a S905X / GXL SoC) one minor comment inline below

Quoted 123 lines

---
v2:
- move enabling / disabling irq to meson_sar_adc_start_sample_engine / meson_sar_adc_stop_sample_engine respectively
- move setting FIFO threshold for irq triggering to meson_sar_adc_hw_enable to avoid a potential issue when uboot should be involved in system resume
- return IRQ_NONE if the interrupt can't be for us
- document interrupts property in DT documentation

---
 .../bindings/iio/adc/amlogic,meson-saradc.txt      |  2 +
 drivers/iio/adc/meson_saradc.c                     | 56 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt +2 -0
index f9e3ff2c..04718919 100644
@@ -7,6 +7,7 @@ Required properties:
                        - "amlogic,meson-gxm-saradc" for GXM
                along with the generic "amlogic,meson-saradc"
 - reg:         the physical base address and length of the registers
+- interrupts:  the interrupt indicating end of sampling
 - clocks:      phandle and clock identifier (see clock-names)
 - clock-names: mandatory clocks:
                        - "clkin" for the reference clock (typically XTAL)

@@ -23,6 +24,7 @@ Example:
                compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
                #io-channel-cells = <1>;
                reg = <0x0 0x8680 0x0 0x34>;
+               interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
                clocks = <&xtal>,
                         <&clkc CLKID_SAR_ADC>,
                         <&clkc CLKID_SANA>,
drivers/iio/adc/meson_saradc.c +21 -4
index 89def603..9f186de2 100644
@@ -18,7 +18,9 @@
 #include <linux/io.h>
 #include <linux/iio/iio.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>

@@ -163,6 +165,7 @@
        #define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK      GENMASK(13, 8)
 
 #define MESON_SAR_ADC_MAX_FIFO_SIZE                            32
+#define MESON_SAR_ADC_TIMEOUT                                  100 /* ms */
 
 #define MESON_SAR_ADC_CHAN(_chan) {                                    \
        .type = IIO_VOLTAGE,                                            \

@@ -229,6 +232,7 @@ struct meson_sar_adc_priv {
        struct clk_gate                         clk_gate;
        struct clk                              *adc_div_clk;
        struct clk_divider                      clk_div;
+       struct completion                       done;
 };
 
 static const struct regmap_config meson_sar_adc_regmap_config = {

@@ -274,11 +278,11 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
                                         int *val)
 {
        struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-       int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
+       int regval, fifo_chan, fifo_val, sum = 0, count = 0;
 
-       ret = meson_sar_adc_wait_busy_clear(indio_dev);
-       if (ret)
-               return ret;
+       if(!wait_for_completion_timeout(&priv->done,
+                               msecs_to_jiffies(MESON_SAR_ADC_TIMEOUT)))
+               return -ETIMEDOUT;
 
        while (meson_sar_adc_get_fifo_count(indio_dev) > 0 &&
               count < MESON_SAR_ADC_MAX_FIFO_SIZE) {

@@ -378,6 +382,12 @@ static void meson_sar_adc_start_sample_engine(struct iio_dev *indio_dev)
 {
        struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
+       reinit_completion(&priv->done);
+
+       regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                          MESON_SAR_ADC_REG0_FIFO_IRQ_EN,
+                          MESON_SAR_ADC_REG0_FIFO_IRQ_EN);
+
        regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                           MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
                           MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);

@@ -392,6 +402,9 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
        struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
        regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                          MESON_SAR_ADC_REG0_FIFO_IRQ_EN, 0);
+
+       regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                           MESON_SAR_ADC_REG0_SAMPLING_STOP,
                           MESON_SAR_ADC_REG0_SAMPLING_STOP);
 

@@ -643,6 +656,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
        struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
        int ret;
+       u32 regval;
 
        ret = meson_sar_adc_lock(indio_dev);
        if (ret)

@@ -667,6 +681,9 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
                goto err_sana_clk;
        }
 
+       regval = FIELD_PREP(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, 1);
+       regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                          MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
        regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
                           MESON_SAR_ADC_REG11_BANDGAP_EN,
                           MESON_SAR_ADC_REG11_BANDGAP_EN);

@@ -728,6 +745,25 @@ static int meson_sar_adc_hw_disable(struct iio_dev *indio_dev)
        return 0;
 }

drivers/iio/adc/meson_saradc.c static int meson_sar_adc_hw_disable(struct iio_dev *indio_dev) · line 756

Quoted 10 lines
drivers/iio/adc/meson_saradc.c +9 -0
 
+static irqreturn_t meson_sar_adc_irq(int irq, void *data)
+{
+       struct iio_dev *indio_dev = data;
+       struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+       unsigned int cnt, threshold;
+       u32 regval;
+
+       regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
+       cnt = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_COUNT_MASK, regval);

this could be replaced with a call to meson_sar_adc_get_fifo_count(), but I guess you wanted to avoid two reads of the MESON_SAR_ADC_REG0. I'm fine with either way though.

Quoted 50 lines
drivers/iio/adc/meson_saradc.c +21 -2
+       threshold = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
+
+       if (cnt < threshold)
+               return IRQ_NONE;
+
+       complete(&priv->done);
+
+       return IRQ_HANDLED;
+}
+
 static const struct iio_info meson_sar_adc_iio_info = {
        .read_raw = meson_sar_adc_iio_info_read_raw,
        .driver_module = THIS_MODULE,

@@ -770,7 +806,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
        struct resource *res;
        void __iomem *base;
        const struct of_device_id *match;
-       int ret;
+       int irq, ret;
 
        indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
        if (!indio_dev) {

@@ -779,6 +815,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
        }
 
        priv = iio_priv(indio_dev);
+       init_completion(&priv->done);
 
        match = of_match_device(meson_sar_adc_of_match, &pdev->dev);
        priv->data = match->data;

@@ -797,6 +834,15 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
        if (IS_ERR(base))
                return PTR_ERR(base);
 
+       irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+       if (!irq)
+               return -EINVAL;
+
+       ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
+                              dev_name(&pdev->dev), indio_dev);
+       if (ret)
+               return ret;
+
        priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
                                             &meson_sar_adc_regmap_config);
        if (IS_ERR(priv->regmap))
--

2.11.1

maintainer
#2 lore raw
In reply to Heiner Kallweit
Quoted 5 lines · Heiner Kallweit

Switch from polling to interrupt mode.

Successfully tested on a S905GXBB-based Odroid C2.

Signed-off-by Heiner Kallweit <[email protected]>

Looks like a nice change to me. I'll wait on Martin taking another look though before applying it.

One question that came up wrt to a similar change in another driver the other day, are there any reasons why we might not want to use the IRQ and hence preserve the existing polled approach? On the other device it turned out there was a limit on the number of interrupts that could be enabled simultaneously so there might be cases where the ADC was unimportant and they'd elect to do it without the interrupt.

I have no idea what the situation is here so thought I'd just ask!

Jonathan

Quoted 180 lines

---
v2:
- move enabling / disabling irq to meson_sar_adc_start_sample_engine / meson_sar_adc_stop_sample_engine respectively
- move setting FIFO threshold for irq triggering to meson_sar_adc_hw_enable to avoid a potential issue when uboot should be involved in system resume
- return IRQ_NONE if the interrupt can't be for us
- document interrupts property in DT documentation

---
 .../bindings/iio/adc/amlogic,meson-saradc.txt      |  2 +
 drivers/iio/adc/meson_saradc.c                     | 56 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt +2 -0
index f9e3ff2c..04718919 100644
@@ -7,6 +7,7 @@ Required properties:
 			- "amlogic,meson-gxm-saradc" for GXM
 		along with the generic "amlogic,meson-saradc"
 - reg:		the physical base address and length of the registers
+- interrupts:	the interrupt indicating end of sampling
 - clocks:	phandle and clock identifier (see clock-names)
 - clock-names:	mandatory clocks:
 			- "clkin" for the reference clock (typically XTAL)

@@ -23,6 +24,7 @@ Example:
 		compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
 		#io-channel-cells = <1>;
 		reg = <0x0 0x8680 0x0 0x34>;
+		interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
 		clocks = <&xtal>,
 			 <&clkc CLKID_SAR_ADC>,
 			 <&clkc CLKID_SANA>,
drivers/iio/adc/meson_saradc.c +51 -5
index 89def603..9f186de2 100644
@@ -18,7 +18,9 @@
 #include <linux/io.h>
 #include <linux/iio/iio.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>

@@ -163,6 +165,7 @@
 	#define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK	GENMASK(13, 8)
 
 #define MESON_SAR_ADC_MAX_FIFO_SIZE				32
+#define MESON_SAR_ADC_TIMEOUT					100 /* ms */
 
 #define MESON_SAR_ADC_CHAN(_chan) {					\
 	.type = IIO_VOLTAGE,						\

@@ -229,6 +232,7 @@ struct meson_sar_adc_priv {
 	struct clk_gate				clk_gate;
 	struct clk				*adc_div_clk;
 	struct clk_divider			clk_div;
+	struct completion			done;
 };
 
 static const struct regmap_config meson_sar_adc_regmap_config = {

@@ -274,11 +278,11 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
 					 int *val)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-	int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
+	int regval, fifo_chan, fifo_val, sum = 0, count = 0;
 
-	ret = meson_sar_adc_wait_busy_clear(indio_dev);
-	if (ret)
-		return ret;
+	if(!wait_for_completion_timeout(&priv->done,
+				msecs_to_jiffies(MESON_SAR_ADC_TIMEOUT)))
+		return -ETIMEDOUT;
 
 	while (meson_sar_adc_get_fifo_count(indio_dev) > 0 &&
 	       count < MESON_SAR_ADC_MAX_FIFO_SIZE) {

@@ -378,6 +382,12 @@ static void meson_sar_adc_start_sample_engine(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
+	reinit_completion(&priv->done);
+
+	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+			   MESON_SAR_ADC_REG0_FIFO_IRQ_EN,
+			   MESON_SAR_ADC_REG0_FIFO_IRQ_EN);
+
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
 			   MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
 			   MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);

@@ -392,6 +402,9 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+			   MESON_SAR_ADC_REG0_FIFO_IRQ_EN, 0);
+
+	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
 			   MESON_SAR_ADC_REG0_SAMPLING_STOP,
 			   MESON_SAR_ADC_REG0_SAMPLING_STOP);
 

@@ -643,6 +656,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
 	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 	int ret;
+	u32 regval;
 
 	ret = meson_sar_adc_lock(indio_dev);
 	if (ret)

@@ -667,6 +681,9 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 		goto err_sana_clk;
 	}
 
+	regval = FIELD_PREP(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, 1);
+	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+			   MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
 	regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
 			   MESON_SAR_ADC_REG11_BANDGAP_EN,
 			   MESON_SAR_ADC_REG11_BANDGAP_EN);

@@ -728,6 +745,25 @@ static int meson_sar_adc_hw_disable(struct iio_dev *indio_dev)
 	return 0;
 }
 
+static irqreturn_t meson_sar_adc_irq(int irq, void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+	unsigned int cnt, threshold;
+	u32 regval;
+
+	regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
+	cnt = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
+	threshold = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
+
+	if (cnt < threshold)
+		return IRQ_NONE;
+
+	complete(&priv->done);
+
+	return IRQ_HANDLED;
+}
+
 static const struct iio_info meson_sar_adc_iio_info = {
 	.read_raw = meson_sar_adc_iio_info_read_raw,
 	.driver_module = THIS_MODULE,

@@ -770,7 +806,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *base;
 	const struct of_device_id *match;
-	int ret;
+	int irq, ret;
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
 	if (!indio_dev) {

@@ -779,6 +815,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	}
 
 	priv = iio_priv(indio_dev);
+	init_completion(&priv->done);
 
 	match = of_match_device(meson_sar_adc_of_match, &pdev->dev);
 	priv->data = match->data;

@@ -797,6 +834,15 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (!irq)
+		return -EINVAL;
+
+	ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
+			       dev_name(&pdev->dev), indio_dev);
+	if (ret)
+		return ret;
+
 	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
 					     &meson_sar_adc_regmap_config);
 	if (IS_ERR(priv->regmap))
reviewer
#3 lore raw
Quoted 8 lines · Jonathan Cameron
Quoted 5 lines · Heiner Kallweit

Switch from polling to interrupt mode.

Successfully tested on a S905GXBB-based Odroid C2.

Signed-off-by Heiner Kallweit <[email protected]>

Looks like a nice change to me. I'll wait on Martin taking another look though before applying it.

I sent my ACK and Tested-By just a few minutes before you sent this email - so consider this done!

Quoted 7 lines

One question that came up wrt to a similar change in another driver the other day, are there any reasons why we might not want to use the IRQ and hence preserve the existing polled approach? On the other device it turned out there was a limit on the number of interrupts that could be enabled simultaneously so there might be cases where the ADC was unimportant and they'd elect to do it without the interrupt.

I have no idea what the situation is here so thought I'd just ask!

interesting question - the SoCs are using an ARM CoreLink GIC-400 interrupt controller.
I could not find a limit for the "maximum number of (simultaneously) enabled interrupts" though (all I could find was a limit of "maximum number of interrupt sources/IDs", about which the S905 datasheet page 88 says: "There are 224 interrupt sources in the chip.")

Quoted 182 lines

Jonathan

Quoted 180 lines

---
v2:
- move enabling / disabling irq to meson_sar_adc_start_sample_engine / meson_sar_adc_stop_sample_engine respectively
- move setting FIFO threshold for irq triggering to meson_sar_adc_hw_enable to avoid a potential issue when uboot should be involved in system resume
- return IRQ_NONE if the interrupt can't be for us
- document interrupts property in DT documentation

---
 .../bindings/iio/adc/amlogic,meson-saradc.txt      |  2 +
 drivers/iio/adc/meson_saradc.c                     | 56 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt +2 -0
index f9e3ff2c..04718919 100644
@@ -7,6 +7,7 @@ Required properties:
                      - "amlogic,meson-gxm-saradc" for GXM
              along with the generic "amlogic,meson-saradc"
 - reg:               the physical base address and length of the registers
+- interrupts:        the interrupt indicating end of sampling
 - clocks:    phandle and clock identifier (see clock-names)
 - clock-names:       mandatory clocks:
                      - "clkin" for the reference clock (typically XTAL)

@@ -23,6 +24,7 @@ Example:
              compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
              #io-channel-cells = <1>;
              reg = <0x0 0x8680 0x0 0x34>;
+             interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
              clocks = <&xtal>,
                       <&clkc CLKID_SAR_ADC>,
                       <&clkc CLKID_SANA>,
drivers/iio/adc/meson_saradc.c +51 -5
index 89def603..9f186de2 100644
@@ -18,7 +18,9 @@
 #include <linux/io.h>
 #include <linux/iio/iio.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>

@@ -163,6 +165,7 @@
      #define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK      GENMASK(13, 8)
 
 #define MESON_SAR_ADC_MAX_FIFO_SIZE                          32
+#define MESON_SAR_ADC_TIMEOUT                                        100 /* ms */
 
 #define MESON_SAR_ADC_CHAN(_chan) {                                  \
      .type = IIO_VOLTAGE,                                            \

@@ -229,6 +232,7 @@ struct meson_sar_adc_priv {
      struct clk_gate                         clk_gate;
      struct clk                              *adc_div_clk;
      struct clk_divider                      clk_div;
+     struct completion                       done;
 };
 
 static const struct regmap_config meson_sar_adc_regmap_config = {

@@ -274,11 +278,11 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
                                       int *val)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-     int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
+     int regval, fifo_chan, fifo_val, sum = 0, count = 0;
 
-     ret = meson_sar_adc_wait_busy_clear(indio_dev);
-     if (ret)
-             return ret;
+     if(!wait_for_completion_timeout(&priv->done,
+                             msecs_to_jiffies(MESON_SAR_ADC_TIMEOUT)))
+             return -ETIMEDOUT;
 
      while (meson_sar_adc_get_fifo_count(indio_dev) > 0 &&
             count < MESON_SAR_ADC_MAX_FIFO_SIZE) {

@@ -378,6 +382,12 @@ static void meson_sar_adc_start_sample_engine(struct iio_dev *indio_dev)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
+     reinit_completion(&priv->done);
+
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN);
+
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                         MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
                         MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);

@@ -392,6 +402,9 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN, 0);
+
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                         MESON_SAR_ADC_REG0_SAMPLING_STOP,
                         MESON_SAR_ADC_REG0_SAMPLING_STOP);
 

@@ -643,6 +656,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
      int ret;
+     u32 regval;
 
      ret = meson_sar_adc_lock(indio_dev);
      if (ret)

@@ -667,6 +681,9 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
              goto err_sana_clk;
      }
 
+     regval = FIELD_PREP(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, 1);
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
                         MESON_SAR_ADC_REG11_BANDGAP_EN,
                         MESON_SAR_ADC_REG11_BANDGAP_EN);

@@ -728,6 +745,25 @@ static int meson_sar_adc_hw_disable(struct iio_dev *indio_dev)
      return 0;
 }
 
+static irqreturn_t meson_sar_adc_irq(int irq, void *data)
+{
+     struct iio_dev *indio_dev = data;
+     struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+     unsigned int cnt, threshold;
+     u32 regval;
+
+     regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
+     cnt = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
+     threshold = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
+
+     if (cnt < threshold)
+             return IRQ_NONE;
+
+     complete(&priv->done);
+
+     return IRQ_HANDLED;
+}
+
 static const struct iio_info meson_sar_adc_iio_info = {
      .read_raw = meson_sar_adc_iio_info_read_raw,
      .driver_module = THIS_MODULE,

@@ -770,7 +806,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      struct resource *res;
      void __iomem *base;
      const struct of_device_id *match;
-     int ret;
+     int irq, ret;
 
      indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
      if (!indio_dev) {

@@ -779,6 +815,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      }
 
      priv = iio_priv(indio_dev);
+     init_completion(&priv->done);
 
      match = of_match_device(meson_sar_adc_of_match, &pdev->dev);
      priv->data = match->data;

@@ -797,6 +834,15 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      if (IS_ERR(base))
              return PTR_ERR(base);
 
+     irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+     if (!irq)
+             return -EINVAL;
+
+     ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
+                            dev_name(&pdev->dev), indio_dev);
+     if (ret)
+             return ret;
+
      priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
                                           &meson_sar_adc_regmap_config);
      if (IS_ERR(priv->regmap))
maintainer
#4 lore raw
Quoted 11 lines · Martin Blumenstingl
Quoted 8 lines · Jonathan Cameron
Quoted 5 lines · Heiner Kallweit

Switch from polling to interrupt mode.

Successfully tested on a S905GXBB-based Odroid C2.

Signed-off-by Heiner Kallweit <[email protected]>

Looks like a nice change to me. I'll wait on Martin taking another look though before applying it.

I sent my ACK and Tested-By just a few minutes before you sent this email - so consider this done!

Cool. Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Quoted 7 lines
Quoted 7 lines

One question that came up wrt to a similar change in another driver the other day, are there any reasons why we might not want to use the IRQ and hence preserve the existing polled approach? On the other device it turned out there was a limit on the number of interrupts that could be enabled simultaneously so there might be cases where the ADC was unimportant and they'd elect to do it without the interrupt.

Quoted 7 lines
Quoted 1 line

I have no idea what the situation is here so thought I'd just ask!

interesting question - the SoCs are using an ARM CoreLink GIC-400 interrupt controller.
I could not find a limit for the "maximum number of (simultaneously) enabled interrupts" though (all I could find was a limit of "maximum number of interrupt sources/IDs", about which the S905 datasheet page 88 says: "There are 224 interrupt sources in the chip.")

Sounds like we are fine then ;)

Jonathan

Quoted 188 lines
Quoted 169 lines

Jonathan

Quoted 167 lines

---
v2:
- move enabling / disabling irq to meson_sar_adc_start_sample_engine / meson_sar_adc_stop_sample_engine respectively
- move setting FIFO threshold for irq triggering to meson_sar_adc_hw_enable to avoid a potential issue when uboot should be involved in system resume
- return IRQ_NONE if the interrupt can't be for us
- document interrupts property in DT documentation

---
 .../bindings/iio/adc/amlogic,meson-saradc.txt      |  2 +
 drivers/iio/adc/meson_saradc.c                     | 56 ++++++++++++++++++++--
 2 files changed, 53 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt +2 -0
index f9e3ff2c..04718919 100644
@@ -7,6 +7,7 @@ Required properties:
                      - "amlogic,meson-gxm-saradc" for GXM
              along with the generic "amlogic,meson-saradc"
 - reg:               the physical base address and length of the registers
+- interrupts:        the interrupt indicating end of sampling
 - clocks:    phandle and clock identifier (see clock-names)
 - clock-names:       mandatory clocks:
                      - "clkin" for the reference clock (typically XTAL)

@@ -23,6 +24,7 @@ Example:
              compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
              #io-channel-cells = <1>;
              reg = <0x0 0x8680 0x0 0x34>;
+             interrupts = <GIC_SPI 73 IRQ_TYPE_EDGE_RISING>;
              clocks = <&xtal>,
                       <&clkc CLKID_SAR_ADC>,
                       <&clkc CLKID_SANA>,
drivers/iio/adc/meson_saradc.c +42 -5
index 89def603..9f186de2 100644
@@ -18,7 +18,9 @@
 #include <linux/io.h>
 #include <linux/iio/iio.h>
 #include <linux/module.h>
+#include <linux/interrupt.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>

@@ -163,6 +165,7 @@
      #define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK      GENMASK(13, 8)
 
 #define MESON_SAR_ADC_MAX_FIFO_SIZE                          32
+#define MESON_SAR_ADC_TIMEOUT                                        100 /* ms */
 
 #define MESON_SAR_ADC_CHAN(_chan) {                                  \
      .type = IIO_VOLTAGE,                                            \

@@ -229,6 +232,7 @@ struct meson_sar_adc_priv {
      struct clk_gate                         clk_gate;
      struct clk                              *adc_div_clk;
      struct clk_divider                      clk_div;
+     struct completion                       done;
 };
 
 static const struct regmap_config meson_sar_adc_regmap_config = {

@@ -274,11 +278,11 @@ static int meson_sar_adc_read_raw_sample(struct iio_dev *indio_dev,
                                       int *val)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
-     int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
+     int regval, fifo_chan, fifo_val, sum = 0, count = 0;
 
-     ret = meson_sar_adc_wait_busy_clear(indio_dev);
-     if (ret)
-             return ret;
+     if(!wait_for_completion_timeout(&priv->done,
+                             msecs_to_jiffies(MESON_SAR_ADC_TIMEOUT)))
+             return -ETIMEDOUT;
 
      while (meson_sar_adc_get_fifo_count(indio_dev) > 0 &&
             count < MESON_SAR_ADC_MAX_FIFO_SIZE) {

@@ -378,6 +382,12 @@ static void meson_sar_adc_start_sample_engine(struct iio_dev *indio_dev)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
+     reinit_completion(&priv->done);
+
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN);
+
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                         MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
                         MESON_SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);

@@ -392,6 +402,9 @@ static void meson_sar_adc_stop_sample_engine(struct iio_dev *indio_dev)
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
 
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_IRQ_EN, 0);
+
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
                         MESON_SAR_ADC_REG0_SAMPLING_STOP,
                         MESON_SAR_ADC_REG0_SAMPLING_STOP);
 

@@ -643,6 +656,7 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
 {
      struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
      int ret;
+     u32 regval;
 
      ret = meson_sar_adc_lock(indio_dev);
      if (ret)

@@ -667,6 +681,9 @@ static int meson_sar_adc_hw_enable(struct iio_dev *indio_dev)
              goto err_sana_clk;
      }
 
+     regval = FIELD_PREP(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, 1);
+     regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG0,
+                        MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
      regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
                         MESON_SAR_ADC_REG11_BANDGAP_EN,
                         MESON_SAR_ADC_REG11_BANDGAP_EN);

@@ -728,6 +745,25 @@ static int meson_sar_adc_hw_disable(struct iio_dev *indio_dev)
      return 0;
 }
 
+static irqreturn_t meson_sar_adc_irq(int irq, void *data)
+{
+     struct iio_dev *indio_dev = data;
+     struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
+     unsigned int cnt, threshold;
+     u32 regval;
+
+     regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
+     cnt = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_COUNT_MASK, regval);
+     threshold = FIELD_GET(MESON_SAR_ADC_REG0_FIFO_CNT_IRQ_MASK, regval);
+
+     if (cnt < threshold)
+             return IRQ_NONE;
+
+     complete(&priv->done);
+
+     return IRQ_HANDLED;
+}
+
 static const struct iio_info meson_sar_adc_iio_info = {
      .read_raw = meson_sar_adc_iio_info_read_raw,
      .driver_module = THIS_MODULE,

@@ -770,7 +806,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      struct resource *res;
      void __iomem *base;
      const struct of_device_id *match;
-     int ret;
+     int irq, ret;
 
      indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
      if (!indio_dev) {

@@ -779,6 +815,7 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      }
 
      priv = iio_priv(indio_dev);
+     init_completion(&priv->done);
 
      match = of_match_device(meson_sar_adc_of_match, &pdev->dev);
      priv->data = match->data;

@@ -797,6 +834,15 @@ static int meson_sar_adc_probe(struct platform_device *pdev)
      if (IS_ERR(base))
              return PTR_ERR(base);
Quoted 14 lines

drivers/iio/adc/meson_saradc.c static int meson_sar_adc_probe(struct platform_device *pdev) · line 848

Quoted 13 lines
drivers/iio/adc/meson_saradc.c +9 -0
+     irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+     if (!irq)
+             return -EINVAL;
+
+     ret = devm_request_irq(&pdev->dev, irq, meson_sar_adc_irq, IRQF_SHARED,
+                            dev_name(&pdev->dev), indio_dev);
+     if (ret)
+             return ret;
+
      priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
                                           &meson_sar_adc_regmap_config);
      if (IS_ERR(priv->regmap))

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