Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
Hi Tang,
The patch looks overall good, though I think it could be split into two
pieces: one for simplifying ret declaration and another for removing the
check after device register.
Despite that, I guess Lucas might already be working on similar changes.
https://lore.kernel.org/linux-iio/cover.1620766020.git.lucas.p.stan...@gmail.com/
As general advice, I would recommend avoiding using generic words such
as fix in the subject line. It's often better to say something about the
nature of what is being done.
Cc: lucas.p.stan...@gmail.com
Best regards,
Marcelo
On 05/17, Tang Bin wrote:
> In the function ad7746_probe(), the return value of
> devm_iio_device_register() can be zero or ret, thus it is
> unnecessary to repeated check here. And delete unused
> initialized value of 'ret', because it will be assigned by
> the function i2c_smbus_write_byte_data().
>
> Signed-off-by: Tang Bin
> ---
> drivers/staging/iio/cdc/ad7746.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7746.c
> b/drivers/staging/iio/cdc/ad7746.c
> index dfd71e99e..d3b6e68df 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -680,7 +680,7 @@ static int ad7746_probe(struct i2c_client *client,
> struct ad7746_chip_info *chip;
> struct iio_dev *indio_dev;
> unsigned char regval = 0;
> - int ret = 0;
> + int ret;
>
> indio_dev = devm_iio_device_alloc(>dev, sizeof(*chip));
> if (!indio_dev)
> @@ -730,11 +730,7 @@ static int ad7746_probe(struct i2c_client *client,
> if (ret < 0)
> return ret;
>
> - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> - if (ret)
> - return ret;
> -
> - return 0;
> + return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> }
>
> static const struct i2c_device_id ad7746_id[] = {
> --
> 2.20.1.windows.1
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] dt-bindings: iio: accel: add binding documentation for ADIS16240
Hi,
Could anyone with some experience in devicetree give us a hint on what
to do here?
I have built a binding doc using the same spi-cpha property.
https://github.com/analogdevicesinc/linux/commit/bb2945e489dfdf2faa0255dd2cf09ae4ee77d826
On 09/13, Ardelean, Alexandru wrote:
> On Thu, 2019-09-12 at 18:39 -0300, Rodrigo Carvalho wrote:
> > This patch add device tree binding documentation for ADIS16240.
> >
> > Signed-off-by: Rodrigo Ribeiro Carvalho
> > ---
> > V2:
> > - Remove true constant for spi-cpha and spi-cpol
> > - Add description field for spi-cpha and spi-cpol
> > - Add maxItems field for spi-cpha and spi-cpol
> >
> > .../bindings/iio/accel/adi,adis16240.yaml | 61 +++
> > 1 file changed, 61 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > new file mode 100644
> > index ..4b1bd2419604
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ADIS16240 Programmable Impact Sensor and Recorder driver
> > +
> > +maintainers:
> > + - Alexandru Ardelean
> > +
> > +description: |
> > + ADIS16240 Programmable Impact Sensor and Recorder driver that supports
> > + SPI interface.
> > +https://www.analog.com/en/products/adis16240.html
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - adi,adis16240
> > +
> > + reg:
> > +maxItems: 1
> > +
> > + spi-cpha:
> > +description: |
> > + See Documentation/devicetree/bindings/spi/spi-controller.yaml
> > +maxItems: 1
>
> Description for standard properties is not required.
>
> For spi-cpha/cpol just "true" seems sufficient.
>
> So
>
> spi-cpha: true
>
> spi-cpol: true
>
I'm not Rob, but I think it is not necessary to explicitly say the
property is true. In this case, it should be enough if it is present. If
needed to know whether some property is "true" or not, one can use the
of_property_read_bool function. For the AD7292 driver on it was enough
to just add the property names. The spi-chpa did not need any further
care. Without the spi-cpha property, the AD7292 vendor ID came as 0x0C
(one bit shifted to the right).
Rodrigo is participating at FLUSP students group. It would be good if we
could test whether these properties are really needed. However, I don't
think we have the ADIS16240 part. Would anyone test it?
> > +
> > + spi-cpol: |
> > +description: |
> > + See Documentation/devicetree/bindings/spi/spi-controller.yaml
> > +maxItems: 1
> > +
> > + interrupts:
> > +maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > +
>
> If spi-cpha & spi-cpol are true, they should typically be also required.
> Though, I think Rob would answer things better here.
>
Some feedback about the need (or not) to link to the spi-controller doc
would also be appreciated.
> > +examples:
> > + - |
> > +#include
> > +#include
> > +spi0 {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +/* Example for a SPI device node */
> > +accelerometer@0 {
> > +compatible = "adi,adis16240";
> > +reg = <0>;
> > +spi-max-frequency = <250>;
> > +spi-cpol;
> > +spi-cpha;
> > +interrupt-parent = <>;
> > +interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +};
> > +};
Thanks,
Marcelo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adis16240: add of_match_table entry
Hi Alexandru,
On 05/24, Alexandru Ardelean wrote:
> On Fri, May 24, 2019 at 6:30 AM Rodrigo Ribeiro wrote:
> >
> > This patch adds of_match_table entry in device driver in order to
> > enable spi fallback probing.
> >
> > Signed-off-by: Rodrigo Ribeiro
> > Reviewed-by: Marcelo Schmitt
> > ---
> > drivers/staging/iio/accel/adis16240.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/iio/accel/adis16240.c
> > b/drivers/staging/iio/accel/adis16240.c
> > index 8c6d23604eca..b80c8529784b 100644
> > --- a/drivers/staging/iio/accel/adis16240.c
> > +++ b/drivers/staging/iio/accel/adis16240.c
> > @@ -444,6 +444,7 @@ MODULE_DEVICE_TABLE(of, adis16240_of_match);
> > static struct spi_driver adis16240_driver = {
> > .driver = {
> > .name = "adis16240",
> > + .of_match_table = adis16240_of_match,
>
> This patch is missing the actual table.
Struct with compatible devices table was included separately in a
previous patch at commit d9e533b6c0a26c7ef8116b7f3477c164c07bb6fb.
Yeah, I also thought it was missing the match table the first time I was
this patch. It's really confusing when we have two patches, one
depending on another, that are not part of the same patch set. We're
trying to avoid things like this the most but that slipped out from our
internal review. We're sorry about that.
>
> > },
> > .probe = adis16240_probe,
> > .remove = adis16240_remove,
> > --
> > 2.20.1
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: iio: ad9832: add devicetree documentation
On 04/02, Alexandru Ardelean wrote:
> On Mon, Apr 1, 2019 at 5:38 PM Marcelo Schmitt
> wrote:
> >
> > Add a devicetree documentation for the ad9832 direct digital
> > synthesizer, waveform generator.
> >
> > Signed-off-by: Marcelo Schmitt
> > ---
> > .../bindings/iio/frequency/ad9832.txt | 26 +++
> > 1 file changed, 26 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> > b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> > new file mode 100644
> > index ..6a35fdff5a48
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> > @@ -0,0 +1,26 @@
> > +Analog Devices AD9832 Direct Digital Synthesizer, Waveform Generator
> > +
> > +Data sheet:
> > +https://www.analog.com/media/en/technical-documentation/data-sheets/AD9832.pdf
> > +
> > +Required properties:
> > + - compatible : Must be "adi,ad9832"
> > + - reg : SPI chip select number for the device
> > + - spi-max-frequency = Max SPI frequency to use (< 2500)
> > + - clocks : The clock reference for the DDS output
> > + - clock-names : Must be "mclk"
>
> It's always a good idea to reference other base dt docs.
> For SPI you could:
>
> ```
> For more information on SPI properties, please consult
> Documentation/devicetree/bindings/spi/spi-bus.txt
> ```
>
> For clock:
> ```
> For more information on clock bindings properties, please consult
> Documentation/devicetree/bindings/clock/clock-bindings.txt
> ```
>
> For regulator:
> ```
> For more information on regulator bindings properties, please consult
> Documentation/devicetree/bindings/regulator/regulator.txt
> ```
Thanks for the advice. I'll have a look at them.
>
> > +
> > +Optional properties:
> > + - avdd-supply: Definition of the regulator used as analog supply
> > + - dvdd-supply : Definition of the regulator used as digital supply
> > +
> > +Example:
> > + adi9832-dds@0 {
> > + compatible = "adi,ad9832";
> > + reg = <0>;
> > + spi-max-frequency = <2500>;
> > + clocks = <_mclk>;
> > + clock-names = "mclk";
> > + avdd-suppy = <>;
> > + dvdd-suppy = <>;
> > + };
> > --
> > 2.20.1
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Work on iio: stating: frequency: ad9832
On 04/02, Alexandru Ardelean wrote: > On Mon, Apr 1, 2019 at 7:13 PM Jonathan Cameron > wrote: > > > > On Mon, 1 Apr 2019 11:25:29 -0300 > > Marcelo Schmitt wrote: > > > > > Hello, > > > > > > I was looking for some work on staging: iio: ad9832 and made some > > > observations while reading the driver. > > > > > > Apparently it had no devicetree documentation so I tried to elaborate > > > one. > > > It uses a platform_data variable to load external clock > > > frequency (I tried to make it use linux's clock framework). > > Good. > > > > > Some device attributes don't seem to be standardized on > > > Documentation/ABI/testing/sysfs-bus-iio and there's no specific ABI > > > for ad9832 nearby nor at staging/iio/Documentation. So maybe those > > > missing ABI could be documented. > > Beware. It's an old driver, so it may be that we actually want to change > > it's ABI rather than documenting what is there (I have haven't looked!) > > OK, I'll take more time studying the device's datasheet to better understand the current ABI. > > This one can actually be coupled a bit with the AD9834 driver. > There's been some work on trying to move that one out of staging as well. > > You can take a look at the patches sent for that driver. > They should be find-able on patchwork > https://patchwork.kernel.org/project/linux-iio/list/?series===*=ad9834=both= > > There are ideas worth borrowing from there. > > The issue with the AD9834 [if i recall correctly] is that it doesn't > quite fit the classical IIO channel model. > Meaning, you can only activate the output of one channel at one moment > in time, and not both. OK, I'll have a look at it. > > > > The device has to set some internal registers to operate correctly, > > > AD9832_FREQXHM and AD9832_PHASEXH, would it be feasible to set iio > > > chanels for this? > > > > What are they? If they correspond to output channels in some sensible > > way then maybe... > > > > > I couldn't understand why checkpatch.pl gave errors on IIO_DEV_ATTR_* > > > macros. To me they seem to have no problem. > > > Also it has that platform_data to be moved to include/linux/iio. Is > > > there any special reason for it not being there already? Which are > > > the criterions a platform_data need to satisfy to be put there? > > A driver moving out of staging shouldn't have platform data. It needs > > to be converted over to more modern mechanisms. We don't have a problem > > supporting platform data for devices that have old school device files > > already in tree, but that shouldn't be the case for a driver in staging. > > > > Hence we can clean it up and move forward with just DT bindings. > > > Understood. Thanks for the explanation. > > > I'm sending a patchset with some things I've already done. > > Cool. I'll look at those later in the week if no one beats me to them. > > > > > > > > Is there something else that could be done in this device driver? > > > Please, tell if I've forgotten something. > > > > I'll take a look, but it may be a little while before I do. > > Hopefully someone else gets there first! > > > > Jonathan > > > > > > > > Any advice is welcome. > > > Thanks, > > > > > > Marcelo > > > > Thanks for the pieces of advice. Marcelo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: iio: ad9832: add SPDX identifier
On 04/01, Greg KH wrote: > On Mon, Apr 01, 2019 at 08:36:50PM +0530, Mukesh Ojha wrote: > > > > On 4/1/2019 8:07 PM, Marcelo Schmitt wrote: > > > Add SPDX identifier of GPL-2.0 for the ad9832 driver. > > > > > > Signed-off-by: Marcelo Schmitt > > > --- > > > drivers/staging/iio/frequency/ad9832.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/iio/frequency/ad9832.c > > > b/drivers/staging/iio/frequency/ad9832.c > > > index 50a583020072..d8d4a7936275 100644 > > > --- a/drivers/staging/iio/frequency/ad9832.c > > > +++ b/drivers/staging/iio/frequency/ad9832.c > > > @@ -1,9 +1,8 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > > FYI > > > > Use C notations here as per https://lkml.org/lkml/2019/2/13/570. > > > > <https://lkml.org/lkml/2019/2/13/570> OK, I'll check it out. > > > > Once you fix it then you can take mine > > Reviewed-by: Mukesh Ojha > > Did you read the documentation about how to put a proper SPDX line in > the kernel documentation? For .c files, you have to use "//", not "/* */" > > Please read: > Documentation/process/license-rules.rst > and see the section "Style" for all of the details. Thanks, I'll read it to understand the details. > > thanks, > > greg k-h Thanks for the advises, Marcelo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: iio: ad9832: use clock framework for clock reference
Previously external clock were set through platform_data struct.
Now device uses clk struct defined in include/linux/clk.h to handle
external clock source.
It also removes mclk from platform_data struct.
Signed-off-by: Marcelo Schmitt
---
drivers/staging/iio/frequency/ad9832.c | 37 +-
drivers/staging/iio/frequency/ad9832.h | 1 -
2 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9832.c
b/drivers/staging/iio/frequency/ad9832.c
index d8d4a7936275..74308a2e72db 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -7,6 +7,7 @@
#include
+#include
#include
#include
#include
@@ -95,7 +96,7 @@ struct ad9832_state {
struct spi_device *spi;
struct regulator*avdd;
struct regulator*dvdd;
- unsigned long mclk;
+ struct clk *mclk;
unsigned short ctrl_fp;
unsigned short ctrl_ss;
unsigned short ctrl_src;
@@ -130,10 +131,10 @@ static int ad9832_write_frequency(struct ad9832_state *st,
{
unsigned long regval;
- if (fout > (st->mclk / 2))
+ if (fout > (clk_get_rate(st->mclk) / 2))
return -EINVAL;
- regval = ad9832_calc_freqreg(st->mclk, fout);
+ regval = ad9832_calc_freqreg(clk_get_rate(st->mclk), fout);
st->freq_data[0] = cpu_to_be16((AD9832_CMD_FRE8BITSW << CMD_SHIFT) |
(addr << ADD_SHIFT) |
@@ -334,7 +335,16 @@ static int ad9832_probe(struct spi_device *spi)
goto error_disable_avdd;
}
- st->mclk = pdata->mclk;
+ st->mclk = devm_clk_get(>dev, "mclk");
+ if (IS_ERR(st->mclk)) {
+ ret = PTR_ERR(st->mclk);
+ goto error_disable_dvdd;
+ }
+
+ ret = clk_prepare_enable(st->mclk);
+ if (ret < 0)
+ goto error_disable_dvdd;
+
st->spi = spi;
mutex_init(>lock);
@@ -385,39 +395,41 @@ static int ad9832_probe(struct spi_device *spi)
ret = spi_sync(st->spi, >msg);
if (ret) {
dev_err(>dev, "device init failed\n");
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
}
ret = ad9832_write_frequency(st, AD9832_FREQ0HM, pdata->freq0);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = ad9832_write_frequency(st, AD9832_FREQ1HM, pdata->freq1);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = ad9832_write_phase(st, AD9832_PHASE0H, pdata->phase0);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = ad9832_write_phase(st, AD9832_PHASE1H, pdata->phase1);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = ad9832_write_phase(st, AD9832_PHASE2H, pdata->phase2);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = ad9832_write_phase(st, AD9832_PHASE3H, pdata->phase3);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
ret = iio_device_register(indio_dev);
if (ret)
- goto error_disable_dvdd;
+ goto error_unprepare_mclk;
return 0;
+error_unprepare_mclk:
+ clk_disable_unprepare(st->mclk);
error_disable_dvdd:
regulator_disable(st->dvdd);
error_disable_avdd:
@@ -432,6 +444,7 @@ static int ad9832_remove(struct spi_device *spi)
struct ad9832_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
+ clk_disable_unprepare(st->mclk);
regulator_disable(st->dvdd);
regulator_disable(st->avdd);
diff --git a/drivers/staging/iio/frequency/ad9832.h
b/drivers/staging/iio/frequency/ad9832.h
index 39d326cc1af9..032579a2d539 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -24,7 +24,6 @@
*/
struct ad9832_platform_data {
- unsigned long mclk;
unsigned long freq0;
unsigned long freq1;
unsigned short phase0;
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: iio: ad9832: add devicetree documentation
Add a devicetree documentation for the ad9832 direct digital
synthesizer, waveform generator.
Signed-off-by: Marcelo Schmitt
---
.../bindings/iio/frequency/ad9832.txt | 26 +++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/frequency/ad9832.txt
diff --git a/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
new file mode 100644
index ..6a35fdff5a48
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
@@ -0,0 +1,26 @@
+Analog Devices AD9832 Direct Digital Synthesizer, Waveform Generator
+
+Data sheet:
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD9832.pdf
+
+Required properties:
+ - compatible : Must be "adi,ad9832"
+ - reg : SPI chip select number for the device
+ - spi-max-frequency = Max SPI frequency to use (< 2500)
+ - clocks : The clock reference for the DDS output
+ - clock-names : Must be "mclk"
+
+Optional properties:
+ - avdd-supply: Definition of the regulator used as analog supply
+ - dvdd-supply : Definition of the regulator used as digital supply
+
+Example:
+ adi9832-dds@0 {
+ compatible = "adi,ad9832";
+ reg = <0>;
+ spi-max-frequency = <2500>;
+ clocks = <_mclk>;
+ clock-names = "mclk";
+ avdd-suppy = <>;
+ dvdd-suppy = <>;
+ };
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: iio: ad9832: add SPDX identifier
Add SPDX identifier of GPL-2.0 for the ad9832 driver. Signed-off-by: Marcelo Schmitt --- drivers/staging/iio/frequency/ad9832.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c index 50a583020072..d8d4a7936275 100644 --- a/drivers/staging/iio/frequency/ad9832.c +++ b/drivers/staging/iio/frequency/ad9832.c @@ -1,9 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* * AD9832 SPI DDS driver * * Copyright 2011 Analog Devices Inc. - * - * Licensed under the GPL-2. */ #include -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: iio: ad9832: organize includes
Organize includes to list them in lexicographic order. Signed-off-by: Marcelo Schmitt --- drivers/staging/iio/frequency/ad9832.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c index a3ce50427724..50a583020072 100644 --- a/drivers/staging/iio/frequency/ad9832.c +++ b/drivers/staging/iio/frequency/ad9832.c @@ -6,22 +6,24 @@ * Licensed under the GPL-2. */ +#include + #include +#include #include +#include +#include #include -#include #include -#include -#include -#include -#include +#include #include #include -#include "dds.h" #include "ad9832.h" +#include "dds.h" + /* Registers */ #define AD9832_FREQ0LL 0x0 -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging: iio: ad9832: use clock framework
This series of patches do minor codestyle ajusts, set SPDX licence identifier, set use of linux's clock framework to handle external clock, and add a devicetree documentation. Marcelo Schmitt (4): staging: iio: ad9832: organize includes staging: iio: ad9832: add SPDX identifier staging: iio: ad9832: use clock framework for clock reference staging: iio: ad9832: add devicetree documentation .../bindings/iio/frequency/ad9832.txt | 26 + drivers/staging/iio/frequency/ad9832.c| 54 --- drivers/staging/iio/frequency/ad9832.h| 1 - 3 files changed, 60 insertions(+), 21 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/frequency/ad9832.txt -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Work on iio: stating: frequency: ad9832
Hello, I was looking for some work on staging: iio: ad9832 and made some observations while reading the driver. Apparently it had no devicetree documentation so I tried to elaborate one. It uses a platform_data variable to load external clock frequency (I tried to make it use linux's clock framework). Some device attributes don't seem to be standardized on Documentation/ABI/testing/sysfs-bus-iio and there's no specific ABI for ad9832 nearby nor at staging/iio/Documentation. So maybe those missing ABI could be documented. The device has to set some internal registers to operate correctly, AD9832_FREQXHM and AD9832_PHASEXH, would it be feasible to set iio chanels for this? I couldn't understand why checkpatch.pl gave errors on IIO_DEV_ATTR_* macros. To me they seem to have no problem. Also it has that platform_data to be moved to include/linux/iio. Is there any special reason for it not being there already? Which are the criterions a platform_data need to satisfy to be put there? I'm sending a patchset with some things I've already done. Is there something else that could be done in this device driver? Please, tell if I've forgotten something. Any advice is welcome. Thanks, Marcelo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Help on testing ad5933 driver
Hello, would anyone mind helping me test ad5933 driver on actual hardware? I went through this (https://oslongjourney.github.io/linux-kernel/experiment-one-iio-dummy/) tutorial so I was able to load iio_simple_dummy driver, create and inspect some dummy devices. Now, as Jonathan has asked me, I would like to test ad5933 driver on an EVAL-AD5933 board which was donated to FLUSP (https://flusp.ime.usp.br/). So far I've been hesitating to plug this device on my Debian distro since this (https://www.analog.com/media/en/technical-documentation/user-guides/UG-364.pdf) user guide for Windows says not to connect it before driver installation. Is there something that could harm the board if plugged on a computer without a proper driver? I also didn't understand the hardware configuration showed on this (https://wiki.analog.com/resources/tools-software/linux-drivers/iio-impedance-analyzer/ad5933) page. Any advice will be greatly appreciated. Thanks in advance, Marcelo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 2/2] staging: iio: ad5933: move out of staging
Move ad5933 impedance-analyzer driver from staging to mainline. The ad5933 is a high precision impedance converter system solution that combines an on-board frequency generator with an analog-to-digital converter (ADC). This driver was designed to be compatible with both ad5933 and ad5934 chips. Signed-off-by: Marcelo Schmitt --- drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/impedance-analyzer/Kconfig| 18 + drivers/iio/impedance-analyzer/Makefile | 5 + drivers/iio/impedance-analyzer/ad5933.c | 811 ++ drivers/staging/iio/Kconfig | 1 - drivers/staging/iio/Makefile | 1 - .../staging/iio/impedance-analyzer/Kconfig| 18 - .../staging/iio/impedance-analyzer/Makefile | 5 - .../staging/iio/impedance-analyzer/ad5933.c | 811 -- 10 files changed, 836 insertions(+), 836 deletions(-) create mode 100644 drivers/iio/impedance-analyzer/Kconfig create mode 100644 drivers/iio/impedance-analyzer/Makefile create mode 100644 drivers/iio/impedance-analyzer/ad5933.c delete mode 100644 drivers/staging/iio/impedance-analyzer/Kconfig delete mode 100644 drivers/staging/iio/impedance-analyzer/Makefile delete mode 100644 drivers/staging/iio/impedance-analyzer/ad5933.c diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig index 014006d1cbb6..db38e6fb62fe 100644 --- a/drivers/iio/Kconfig +++ b/drivers/iio/Kconfig @@ -81,6 +81,7 @@ source "drivers/iio/frequency/Kconfig" source "drivers/iio/gyro/Kconfig" source "drivers/iio/health/Kconfig" source "drivers/iio/humidity/Kconfig" +source "drivers/iio/impedance-analyzer/Kconfig" source "drivers/iio/imu/Kconfig" source "drivers/iio/light/Kconfig" source "drivers/iio/magnetometer/Kconfig" diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile index cb5993251381..fae55479356b 100644 --- a/drivers/iio/Makefile +++ b/drivers/iio/Makefile @@ -27,6 +27,7 @@ obj-y += gyro/ obj-y += frequency/ obj-y += health/ obj-y += humidity/ +obj-y += impedance-analyzer/ obj-y += imu/ obj-y += light/ obj-y += magnetometer/ diff --git a/drivers/iio/impedance-analyzer/Kconfig b/drivers/iio/impedance-analyzer/Kconfig new file mode 100644 index ..b9a679cdd146 --- /dev/null +++ b/drivers/iio/impedance-analyzer/Kconfig @@ -0,0 +1,18 @@ +# +# Impedance Converter, Network Analyzer drivers +# +menu "Network Analyzer, Impedance Converters" + +config AD5933 + tristate "Analog Devices AD5933, AD5934 driver" + depends on I2C + select IIO_BUFFER + select IIO_KFIFO_BUF + help + Say yes here to build support for Analog Devices Impedance Converter, + Network Analyzer, AD5933/4. + + To compile this driver as a module, choose M here: the + module will be called ad5933. + +endmenu diff --git a/drivers/iio/impedance-analyzer/Makefile b/drivers/iio/impedance-analyzer/Makefile new file mode 100644 index ..7604d786583e --- /dev/null +++ b/drivers/iio/impedance-analyzer/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for Impedance Converter, Network Analyzer drivers +# + +obj-$(CONFIG_AD5933) += ad5933.o diff --git a/drivers/iio/impedance-analyzer/ad5933.c b/drivers/iio/impedance-analyzer/ad5933.c new file mode 100644 index ..2b0f8f899e3f --- /dev/null +++ b/drivers/iio/impedance-analyzer/ad5933.c @@ -0,0 +1,811 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * AD5933 AD5934 Impedance Converter, Network Analyzer + * + * Copyright 2011 Analog Devices Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +/* AD5933/AD5934 Registers */ +#define AD5933_REG_CONTROL_HB 0x80/* R/W, 1 byte */ +#define AD5933_REG_CONTROL_LB 0x81/* R/W, 1 byte */ +#define AD5933_REG_FREQ_START 0x82/* R/W, 3 bytes */ +#define AD5933_REG_FREQ_INC0x85/* R/W, 3 bytes */ +#define AD5933_REG_INC_NUM 0x88/* R/W, 2 bytes, 9 bit */ +#define AD5933_REG_SETTLING_CYCLES 0x8A/* R/W, 2 bytes */ +#define AD5933_REG_STATUS 0x8F/* R, 1 byte */ +#define AD5933_REG_TEMP_DATA 0x92/* R, 2 bytes*/ +#define AD5933_REG_REAL_DATA 0x94/* R, 2 bytes*/ +#define AD5933_REG_IMAG_DATA 0x96/* R, 2 bytes*/ + +/* AD5933_REG_CONTROL_HB Bits */ +#define AD5933_CTRL_INIT_START_FREQ(0x1 << 4) +#define AD5933_CTRL_START_SWEEP(0x2 << 4) +#define AD5933_CTRL_INC_FREQ (0x3 << 4) +#define AD5933_CTRL_REPEAT_FREQ(0x4 << 4) +#define AD5933_CTRL_MEASURE_TEMP (0x9 << 4) +#define AD5933_CTRL_POWER_DOWN (0xA << 4) +#define AD5933_CTRL_STANDB
[PATCH v5 1/2] staging: iio: ad5933: change attributes to match ABI
Change device attributes' names to match ABI documentation. Names were
chosen such that they tend to be similar to existing ABI so it should
be easier to standardize them when necessary.
Signed-off-by: Marcelo Schmitt
---
.../staging/iio/impedance-analyzer/ad5933.c | 24 +--
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index d75bdfbf93de..2b0f8f899e3f 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -315,12 +315,12 @@ static ssize_t ad5933_store_frequency(struct device *dev,
return ret ? ret : len;
}
-static IIO_DEVICE_ATTR(out_voltage0_freq_start, 0644,
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency_start, 0644,
ad5933_show_frequency,
ad5933_store_frequency,
AD5933_REG_FREQ_START);
-static IIO_DEVICE_ATTR(out_voltage0_freq_increment, 0644,
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency_increment, 0644,
ad5933_show_frequency,
ad5933_store_frequency,
AD5933_REG_FREQ_INC);
@@ -443,12 +443,12 @@ static ssize_t ad5933_store(struct device *dev,
return ret ? ret : len;
}
-static IIO_DEVICE_ATTR(out_voltage0_scale, 0644,
+static IIO_DEVICE_ATTR(out_altvoltage0_raw, 0644,
ad5933_show,
ad5933_store,
AD5933_OUT_RANGE);
-static IIO_DEVICE_ATTR(out_voltage0_scale_available, 0444,
+static IIO_DEVICE_ATTR(out_altvoltage0_scale_available, 0444,
ad5933_show,
NULL,
AD5933_OUT_RANGE_AVAIL);
@@ -463,12 +463,12 @@ static IIO_DEVICE_ATTR(in_voltage0_scale_available, 0444,
NULL,
AD5933_IN_PGA_GAIN_AVAIL);
-static IIO_DEVICE_ATTR(out_voltage0_freq_points, 0644,
+static IIO_DEVICE_ATTR(out_altvoltage0_frequency_points, 0644,
ad5933_show,
ad5933_store,
AD5933_FREQ_POINTS);
-static IIO_DEVICE_ATTR(out_voltage0_settling_cycles, 0644,
+static IIO_DEVICE_ATTR(out_altvoltage0_settling_cycles, 0644,
ad5933_show,
ad5933_store,
AD5933_OUT_SETTLING_CYCLES);
@@ -480,12 +480,12 @@ static IIO_DEVICE_ATTR(out_voltage0_settling_cycles, 0644,
* don't create dedicated sysfs channel attributes for out0 and in0.
*/
static struct attribute *ad5933_attributes[] = {
- _dev_attr_out_voltage0_scale.dev_attr.attr,
- _dev_attr_out_voltage0_scale_available.dev_attr.attr,
- _dev_attr_out_voltage0_freq_start.dev_attr.attr,
- _dev_attr_out_voltage0_freq_increment.dev_attr.attr,
- _dev_attr_out_voltage0_freq_points.dev_attr.attr,
- _dev_attr_out_voltage0_settling_cycles.dev_attr.attr,
+ _dev_attr_out_altvoltage0_raw.dev_attr.attr,
+ _dev_attr_out_altvoltage0_scale_available.dev_attr.attr,
+ _dev_attr_out_altvoltage0_frequency_start.dev_attr.attr,
+ _dev_attr_out_altvoltage0_frequency_increment.dev_attr.attr,
+ _dev_attr_out_altvoltage0_frequency_points.dev_attr.attr,
+ _dev_attr_out_altvoltage0_settling_cycles.dev_attr.attr,
_dev_attr_in_voltage0_scale.dev_attr.attr,
_dev_attr_in_voltage0_scale_available.dev_attr.attr,
NULL
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/2] staging: iio: ad5933: move out of staging
This series of patches makes device attributes' names equal to ABI documentation and move ad5933 driver out of staging. More precisely: It changes device attributes' names to match or be similar to existing ABI. It moves the ad5933 driver from staging directory to iio main drivers directory. Marcelo Schmitt (2): staging: iio: ad5933: change attributes to match ABI staging: iio: ad5933: move out of staging drivers/iio/Kconfig | 1 + drivers/iio/Makefile | 1 + drivers/iio/impedance-analyzer/Kconfig| 18 + drivers/iio/impedance-analyzer/Makefile | 5 + drivers/iio/impedance-analyzer/ad5933.c | 811 ++ drivers/staging/iio/Kconfig | 1 - drivers/staging/iio/Makefile | 1 - .../staging/iio/impedance-analyzer/Kconfig| 18 - .../staging/iio/impedance-analyzer/Makefile | 5 - .../staging/iio/impedance-analyzer/ad5933.c | 811 -- 10 files changed, 836 insertions(+), 836 deletions(-) create mode 100644 drivers/iio/impedance-analyzer/Kconfig create mode 100644 drivers/iio/impedance-analyzer/Makefile create mode 100644 drivers/iio/impedance-analyzer/ad5933.c delete mode 100644 drivers/staging/iio/impedance-analyzer/Kconfig delete mode 100644 drivers/staging/iio/impedance-analyzer/Makefile delete mode 100644 drivers/staging/iio/impedance-analyzer/ad5933.c -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Add devicetree support for ad5933
Parts of this work came from contributions of Alexandru Ardelean and Dragos Bogdan, I and Gabriel would like to thank for the insights provided by their previous patches. Maybe it would be the case to add them as co-authors of this patch set. We also wanted to thank Jhonatan Cameron for giving us the pieces of advice needed for this work. Thanks, Marcelo Em sáb, 8 de dez de 2018 às 16:18, Marcelo Schmitt escreveu: > > This series of patches change voltage regulator error handling for the > ad5933. > It also add an option to specify external clock reference using a clock > framework and remove the old platform data structure. > Finally it adds binding documentation for devicetree. > > Marcelo Schmitt (3): > staging: iio: ad5933: change regulator binging for vref > staging: iio: ad5933: use clock framework for clock reference > staging: iio: ad5933: add binding doc for ad5933 > > .../iio/impedance-analyzer/ad5933.txt | 26 + > .../staging/iio/impedance-analyzer/ad5933.c | 57 +-- > 2 files changed, 54 insertions(+), 29 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt > > -- > 2.17.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: iio: ad5933: add binding doc for ad5933
Add a devicetree documentation for the ad5933 and ad5934 impedance
converter, network analyzer.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
.../iio/impedance-analyzer/ad5933.txt | 26 +++
1 file changed, 26 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
diff --git
a/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
new file mode 100644
index ..5ff38728ff91
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
@@ -0,0 +1,26 @@
+Analog Devices AD5933/AD5934 Impedance Converter, Network Analyzer
+
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5934.pdf
+
+Required properties:
+ - compatible : should be one of
+ "adi,ad5933"
+ "adi,ad5934"
+ - reg : the I2C address.
+ - vdd-supply : The regulator supply for DVDD, AVDD1 and AVDD2 when they
+ are connected together.
+
+Optional properties:
+- clocks : external clock reference.
+- clock-names : must be "mclk" if clocks is set.
+
+Example for a I2C device node:
+
+ impedance-analyzer@0d {
+ compatible = "adi,adxl345";
+ reg = <0x0d>;
+ vdd-supply = <_supply>;
+ clocks = <_clk>;
+ clock-names = "mclk";
+ };
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: iio: ad5933: change regulator binging for vref
Set a single voltage regulator for all voltage references.
Remove voltage reference value from default platafrom data struct.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 9e52384f5370..730bc397a26b 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -91,7 +91,6 @@
struct ad5933_platform_data {
unsigned long ext_clk_hz;
- unsigned short vref_mv;
};
struct ad5933_state {
@@ -113,7 +112,6 @@ struct ad5933_state {
};
static struct ad5933_platform_data ad5933_default_pdata = {
- .vref_mv = 3300,
};
#define AD5933_CHANNEL(_type, _extend_name, _info_mask_separate, _address, \
@@ -691,7 +689,7 @@ static void ad5933_work(struct work_struct *work)
static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- int ret, voltage_uv = 0;
+ int ret;
struct ad5933_platform_data *pdata = dev_get_platdata(>dev);
struct ad5933_state *st;
struct iio_dev *indio_dev;
@@ -718,12 +716,12 @@ static int ad5933_probe(struct i2c_client *client,
dev_err(>dev, "Failed to enable specified VDD
supply\n");
return ret;
}
- voltage_uv = regulator_get_voltage(st->reg);
+ ret = regulator_get_voltage(st->reg);
+
+ if (ret < 0)
+ goto error_disable_reg;
- if (voltage_uv)
- st->vref_mv = voltage_uv / 1000;
- else
- st->vref_mv = pdata->vref_mv;
+ st->vref_mv = ret / 1000;
if (pdata->ext_clk_hz) {
st->mclk_hz = pdata->ext_clk_hz;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: iio: ad5933: use clock framework for clock reference
Add the option to specify the external clock (MCLK) using the clock
framework.
Also remove the old platform data structure.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
.../staging/iio/impedance-analyzer/ad5933.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 730bc397a26b..3134295f014f 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -82,20 +83,10 @@
#define AD5933_POLL_TIME_ms10
#define AD5933_INIT_EXCITATION_TIME_ms 100
-/**
- * struct ad5933_platform_data - platform specific data
- * @ext_clk_hz:the external clock frequency in Hz, if not set
- * the driver uses the internal clock (16.776 MHz)
- * @vref_mv: the external reference voltage in millivolt
- */
-
-struct ad5933_platform_data {
- unsigned long ext_clk_hz;
-};
-
struct ad5933_state {
struct i2c_client *client;
struct regulator*reg;
+ struct clk *mclk;
struct delayed_work work;
struct mutexlock; /* Protect sensor state */
unsigned long mclk_hz;
@@ -111,9 +102,6 @@ struct ad5933_state {
unsigned intpoll_time_jiffies;
};
-static struct ad5933_platform_data ad5933_default_pdata = {
-};
-
#define AD5933_CHANNEL(_type, _extend_name, _info_mask_separate, _address, \
_scan_index, _realbits) { \
.type = (_type), \
@@ -690,9 +678,9 @@ static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
int ret;
- struct ad5933_platform_data *pdata = dev_get_platdata(>dev);
struct ad5933_state *st;
struct iio_dev *indio_dev;
+ unsigned long ext_clk_hz = 0;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -704,9 +692,6 @@ static int ad5933_probe(struct i2c_client *client,
mutex_init(>lock);
- if (!pdata)
- pdata = _default_pdata;
-
st->reg = devm_regulator_get(>dev, "vdd");
if (IS_ERR(st->reg))
return PTR_ERR(st->reg);
@@ -723,8 +708,21 @@ static int ad5933_probe(struct i2c_client *client,
st->vref_mv = ret / 1000;
- if (pdata->ext_clk_hz) {
- st->mclk_hz = pdata->ext_clk_hz;
+ st->mclk = devm_clk_get(>dev, "mclk");
+ if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT) {
+ ret = PTR_ERR(st->mclk);
+ goto error_disable_reg;
+ }
+
+ if (!IS_ERR(st->mclk)) {
+ ret = clk_prepare_enable(st->mclk);
+ if (ret < 0)
+ goto error_disable_reg;
+ ext_clk_hz = clk_get_rate(st->mclk);
+ }
+
+ if (ext_clk_hz) {
+ st->mclk_hz = ext_clk_hz;
st->ctrl_lb = AD5933_CTRL_EXT_SYSCLK;
} else {
st->mclk_hz = AD5933_INT_OSC_FREQ_Hz;
@@ -744,7 +742,7 @@ static int ad5933_probe(struct i2c_client *client,
ret = ad5933_register_ring_funcs_and_init(indio_dev);
if (ret)
- goto error_disable_reg;
+ goto error_disable_mclk;
ret = ad5933_setup(st);
if (ret)
@@ -758,6 +756,8 @@ static int ad5933_probe(struct i2c_client *client,
error_unreg_ring:
iio_kfifo_free(indio_dev->buffer);
+error_disable_mclk:
+ clk_disable_unprepare(st->mclk);
error_disable_reg:
regulator_disable(st->reg);
@@ -772,6 +772,7 @@ static int ad5933_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
iio_kfifo_free(indio_dev->buffer);
regulator_disable(st->reg);
+ clk_disable_unprepare(st->mclk);
return 0;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] Add devicetree support for ad5933
This series of patches change voltage regulator error handling for the ad5933. It also add an option to specify external clock reference using a clock framework and remove the old platform data structure. Finally it adds binding documentation for devicetree. Marcelo Schmitt (3): staging: iio: ad5933: change regulator binging for vref staging: iio: ad5933: use clock framework for clock reference staging: iio: ad5933: add binding doc for ad5933 .../iio/impedance-analyzer/ad5933.txt | 26 + .../staging/iio/impedance-analyzer/ad5933.c | 57 +-- 2 files changed, 54 insertions(+), 29 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad5933: replaced kfifo by triggered_buffer
On 11/25, Jonathan Cameron wrote:
> On Thu, 22 Nov 2018 10:53:47 -0200
> Marcelo Schmitt wrote:
>
> > Previously, there was an implicit creation of a kfifo which was replaced
> > by a call to triggered_buffer_setup, which is already implemented in iio
> > infrastructure.
> >
> > Signed-off-by: Marcelo Schmitt
>
> I'm a little surprised that this would work without screaming a lot as
> it will register an interrupt with no handlers. Do you have this
> device to test? It's rapidly heading in the direction of too complex
> a driver to fix without test hardware. Also, semantically this change
> is not sensible as it implies an operating mode which the driver
> is not using.
>
Thanks for reviewing the patch. I'm not expert at electronics so I'm
still studying the datasheet to understand how the ad5933 works.
> There are fundamental questions about how we handle an autotriggered
> sweep that need answering before this driver can move forwards.
> It needs some concept of a higher level trigger rather than a per
> sample one like we typically use in IIO.
>
>From what I've read so far it seems that we would need to have two
operation modes: one for the frequency sweep (that need to be
discussed yet) and another for the receive stage (in which ad5933 would
be continuously requested for data through I2C). So my first approach
would be to build up an abstract trigger that would allow switching
between these two operation modes. What do you think about that?
> The main focus in the short term should be around defining that ABI
> as it may fundamentally change the structure of the driver.
> If you want to take this on (and it'll be a big job I think!) then
> it may be possible to source some hardware to support that effort.
>
> Thanks,
>
> Jonathan
>
As a member of the FLOSS group at Universidade de São Paulo I have the
hardware to test the driver though I didn't figure out all the steps to
do it yet. I intend to continue development on this driver so I'm really
thankful for all advise given.
Thanks,
Marcelo
> > ---
> > .../staging/iio/impedance-analyzer/Kconfig| 2 +-
> > .../staging/iio/impedance-analyzer/ad5933.c | 25 ---
> > 2 files changed, 6 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig
> > b/drivers/staging/iio/impedance-analyzer/Kconfig
> > index dd97b6bb3fd0..d0af5aa55dc0 100644
> > --- a/drivers/staging/iio/impedance-analyzer/Kconfig
> > +++ b/drivers/staging/iio/impedance-analyzer/Kconfig
> > @@ -7,7 +7,7 @@ config AD5933
> > tristate "Analog Devices AD5933, AD5934 driver"
> > depends on I2C
> > select IIO_BUFFER
> > - select IIO_KFIFO_BUF
> > + select IIO_TRIGGERED_BUFFER
> > help
> > Say yes here to build support for Analog Devices Impedance Converter,
> > Network Analyzer, AD5933/4, provides direct access via sysfs.
> > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > index f9bcb8310e21..edb8b540bbf1 100644
> > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > @@ -20,7 +20,7 @@
> > #include
> > #include
> > #include
> > -#include
> > +#include
> >
> > /* AD5933/AD5934 Registers */
> > #define AD5933_REG_CONTROL_HB 0x80/* R/W, 1 byte */
> > @@ -615,22 +615,6 @@ static const struct iio_buffer_setup_ops
> > ad5933_ring_setup_ops = {
> > .postdisable = ad5933_ring_postdisable,
> > };
> >
> > -static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> > -{
> > - struct iio_buffer *buffer;
> > -
> > - buffer = iio_kfifo_allocate();
> > - if (!buffer)
> > - return -ENOMEM;
> > -
> > - iio_device_attach_buffer(indio_dev, buffer);
> > -
> > - /* Ring buffer functions - here trigger setup related */
> > - indio_dev->setup_ops = _ring_setup_ops;
> > -
> > - return 0;
> > -}
> > -
> > static void ad5933_work(struct work_struct *work)
> > {
> > struct ad5933_state *st = container_of(work,
> > @@ -744,7 +728,8 @@ static int ad5933_probe(struct i2c_client *client,
> > indio_dev->channels = ad5933_channels;
> > indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
> >
> > - ret = ad5933_register_ring_funcs_and_init(indio_dev);
> > + ret = iio_triggered_buffer_setup(indio_dev, NULL, NULL,
> > +_ring_setup_ops
[PATCH] staging: iio: ad5933: add binding doc for ad5933
Add a devicetree documentation for the ad5933 and ad5934 impedance
converter, network analyzer.
Co-Developed-by: Gabriel Capella
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
---
.../iio/impedance-analyzer/ad5933.txt | 23 +++
1 file changed, 23 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
diff --git
a/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
new file mode 100644
index ..d9ae2babf016
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
@@ -0,0 +1,23 @@
+Analog Devices AD5933/AD5934 Impedance Converter, Network Analyzer
+
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5934.pdf
+
+Required properties:
+ - compatible : should be one of
+ "adi,ad5933"
+ "adi,ad5934"
+ - reg : the I2C address
+ - vdd-supply : supply reference for the device.
+
+Optional properties:
+ - vref_mv : default voltage reference.
+ - ext_clk_hz : external master clock for the system.
+
+Example:
+
+ impedance-analyzer@0d {
+ compatible = "adi,ad5933";
+ reg = <0x0d>;
+ vdd-supply = <_supply>;
+ };
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: iio: ad5933: add device tree support
Add a of_device_id struct variable and subsequent call to
MODULE_DEVICE_TABLE macro to complete device tree support.
Signed-off-by: Marcelo Schmitt
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index edb8b540bbf1..6faa2700dc8d 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -771,9 +771,18 @@ static const struct i2c_device_id ad5933_id[] = {
MODULE_DEVICE_TABLE(i2c, ad5933_id);
+static const struct of_device_id ad5933_of_match[] = {
+ { .compatible = "adi,ad5933" },
+ { .compatible = "adi,ad5934" },
+ { },
+};
+
+MODULE_DEVICE_TABLE(of, ad5933_of_match);
+
static struct i2c_driver ad5933_driver = {
.driver = {
.name = "ad5933",
+ .of_match_table = ad5933_of_match,
},
.probe = ad5933_probe,
.remove = ad5933_remove,
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad5933: finalized device-tree support
Added a of_device_id struct variable and subsequent call to
MODULE_DEVICE_TABLE macro to complete device-tree support for this
driver.
Signed-off-by: Marcelo Schmitt
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index edb8b540bbf1..19e8b6d2865c 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -771,9 +771,18 @@ static const struct i2c_device_id ad5933_id[] = {
MODULE_DEVICE_TABLE(i2c, ad5933_id);
+static const struct of_device_id ad5933_of_match[] = {
+ { .compatible = "analog-devices,ad5933" },
+ { .compatible = "analog-devices,ad5934" },
+ { },
+};
+
+MODULE_DEVICE_TABLE(of, ad5933_of_match);
+
static struct i2c_driver ad5933_driver = {
.driver = {
.name = "ad5933",
+ .of_match_table = ad5933_of_match,
},
.probe = ad5933_probe,
.remove = ad5933_remove,
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad5933: replaced kfifo by triggered_buffer
Previously, there was an implicit creation of a kfifo which was replaced
by a call to triggered_buffer_setup, which is already implemented in iio
infrastructure.
Signed-off-by: Marcelo Schmitt
---
.../staging/iio/impedance-analyzer/Kconfig| 2 +-
.../staging/iio/impedance-analyzer/ad5933.c | 25 ---
2 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig
b/drivers/staging/iio/impedance-analyzer/Kconfig
index dd97b6bb3fd0..d0af5aa55dc0 100644
--- a/drivers/staging/iio/impedance-analyzer/Kconfig
+++ b/drivers/staging/iio/impedance-analyzer/Kconfig
@@ -7,7 +7,7 @@ config AD5933
tristate "Analog Devices AD5933, AD5934 driver"
depends on I2C
select IIO_BUFFER
- select IIO_KFIFO_BUF
+ select IIO_TRIGGERED_BUFFER
help
Say yes here to build support for Analog Devices Impedance Converter,
Network Analyzer, AD5933/4, provides direct access via sysfs.
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index f9bcb8310e21..edb8b540bbf1 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -20,7 +20,7 @@
#include
#include
#include
-#include
+#include
/* AD5933/AD5934 Registers */
#define AD5933_REG_CONTROL_HB 0x80/* R/W, 1 byte */
@@ -615,22 +615,6 @@ static const struct iio_buffer_setup_ops
ad5933_ring_setup_ops = {
.postdisable = ad5933_ring_postdisable,
};
-static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
- struct iio_buffer *buffer;
-
- buffer = iio_kfifo_allocate();
- if (!buffer)
- return -ENOMEM;
-
- iio_device_attach_buffer(indio_dev, buffer);
-
- /* Ring buffer functions - here trigger setup related */
- indio_dev->setup_ops = _ring_setup_ops;
-
- return 0;
-}
-
static void ad5933_work(struct work_struct *work)
{
struct ad5933_state *st = container_of(work,
@@ -744,7 +728,8 @@ static int ad5933_probe(struct i2c_client *client,
indio_dev->channels = ad5933_channels;
indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
- ret = ad5933_register_ring_funcs_and_init(indio_dev);
+ ret = iio_triggered_buffer_setup(indio_dev, NULL, NULL,
+_ring_setup_ops);
if (ret)
goto error_disable_reg;
@@ -759,7 +744,7 @@ static int ad5933_probe(struct i2c_client *client,
return 0;
error_unreg_ring:
- iio_kfifo_free(indio_dev->buffer);
+ iio_triggered_buffer_cleanup(indio_dev);
error_disable_reg:
regulator_disable(st->reg);
@@ -772,7 +757,7 @@ static int ad5933_remove(struct i2c_client *client)
struct ad5933_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- iio_kfifo_free(indio_dev->buffer);
+ iio_triggered_buffer_cleanup(indio_dev);
regulator_disable(st->reg);
return 0;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] Staging iio: adc: Match parenthesis alignment
Change close parenthesis alignment to match respective open parenthesis at iio/drivers/staging/iio/adc/ad7606.c line 379. This makes the file more compliant with the preferred coding style for the linux kernel. Signed-of-by: Marcelo Schmitt --- drivers/staging/iio/adc/ad7606.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c index 0b728b6ea135..230faae38c12 100644 --- a/drivers/staging/iio/adc/ad7606.c +++ b/drivers/staging/iio/adc/ad7606.c @@ -376,7 +376,7 @@ static int ad7606_request_gpios(struct ad7606_state *st) return 0; st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio", - GPIOD_OUT_LOW); + GPIOD_OUT_LOW); return PTR_ERR_OR_ZERO(st->gpio_os); } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging iio/adc: fixes parenthesis alignment
Fixes close parenthesis alignment to match open parenthesis at iio/drivers/staging/iio/adc/ad7606.c line 379. Signed-of-by: Marcelo Schmitt --- drivers/staging/iio/adc/ad7606.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c index 0b728b6ea135..230faae38c12 100644 --- a/drivers/staging/iio/adc/ad7606.c +++ b/drivers/staging/iio/adc/ad7606.c @@ -376,7 +376,7 @@ static int ad7606_request_gpios(struct ad7606_state *st) return 0; st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio", - GPIOD_OUT_LOW); + GPIOD_OUT_LOW); return PTR_ERR_OR_ZERO(st->gpio_os); } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
