On 19/02/17 12:37, Martin Blumenstingl wrote:
> 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!
Cool. Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with 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!
> 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
>
>> 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
>
--
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
