This patchset fixes the
1. bug causing a crash during module removal for exynos_adc.ko.
-> The bug was seen by Doug, while trying to compile the whole IIO subsystem
as module @ https://lkml.org/lkml/2014/4/21/481 from Doug.
2. rearrange the clock and regulator enable/disable calls during
probe, remove, suspend and resume calls
-> Comments give by Jonathan @ https://lkml.org/lkml/2014/4/23/644
3. reduces the timeout and uses wait_for_completion_timeout instead of the
interruptible varient.
-> This change was under review @ https://lkml.org/lkml/2013/11/5/92
Final comments were given by Tomasz, to split and submit.
Naveen Krishna Ch (2):
iio: exynos_adc: use indio_dev->dev structure to handle child nodes
iio: exynos_adc: rearrange clk and regulator enable/disable calls
Naveen Krishna Chatradhi (3):
iio: exynos_adc: reduce timeout and use wait_for_completion_timeout
iio: exynos_adc: do a soft reset in case of timeout
iio: exynos_adc: do a reinit_completion before the conversion
drivers/iio/adc/exynos_adc.c | 137 +++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 63 deletions(-)
--
1.7.9.5
Add reinit_completion() before the wait_for_completion_timeout in
raw_read() call.
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
---
Changes since v2:
None
Changes since v1:
None
v0:
This change is a part of the patch reviewed at https://lkml.org/lkml/2013/11/5/92
drivers/iio/adc/exynos_adc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index eddc58e..010578f 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -151,6 +151,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
return -EINVAL;
mutex_lock(&indio_dev->mlock);
+ reinit_completion(&info->completion);
/* Select the channel to be used and Trigger conversion */
if (info->version == ADC_V2) {
--
1.7.9.5
Do a soft reset software if a timeout happens.
This is applicable only for ADC_V2.
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
---
Changes since v2:
None
Changes since v1:
None
v0:
This change is a part of the patch reviewed at https://lkml.org/lkml/2013/11/5/92
drivers/iio/adc/exynos_adc.c | 50 ++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 939aaf7..eddc58e 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -112,6 +112,30 @@ static inline unsigned int exynos_adc_get_version(struct platform_device *pdev)
return (unsigned int)match->data;
}
+static void exynos_adc_hw_init(struct exynos_adc *info)
+{
+ u32 con1, con2;
+
+ if (info->version == ADC_V2) {
+ con1 = ADC_V2_CON1_SOFT_RESET;
+ writel(con1, ADC_V2_CON1(info->regs));
+
+ con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
+ ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
+ writel(con2, ADC_V2_CON2(info->regs));
+
+ /* Enable interrupts */
+ writel(1, ADC_V2_INT_EN(info->regs));
+ } else {
+ /* set default prescaler values and Enable prescaler */
+ con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
+
+ /* Enable 12-bit ADC resolution */
+ con1 |= ADC_V1_CON_RES;
+ writel(con1, ADC_V1_CON(info->regs));
+ }
+}
+
static int exynos_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val,
@@ -149,6 +173,8 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
timeout = wait_for_completion_timeout
(&info->completion, EXYNOS_ADC_TIMEOUT);
if (timeout == 0) {
+ dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
+ exynos_adc_hw_init(info);
ret = -ETIMEDOUT;
} else {
*val = info->value;
@@ -230,30 +256,6 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
return 0;
}
-static void exynos_adc_hw_init(struct exynos_adc *info)
-{
- u32 con1, con2;
-
- if (info->version == ADC_V2) {
- con1 = ADC_V2_CON1_SOFT_RESET;
- writel(con1, ADC_V2_CON1(info->regs));
-
- con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
- ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
- writel(con2, ADC_V2_CON2(info->regs));
-
- /* Enable interrupts */
- writel(1, ADC_V2_INT_EN(info->regs));
- } else {
- /* set default prescaler values and Enable prescaler */
- con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
-
- /* Enable 12-bit ADC resolution */
- con1 |= ADC_V1_CON_RES;
- writel(con1, ADC_V1_CON(info->regs));
- }
-}
-
static int exynos_adc_probe(struct platform_device *pdev)
{
struct exynos_adc *info = NULL;
--
1.7.9.5
ADC module on Exynos5 SoCs runs at 600KSPS. At this conversion rate,
waiting for 1000 msecs is wasteful (incase of h/w failure).
Hence, reduce the time out to 100msecs and use
wait_for_completion_timeout() instead of
wait_for_completion_interruptible_timeout()
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
---
Changes since v2:
None
Changes since v1:
None
v0:
This change is a part of the patch reviewd at https://lkml.org/lkml/2013/11/5/92
drivers/iio/adc/exynos_adc.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index ff98c5d..939aaf7 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -82,7 +82,7 @@ enum adc_version {
#define ADC_CON_EN_START (1u << 0)
#define ADC_DATX_MASK 0xFFF
-#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(1000))
+#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
struct exynos_adc {
void __iomem *regs;
@@ -121,6 +121,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
struct exynos_adc *info = iio_priv(indio_dev);
unsigned long timeout;
u32 con1, con2;
+ int ret;
if (mask != IIO_CHAN_INFO_RAW)
return -EINVAL;
@@ -145,16 +146,19 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
ADC_V1_CON(info->regs));
}
- timeout = wait_for_completion_interruptible_timeout
+ timeout = wait_for_completion_timeout
(&info->completion, EXYNOS_ADC_TIMEOUT);
- *val = info->value;
+ if (timeout == 0) {
+ ret = -ETIMEDOUT;
+ } else {
+ *val = info->value;
+ *val2 = 0;
+ ret = IIO_VAL_INT;
+ }
mutex_unlock(&indio_dev->mlock);
- if (timeout == 0)
- return -ETIMEDOUT;
-
- return IIO_VAL_INT;
+ return ret;
}
static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
--
1.7.9.5
From: Naveen Krishna Ch <[email protected]>
Using pdev->dev with device_for_each_child() would iterate over all
of the children of the platform device and delete them.
Thus, causing crashes during module unload.
We should be using the indio_dev->dev structure for
registering/unregistering child nodes.
Signed-off-by: Naveen Krishna Ch <[email protected]>
Reported-by: Doug Anderson <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
Tested-by: Doug Anderson <[email protected]>
---
Changes since v2:
None
Changes since v1:
Adding Doug's tags
v0:
This change was tested on top of
https://lkml.org/lkml/2014/4/21/481 from Doug.
drivers/iio/adc/exynos_adc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index d25b262..affa93f 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -344,7 +344,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
exynos_adc_hw_init(info);
- ret = of_platform_populate(np, exynos_adc_match, NULL, &pdev->dev);
+ ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
if (ret < 0) {
dev_err(&pdev->dev, "failed adding child nodes\n");
goto err_of_populate;
@@ -353,7 +353,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
return 0;
err_of_populate:
- device_for_each_child(&pdev->dev, NULL,
+ device_for_each_child(&indio_dev->dev, NULL,
exynos_adc_remove_devices);
regulator_disable(info->vdd);
clk_disable_unprepare(info->clk);
@@ -369,7 +369,7 @@ static int exynos_adc_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct exynos_adc *info = iio_priv(indio_dev);
- device_for_each_child(&pdev->dev, NULL,
+ device_for_each_child(&indio_dev->dev, NULL,
exynos_adc_remove_devices);
regulator_disable(info->vdd);
clk_disable_unprepare(info->clk);
--
1.7.9.5
From: Naveen Krishna Ch <[email protected]>
This patch maintains the following order in
probe(), remove(), resume() and suspend() calls
regulator enable, clk prepare enable
...
clk disable unprepare, regulator disable
While at it,
1. enable the regulator before the iio_device_register()
2. handle the return values for enable/disable calls
Signed-off-by: Naveen Krishna Ch <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
---
Changes since v2:
Remove extra unused line and add Doug's Reviewed-by
Changes since v1:
corrected the register/unregister and enabling/disbaling sequences
drivers/iio/adc/exynos_adc.c | 62 ++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index affa93f..ff98c5d 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -290,32 +290,30 @@ static int exynos_adc_probe(struct platform_device *pdev)
init_completion(&info->completion);
- ret = request_irq(info->irq, exynos_adc_isr,
- 0, dev_name(&pdev->dev), info);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
- info->irq);
- return ret;
- }
-
- writel(1, info->enable_reg);
-
info->clk = devm_clk_get(&pdev->dev, "adc");
if (IS_ERR(info->clk)) {
dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
PTR_ERR(info->clk));
- ret = PTR_ERR(info->clk);
- goto err_irq;
+ return PTR_ERR(info->clk);
}
info->vdd = devm_regulator_get(&pdev->dev, "vdd");
if (IS_ERR(info->vdd)) {
dev_err(&pdev->dev, "failed getting regulator, err = %ld\n",
PTR_ERR(info->vdd));
- ret = PTR_ERR(info->vdd);
- goto err_irq;
+ return PTR_ERR(info->vdd);
}
+ ret = regulator_enable(info->vdd);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(info->clk);
+ if (ret)
+ goto err_disable_reg;
+
+ writel(1, info->enable_reg);
+
info->version = exynos_adc_get_version(pdev);
platform_set_drvdata(pdev, indio_dev);
@@ -332,16 +330,18 @@ static int exynos_adc_probe(struct platform_device *pdev)
else
indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
+ ret = request_irq(info->irq, exynos_adc_isr,
+ 0, dev_name(&pdev->dev), info);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
+ info->irq);
+ goto err_disable_clk;
+ }
+
ret = iio_device_register(indio_dev);
if (ret)
goto err_irq;
- ret = regulator_enable(info->vdd);
- if (ret)
- goto err_iio_dev;
-
- clk_prepare_enable(info->clk);
-
exynos_adc_hw_init(info);
ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
@@ -355,12 +355,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
err_of_populate:
device_for_each_child(&indio_dev->dev, NULL,
exynos_adc_remove_devices);
- regulator_disable(info->vdd);
- clk_disable_unprepare(info->clk);
-err_iio_dev:
iio_device_unregister(indio_dev);
err_irq:
free_irq(info->irq, info);
+err_disable_clk:
+ writel(0, info->enable_reg);
+ clk_disable_unprepare(info->clk);
+err_disable_reg:
+ regulator_disable(info->vdd);
return ret;
}
@@ -371,11 +373,11 @@ static int exynos_adc_remove(struct platform_device *pdev)
device_for_each_child(&indio_dev->dev, NULL,
exynos_adc_remove_devices);
- regulator_disable(info->vdd);
- clk_disable_unprepare(info->clk);
- writel(0, info->enable_reg);
iio_device_unregister(indio_dev);
free_irq(info->irq, info);
+ writel(0, info->enable_reg);
+ clk_disable_unprepare(info->clk);
+ regulator_disable(info->vdd);
return 0;
}
@@ -397,8 +399,8 @@ static int exynos_adc_suspend(struct device *dev)
writel(con, ADC_V1_CON(info->regs));
}
- clk_disable_unprepare(info->clk);
writel(0, info->enable_reg);
+ clk_disable_unprepare(info->clk);
regulator_disable(info->vdd);
return 0;
@@ -414,9 +416,11 @@ static int exynos_adc_resume(struct device *dev)
if (ret)
return ret;
- writel(1, info->enable_reg);
- clk_prepare_enable(info->clk);
+ ret = clk_prepare_enable(info->clk);
+ if (ret)
+ return ret;
+ writel(1, info->enable_reg);
exynos_adc_hw_init(info);
return 0;
--
1.7.9.5
On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
> From: Naveen Krishna Ch <[email protected]>
>
> Using pdev->dev with device_for_each_child() would iterate over all
> of the children of the platform device and delete them.
> Thus, causing crashes during module unload.
>
> We should be using the indio_dev->dev structure for
> registering/unregistering child nodes.
>
> Signed-off-by: Naveen Krishna Ch <[email protected]>
> Reported-by: Doug Anderson <[email protected]>
> Reviewed-by: Doug Anderson <[email protected]>
> Tested-by: Doug Anderson <[email protected]>
This one has been in my fixes-togreg branch for a few days and
a pull request has gone to Greg.
> ---
> Changes since v2:
> None
> Changes since v1:
> Adding Doug's tags
> v0:
> This change was tested on top of
> https://lkml.org/lkml/2014/4/21/481 from Doug.
>
> drivers/iio/adc/exynos_adc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index d25b262..affa93f 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -344,7 +344,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
>
> exynos_adc_hw_init(info);
>
> - ret = of_platform_populate(np, exynos_adc_match, NULL, &pdev->dev);
> + ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
> if (ret < 0) {
> dev_err(&pdev->dev, "failed adding child nodes\n");
> goto err_of_populate;
> @@ -353,7 +353,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
> return 0;
>
> err_of_populate:
> - device_for_each_child(&pdev->dev, NULL,
> + device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> regulator_disable(info->vdd);
> clk_disable_unprepare(info->clk);
> @@ -369,7 +369,7 @@ static int exynos_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct exynos_adc *info = iio_priv(indio_dev);
>
> - device_for_each_child(&pdev->dev, NULL,
> + device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> regulator_disable(info->vdd);
> clk_disable_unprepare(info->clk);
>
On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
> From: Naveen Krishna Ch <[email protected]>
>
> This patch maintains the following order in
> probe(), remove(), resume() and suspend() calls
>
> regulator enable, clk prepare enable
> ...
> clk disable unprepare, regulator disable
>
> While at it,
> 1. enable the regulator before the iio_device_register()
> 2. handle the return values for enable/disable calls
>
> Signed-off-by: Naveen Krishna Ch <[email protected]>
> Reviewed-by: Doug Anderson <[email protected]>
Applied to the togreg branch of iio.git.
Thanks,
> ---
> Changes since v2:
> Remove extra unused line and add Doug's Reviewed-by
> Changes since v1:
> corrected the register/unregister and enabling/disbaling sequences
>
> drivers/iio/adc/exynos_adc.c | 62 ++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index affa93f..ff98c5d 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -290,32 +290,30 @@ static int exynos_adc_probe(struct platform_device *pdev)
>
> init_completion(&info->completion);
>
> - ret = request_irq(info->irq, exynos_adc_isr,
> - 0, dev_name(&pdev->dev), info);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> - info->irq);
> - return ret;
> - }
> -
> - writel(1, info->enable_reg);
> -
> info->clk = devm_clk_get(&pdev->dev, "adc");
> if (IS_ERR(info->clk)) {
> dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> PTR_ERR(info->clk));
> - ret = PTR_ERR(info->clk);
> - goto err_irq;
> + return PTR_ERR(info->clk);
> }
>
> info->vdd = devm_regulator_get(&pdev->dev, "vdd");
> if (IS_ERR(info->vdd)) {
> dev_err(&pdev->dev, "failed getting regulator, err = %ld\n",
> PTR_ERR(info->vdd));
> - ret = PTR_ERR(info->vdd);
> - goto err_irq;
> + return PTR_ERR(info->vdd);
> }
>
> + ret = regulator_enable(info->vdd);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(info->clk);
> + if (ret)
> + goto err_disable_reg;
> +
> + writel(1, info->enable_reg);
> +
> info->version = exynos_adc_get_version(pdev);
>
> platform_set_drvdata(pdev, indio_dev);
> @@ -332,16 +330,18 @@ static int exynos_adc_probe(struct platform_device *pdev)
> else
> indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
>
> + ret = request_irq(info->irq, exynos_adc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> + info->irq);
> + goto err_disable_clk;
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret)
> goto err_irq;
>
> - ret = regulator_enable(info->vdd);
> - if (ret)
> - goto err_iio_dev;
> -
> - clk_prepare_enable(info->clk);
> -
> exynos_adc_hw_init(info);
>
> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
> @@ -355,12 +355,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
> err_of_populate:
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> - regulator_disable(info->vdd);
> - clk_disable_unprepare(info->clk);
> -err_iio_dev:
> iio_device_unregister(indio_dev);
> err_irq:
> free_irq(info->irq, info);
> +err_disable_clk:
> + writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> +err_disable_reg:
> + regulator_disable(info->vdd);
> return ret;
> }
>
> @@ -371,11 +373,11 @@ static int exynos_adc_remove(struct platform_device *pdev)
>
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> - regulator_disable(info->vdd);
> - clk_disable_unprepare(info->clk);
> - writel(0, info->enable_reg);
> iio_device_unregister(indio_dev);
> free_irq(info->irq, info);
> + writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> + regulator_disable(info->vdd);
>
> return 0;
> }
> @@ -397,8 +399,8 @@ static int exynos_adc_suspend(struct device *dev)
> writel(con, ADC_V1_CON(info->regs));
> }
>
> - clk_disable_unprepare(info->clk);
> writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> regulator_disable(info->vdd);
>
> return 0;
> @@ -414,9 +416,11 @@ static int exynos_adc_resume(struct device *dev)
> if (ret)
> return ret;
>
> - writel(1, info->enable_reg);
> - clk_prepare_enable(info->clk);
> + ret = clk_prepare_enable(info->clk);
> + if (ret)
> + return ret;
>
> + writel(1, info->enable_reg);
> exynos_adc_hw_init(info);
>
> return 0;
>
On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
> ADC module on Exynos5 SoCs runs at 600KSPS. At this conversion rate,
> waiting for 1000 msecs is wasteful (incase of h/w failure).
>
> Hence, reduce the time out to 100msecs and use
> wait_for_completion_timeout() instead of
> wait_for_completion_interruptible_timeout()
>
> Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
> Reviewed-by: Doug Anderson <[email protected]>
Applied to the togreg branch of iio.git
Thanks
> ---
> Changes since v2:
> None
> Changes since v1:
> None
> v0:
> This change is a part of the patch reviewd at https://lkml.org/lkml/2013/11/5/92
>
> drivers/iio/adc/exynos_adc.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index ff98c5d..939aaf7 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -82,7 +82,7 @@ enum adc_version {
> #define ADC_CON_EN_START (1u << 0)
> #define ADC_DATX_MASK 0xFFF
>
> -#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(1000))
> +#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>
> struct exynos_adc {
> void __iomem *regs;
> @@ -121,6 +121,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> struct exynos_adc *info = iio_priv(indio_dev);
> unsigned long timeout;
> u32 con1, con2;
> + int ret;
>
> if (mask != IIO_CHAN_INFO_RAW)
> return -EINVAL;
> @@ -145,16 +146,19 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> ADC_V1_CON(info->regs));
> }
>
> - timeout = wait_for_completion_interruptible_timeout
> + timeout = wait_for_completion_timeout
> (&info->completion, EXYNOS_ADC_TIMEOUT);
> - *val = info->value;
> + if (timeout == 0) {
> + ret = -ETIMEDOUT;
> + } else {
> + *val = info->value;
> + *val2 = 0;
> + ret = IIO_VAL_INT;
> + }
>
> mutex_unlock(&indio_dev->mlock);
>
> - if (timeout == 0)
> - return -ETIMEDOUT;
> -
> - return IIO_VAL_INT;
> + return ret;
> }
>
> static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>
On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
> Do a soft reset software if a timeout happens.
> This is applicable only for ADC_V2.
>
> Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
> Reviewed-by: Doug Anderson <[email protected]>
Applied to the togreg branch of iio.git
Thanks,
> ---
> Changes since v2:
> None
> Changes since v1:
> None
> v0:
> This change is a part of the patch reviewed at https://lkml.org/lkml/2013/11/5/92
>
> drivers/iio/adc/exynos_adc.c | 50 ++++++++++++++++++++++--------------------
> 1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 939aaf7..eddc58e 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -112,6 +112,30 @@ static inline unsigned int exynos_adc_get_version(struct platform_device *pdev)
> return (unsigned int)match->data;
> }
>
> +static void exynos_adc_hw_init(struct exynos_adc *info)
> +{
> + u32 con1, con2;
> +
> + if (info->version == ADC_V2) {
> + con1 = ADC_V2_CON1_SOFT_RESET;
> + writel(con1, ADC_V2_CON1(info->regs));
> +
> + con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
> + ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
> + writel(con2, ADC_V2_CON2(info->regs));
> +
> + /* Enable interrupts */
> + writel(1, ADC_V2_INT_EN(info->regs));
> + } else {
> + /* set default prescaler values and Enable prescaler */
> + con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
> +
> + /* Enable 12-bit ADC resolution */
> + con1 |= ADC_V1_CON_RES;
> + writel(con1, ADC_V1_CON(info->regs));
> + }
> +}
> +
> static int exynos_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val,
> @@ -149,6 +173,8 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> timeout = wait_for_completion_timeout
> (&info->completion, EXYNOS_ADC_TIMEOUT);
> if (timeout == 0) {
> + dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
> + exynos_adc_hw_init(info);
> ret = -ETIMEDOUT;
> } else {
> *val = info->value;
> @@ -230,30 +256,6 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
> return 0;
> }
>
> -static void exynos_adc_hw_init(struct exynos_adc *info)
> -{
> - u32 con1, con2;
> -
> - if (info->version == ADC_V2) {
> - con1 = ADC_V2_CON1_SOFT_RESET;
> - writel(con1, ADC_V2_CON1(info->regs));
> -
> - con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
> - ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
> - writel(con2, ADC_V2_CON2(info->regs));
> -
> - /* Enable interrupts */
> - writel(1, ADC_V2_INT_EN(info->regs));
> - } else {
> - /* set default prescaler values and Enable prescaler */
> - con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
> -
> - /* Enable 12-bit ADC resolution */
> - con1 |= ADC_V1_CON_RES;
> - writel(con1, ADC_V1_CON(info->regs));
> - }
> -}
> -
> static int exynos_adc_probe(struct platform_device *pdev)
> {
> struct exynos_adc *info = NULL;
>
On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
> Add reinit_completion() before the wait_for_completion_timeout in
> raw_read() call.
>
> Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
> Reviewed-by: Doug Anderson <[email protected]>
Applied to the togreg branch of iio.git
I wasn't sure if this one was technically a fix, but as it isn't
marked clearly as such it can go in during the next merge window.
I won't have pushed this out to a non rebasing branch until tomorrow
so if this fixes an observed issue then let me know and it might move
to the fixes-togreg branch.
Thanks,
Jonathan
> ---
> Changes since v2:
> None
> Changes since v1:
> None
> v0:
> This change is a part of the patch reviewed at https://lkml.org/lkml/2013/11/5/92
>
> drivers/iio/adc/exynos_adc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index eddc58e..010578f 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -151,6 +151,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> return -EINVAL;
>
> mutex_lock(&indio_dev->mlock);
> + reinit_completion(&info->completion);
>
> /* Select the channel to be used and Trigger conversion */
> if (info->version == ADC_V2) {
>
Jonathan,
On Wed, Apr 30, 2014 at 1:49 PM, Jonathan Cameron <[email protected]> wrote:
> On 30/04/14 10:26, Naveen Krishna Chatradhi wrote:
>>
>> Add reinit_completion() before the wait_for_completion_timeout in
>> raw_read() call.
>>
>> Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
>> Reviewed-by: Doug Anderson <[email protected]>
>
> Applied to the togreg branch of iio.git
> I wasn't sure if this one was technically a fix, but as it isn't
> marked clearly as such it can go in during the next merge window.
> I won't have pushed this out to a non rebasing branch until tomorrow
> so if this fixes an observed issue then let me know and it might move
> to the fixes-togreg branch.
As far as I know this doesn't fix an observed issue. I believe it was
requested as part of a previous review and is finally reposted. I
believe it's just extra precautions to make sure that some old IRQ
didn't accidentally complete us between the last transaction and this
one.
Old discussion is somewhere around here
<https://patchwork.kernel.org/patch/2279591/>
Thanks!
-Doug