From: Fabrizio Lamarque <[email protected]>
Here is a patch set to overcome a number of issues in current ad7192
adc driver implementation prevending the driver to be loaded and
behave correctly:
- null pointer dereference causing kernel panic on driver probe;
- wrong internal clock selection;
- use of "avdd" regulator name in place of vref (reference voltage);
- missing clock options in bindings docs only (already implemented).
The first two issues are regressions.
Backported patches have been tested on a platform with an ARM Cortex-A7
CPU from NXP with kernel 5.15.
As a side note, on the tested platform there is still an issue with a
pending interrupt that I worked around by setting the DRDY IRQ
to LEVEL in place of EDGE sensing. Also, setting IRQ_DISABLE_UNLAZY
flag does not help in my case.
You may find further information here:
https://lore.kernel.org/all/CAPJMGm4GaSjD6bdqMwCr2EVZGenWzT-nCCf3BMRaD1TSfAabpA@mail.gmail.com/
Version log:
v2->v3
- Reworded commit messages
- Split binding fixes
v1->v2
- Obtained ad7192_state from iio_dev pointer
- Added patch on bindings documentation
Fabrizio Lamarque (5):
iio: adc: ad7192: Fix null ad7192_state pointer access
iio: adc: ad7192: Fix internal/external clock selection
iio: adc: ad7192: Use VRef instead of AVdd as reference voltage source
dt-bindings: iio: ad7192: Add mandatory reference voltage source
dt-bindings: iio: ad7192: Allow selection of clock modes
.../bindings/iio/adc/adi,ad7192.yaml | 32 +++++++++++++++++--
drivers/iio/adc/ad7192.c | 31 +++++++++++++++---
2 files changed, 55 insertions(+), 8 deletions(-)
--
2.34.1
From: Fabrizio Lamarque <[email protected]>
Pointer to indio_dev structure is obtained via spi_get_drvdata() at
the beginning of function ad7192_setup(), but the spi->dev->driver_data
member is not initialized, hence a NULL pointer is returned.
Fix by changing ad7192_setup() signature to take pointer to struct
iio_dev, and get ad7192_state pointer via st = iio_priv(indio_dev);
Fixes: bd5dcdeb3fd0 ("iio: adc: ad7192: convert to device-managed functions")
Signed-off-by: Fabrizio Lamarque <[email protected]>
Reviewed-by: Nuno Sa <[email protected]>
---
drivers/iio/adc/ad7192.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 55a6ab591016..94a9cf34a255 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -380,9 +380,9 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
return clock_sel;
}
-static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
+static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
{
- struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
+ struct ad7192_state *st = iio_priv(indio_dev);
bool rej60_en, refin2_en;
bool buf_en, bipolar, burnout_curr_en;
unsigned long long scale_uv;
@@ -1073,7 +1073,7 @@ static int ad7192_probe(struct spi_device *spi)
}
}
- ret = ad7192_setup(st, spi->dev.of_node);
+ ret = ad7192_setup(indio_dev, spi->dev.of_node);
if (ret)
return ret;
--
2.34.1
From: Fabrizio Lamarque <[email protected]>
Fix wrong selection of internal clock when mclk is defined.
Resolve a logical inversion introduced in c9ec2cb328e3.
Fixes: c9ec2cb328e3 ("iio: adc: ad7192: use devm_clk_get_optional() for mclk")
Signed-off-by: Fabrizio Lamarque <[email protected]>
Reviewed-by: Nuno Sa <[email protected]>
---
drivers/iio/adc/ad7192.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 94a9cf34a255..5a9c8898f8af 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -367,7 +367,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
clock_sel = AD7192_CLK_INT;
/* use internal clock */
- if (st->mclk) {
+ if (!st->mclk) {
if (of_property_read_bool(np, "adi,int-clock-output-enable"))
clock_sel = AD7192_CLK_INT_CO;
} else {
--
2.34.1
On Tue, 30 May 2023 09:53:07 +0200
[email protected] wrote:
> From: Fabrizio Lamarque <[email protected]>
>
> Pointer to indio_dev structure is obtained via spi_get_drvdata() at
> the beginning of function ad7192_setup(), but the spi->dev->driver_data
> member is not initialized, hence a NULL pointer is returned.
>
> Fix by changing ad7192_setup() signature to take pointer to struct
> iio_dev, and get ad7192_state pointer via st = iio_priv(indio_dev);
>
> Fixes: bd5dcdeb3fd0 ("iio: adc: ad7192: convert to device-managed functions")
> Signed-off-by: Fabrizio Lamarque <[email protected]>
> Reviewed-by: Nuno Sa <[email protected]>
I'll pick those of this series up that everyone seems happy with.
Applied to the fixes-togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7192.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 55a6ab591016..94a9cf34a255 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -380,9 +380,9 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
> return clock_sel;
> }
>
> -static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
> +static int ad7192_setup(struct iio_dev *indio_dev, struct device_node *np)
> {
> - struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> + struct ad7192_state *st = iio_priv(indio_dev);
> bool rej60_en, refin2_en;
> bool buf_en, bipolar, burnout_curr_en;
> unsigned long long scale_uv;
> @@ -1073,7 +1073,7 @@ static int ad7192_probe(struct spi_device *spi)
> }
> }
>
> - ret = ad7192_setup(st, spi->dev.of_node);
> + ret = ad7192_setup(indio_dev, spi->dev.of_node);
> if (ret)
> return ret;
>
On Tue, 30 May 2023 09:53:08 +0200
[email protected] wrote:
> From: Fabrizio Lamarque <[email protected]>
>
> Fix wrong selection of internal clock when mclk is defined.
>
> Resolve a logical inversion introduced in c9ec2cb328e3.
>
> Fixes: c9ec2cb328e3 ("iio: adc: ad7192: use devm_clk_get_optional() for mclk")
> Signed-off-by: Fabrizio Lamarque <[email protected]>
> Reviewed-by: Nuno Sa <[email protected]>
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7192.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 94a9cf34a255..5a9c8898f8af 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -367,7 +367,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
> clock_sel = AD7192_CLK_INT;
>
> /* use internal clock */
> - if (st->mclk) {
> + if (!st->mclk) {
> if (of_property_read_bool(np, "adi,int-clock-output-enable"))
> clock_sel = AD7192_CLK_INT_CO;
> } else {