On Sun, Feb 19, 2017 at 1:19 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 15/02/17 19:31, Heiner Kallweit wrote:
>> Switch from polling to interrupt mode.
>>
>> Successfully tested on a S905GXBB-based Odroid C2.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> 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!
> 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.")
> Jonathan
>> ---
>> 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(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>> index f9e3ff2c..04718919 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>> @@ -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>,
>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
>> index 89def603..9f186de2 100644
>> --- a/drivers/iio/adc/meson_saradc.c
>> +++ b/drivers/iio/adc/meson_saradc.c
>> @@ -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, ®val);
>> + 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))
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
