Re: [PATCH] staging:iio:adc:ad7280a: add of_match_table entry
On Fri, Jul 26, 2019 at 2:30 AM Ardelean, Alexandru
wrote:
>
> On Fri, 2019-07-26 at 01:38 +0530, Kartik Kulkarni wrote:
> > Add the of_device_id struct and the respective
> > of_match_device entry to complete device tree support.
> >
>
> This would be a [V2] I suppose.
>
> This change also does the rename of the driver name in a single go.
> Since it's a trivial change, it's fine from my side.
I think there was a small confusion when we sent the patches. Sorry
for that. Originally, Kartik made the rename in its own patch. Would
it be better if we resend the two patches separately?
Thanks,
Matheus
> Reviewed-by: Alexandru Ardelean
>
>
> > Signed-off-by: Kartik Kulkarni
> > Reviewed-by: Matheus Tavares
> > ---
> > drivers/staging/iio/adc/ad7280a.c | 9 -
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > b/drivers/staging/iio/adc/ad7280a.c
> > index 19a5f244dcae..ded0ba093a28 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -1027,9 +1027,16 @@ static const struct spi_device_id ad7280_id[] = {
> > };
> > MODULE_DEVICE_TABLE(spi, ad7280_id);
> >
> > +static const struct of_device_id ad7280_of_match[] = {
> > + { .compatible = "adi,ad7280a", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, ad7280_of_match);
> > +
> > static struct spi_driver ad7280_driver = {
> > .driver = {
> > - .name = "ad7280",
> > + .name = "ad7280a",
> > + .of_match_table = ad7280_of_match,
> > },
> > .probe = ad7280_probe,
> > .id_table = ad7280_id,
>
> --
> You received this message because you are subscribed to the Google Groups
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-usp+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kernel-usp/0e273486f1c4fb6249896225837cdf2da0fd2415.camel%40analog.com.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 7/7] staging:iio:ad2s90: Move out of staging
Move ad2s90 resolver driver out of staging to the main tree.
Signed-off-by: Matheus Tavares
Signed-off-by: Victor Colombo
---
Changes in v3:
- none
Changes in v2:
- Disabled git move detection, to see the whole code, as Jonathan
suggested
drivers/iio/resolver/Kconfig | 10 ++
drivers/iio/resolver/Makefile | 1 +
drivers/iio/resolver/ad2s90.c | 131 ++
drivers/staging/iio/resolver/Kconfig | 10 --
drivers/staging/iio/resolver/Makefile | 1 -
drivers/staging/iio/resolver/ad2s90.c | 131 --
6 files changed, 142 insertions(+), 142 deletions(-)
create mode 100644 drivers/iio/resolver/ad2s90.c
delete mode 100644 drivers/staging/iio/resolver/ad2s90.c
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
index 2ced9f22aa70..786801be54f6 100644
--- a/drivers/iio/resolver/Kconfig
+++ b/drivers/iio/resolver/Kconfig
@@ -3,6 +3,16 @@
#
menu "Resolver to digital converters"
+config AD2S90
+ tristate "Analog Devices ad2s90 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s90, provides direct access via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s90.
+
config AD2S1200
tristate "Analog Devices ad2s1200/ad2s1205 driver"
depends on SPI
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
index 4e1dccae07e7..398d82d50028 100644
--- a/drivers/iio/resolver/Makefile
+++ b/drivers/iio/resolver/Makefile
@@ -2,4 +2,5 @@
# Makefile for Resolver/Synchro drivers
#
+obj-$(CONFIG_AD2S90) += ad2s90.o
obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s90.c b/drivers/iio/resolver/ad2s90.c
new file mode 100644
index ..a41f5cb10da5
--- /dev/null
+++ b/drivers/iio/resolver/ad2s90.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ 83
+
+struct ad2s90_state {
+ struct mutex lock; /* lock to protect rx buffer */
+ struct spi_device *sdev;
+ u8 rx[2] cacheline_aligned;
+};
+
+static int ad2s90_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long m)
+{
+ int ret;
+ struct ad2s90_state *st = iio_priv(indio_dev);
+
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / 2^12 */
+ *val = 6283; /* mV */
+ *val2 = 12;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
+ mutex_unlock(>lock);
+
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_info ad2s90_info = {
+ .read_raw = ad2s90_read_raw,
+};
+
+static const struct iio_chan_spec ad2s90_chan = {
+ .type = IIO_ANGL,
+ .indexed = 1,
+ .channel = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+};
+
+static int ad2s90_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct ad2s90_state *st;
+
+ if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+ dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+ spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+ return -EINVAL;
+ }
+
+ indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ mutex_init(>lock);
+ st->sdev = spi;
+ indio_dev->dev.parent = >dev;
+ indio_dev->info = _info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = _chan;
+ indio_dev->num_channels = 1;
+ indio_dev->name = spi_get_device_id(spi)->name;
+
+ return devm_iio_device_register(ind
[PATCH v3 2/7] staging:iio:ad2s90: Remove spi setup that should be done via dt
The ad2s90 driver currently sets some spi settings (max_speed_hz and
mode) at ad2s90_probe. Since the maximum frequency is a required element
in DT binding for spi slave devices and because the spi mode for the
device can be either (0,0) or (1,1), these settings should be handled
via device tree, not in the driver's code. This patch removes them from
the probe function.
Note: The way in which the mentioned spi settings need to be specified
on the ad2s90's node of a device tree will be documented in the future
patch "dt-bindings:iio:resolver: Add docs for ad2s90".
Signed-off-by: Matheus Tavares
---
Changes in v3:
- none
Changes in v2:
- Rewritten patch message to better explain why the code snippet in
question should be removed.
drivers/staging/iio/resolver/ad2s90.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index fdae067ed866..abb9b9147ee6 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- /* need 600ns between CS and the first falling edge of SCLK */
- spi->max_speed_hz = 83;
- spi->mode = SPI_MODE_3;
- ret = spi_setup(spi);
-
- if (ret < 0) {
- dev_err(>dev, "spi_setup failed!\n");
- return ret;
- }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 6/7] staging:iio:ad2s90: Add comment to device state mutex
From: Victor Colombo
Fix the checkpatch.pl issue:
"CHECK: struct mutex definition without comment".
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
Changes in v3:
- none
Changes in v2:
- Patch added in v2
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 678351dabe6b..a41f5cb10da5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -22,7 +22,7 @@
#define AD2S90_MAX_SPI_FREQ_HZ 83
struct ad2s90_state {
- struct mutex lock;
+ struct mutex lock; /* lock to protect rx buffer */
struct spi_device *sdev;
u8 rx[2] cacheline_aligned;
};
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/7] dt-bindings:iio:resolver: Add docs for ad2s90
This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.
Signed-off-by: Matheus Tavares
---
Changes in v3:
- Added reference to spi-bus documentation after spi properties, as
suggested by Alexandru Ardelean.
Changes in v2:
- Rewritten 'spi-cpol and spi-cpha' item to say that the device can
work in either mode (0,0) or (1,1) and explain how they should be
specified in DT.
.../bindings/iio/resolver/ad2s90.txt | 31 +++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
new file mode 100644
index ..477d41fa6467
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
@@ -0,0 +1,31 @@
+Analog Devices AD2S90 Resolver-to-Digital Converter
+
+https://www.analog.com/en/products/ad2s90.html
+
+Required properties:
+ - compatible: should be "adi,ad2s90"
+ - reg: SPI chip select number for the device
+ - spi-max-frequency: set maximum clock frequency, must be 83
+ - spi-cpol and spi-cpha:
+Either SPI mode (0,0) or (1,1) must be used, so specify none or both of
+spi-cpha, spi-cpol.
+
+See for more details:
+Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Note about max frequency:
+Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 600ns
+delay is expected between the application of a logic LO to CS and the
+application of SCLK, as also specified. And since the delay is not
+implemented in the spi code, to satisfy it, SCLK's period should be at most
+2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
+roughly 83Hz.
+
+Example:
+resolver@0 {
+ compatible = "adi,ad2s90";
+ reg = <0>;
+ spi-max-frequency = <83>;
+ spi-cpol;
+ spi-cpha;
+};
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 5/7] staging:iio:ad2s90: Replace license text w/ SPDX identifier
This patch removes the license boilerplate text at the top of ad2s90.c and, instead, adds the SPDX GPL-2.0 license identifier, which solves the checkpatch.pl warning: "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1". Signed-off-by: Matheus Tavares --- Changes in v3: - none Changes in v2: - Changed GPL-2.0-only identifier to GPL-2.0 - Removed license boilerplate text - Rewritten patch message to reflect these modifications drivers/staging/iio/resolver/ad2s90.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index 4721e9bbb8b0..678351dabe6b 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -1,12 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90 * * Copyright (c) 2010-2010 Analog Devices Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * */ #include #include -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/7] staging:iio:ad2s90: Add max frequency check at probe
From: Alexandru Ardelean
This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.
The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.
Signed-off-by: Alexandru Ardelean
Signed-off-by: Matheus Tavares
---
Changes in v3:
- none
Changes in v2:
- Correctly credit Alexandru as the patch's author
drivers/staging/iio/resolver/ad2s90.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index abb9b9147ee6..4721e9bbb8b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
#include
#include
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ 83
+
struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
+ if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+ dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+ spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+ return -EINVAL;
+ }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/7] staging:iio:ad2s90: Add device tree support
This patch adds device tree support to ad2s90 with standard
device tree id table.
Signed-off-by: Matheus Tavares
---
Changes in v3:
- Removed of_patch_ptr from of_match_table assignment
Changes in v2:
- none
drivers/staging/iio/resolver/ad2s90.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 3e257ac46f7a..fdae067ed866 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
+static const struct of_device_id ad2s90_of_match[] = {
+ { .compatible = "adi,ad2s90", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad2s90_of_match);
+
static const struct spi_device_id ad2s90_id[] = {
{ "ad2s90" },
{}
@@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
static struct spi_driver ad2s90_driver = {
.driver = {
.name = "ad2s90",
+ .of_match_table = ad2s90_of_match,
},
.probe = ad2s90_probe,
.id_table = ad2s90_id,
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/7] staging:iio:ad2s90: Add dt support and move out of staging
This series adds device tree support to ad2s90, adds the respective dt-binding documentation, solves all remaining codestyle problems for ad2s90 and move it out of staging. This patch set completes all the remaining itens listed to be done before moving the driver out of staging, enumerated in this mail thread: https://marc.info/?l=linux-iio=154028966111330=2. Alexandru Ardelean (1): staging:iio:ad2s90: Add max frequency check at probe Matheus Tavares (5): staging:iio:ad2s90: Add device tree support staging:iio:ad2s90: Remove spi setup that should be done via dt dt-bindings:iio:resolver: Add docs for ad2s90 staging:iio:ad2s90: Replace license text w/ SPDX identifier staging:iio:ad2s90: Move out of staging Victor Colombo (1): staging:iio:ad2s90: Add comment to device state mutex .../bindings/iio/resolver/ad2s90.txt | 31 + drivers/iio/resolver/Kconfig | 10 ++ drivers/iio/resolver/Makefile | 1 + drivers/iio/resolver/ad2s90.c | 131 ++ drivers/staging/iio/resolver/Kconfig | 10 -- drivers/staging/iio/resolver/Makefile | 1 - drivers/staging/iio/resolver/ad2s90.c | 127 - 7 files changed, 173 insertions(+), 138 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt create mode 100644 drivers/iio/resolver/ad2s90.c delete mode 100644 drivers/staging/iio/resolver/ad2s90.c -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/7] staging:iio:ad2s90: Add device tree support
On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds device tree support to ad2s90 with standard
> > device tree id table.
> >
>
> Hey,
>
> Comment inline
>
> > Signed-off-by: Matheus Tavares
> > ---
> > Changes in v2:
> > - none
> >
> > drivers/staging/iio/resolver/ad2s90.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index 3e257ac46f7a..6ffbac66b837 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
> > return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> > }
> >
> > +static const struct of_device_id ad2s90_of_match[] = {
> > + { .compatible = "adi,ad2s90", },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ad2s90_of_match);
> > +
> > static const struct spi_device_id ad2s90_id[] = {
> > { "ad2s90" },
> > {}
> > @@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
> > static struct spi_driver ad2s90_driver = {
> > .driver = {
> > .name = "ad2s90",
> > + .of_match_table = of_match_ptr(ad2s90_of_match),
>
> I think you need to remove the of_match_ptr().
> There was a comment from Jonathan on another thread about this.
> See:
>https://patchwork.kernel.org/patch/10682963/
>
Hm, got it, thanks!
I don't understand much about ACPI yet, and I had understood the
"of_match_ptr" as a guard. Could someone point me in which cases it
should be used? Or is it obsolete?
Matheus
> So,
> + .of_match_table = of_match_ptr(ad2s90_of_match),
>
> becomes
> > + .of_match_table = ad2s90_of_match,
>
> > },
> > .probe = ad2s90_probe,
> > .id_table = ad2s90_id,
>
> --
> You received this message because you are subscribed to the Google Groups
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kernel-usp/f250fa3a01b51d59979e7a2e3e42cc34d02aa52e.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90
On Mon, Nov 19, 2018 at 6:22 AM Ardelean, Alexandru
wrote:
>
> On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
>
> One minor comment inline.
>
> > Signed-off-by: Matheus Tavares
> > ---
> > Changes in v2:
> > - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
> > work in either mode (0,0) or (1,1) and explain how they should be
> > specified in DT.
> >
> > .../bindings/iio/resolver/ad2s90.txt | 28 +++
> > 1 file changed, 28 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..594417539938
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,28 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > + - compatible: should be "adi,ad2s90"
> > + - reg: SPI chip select number for the device
> > + - spi-max-frequency: set maximum clock frequency, must be 83
> > + - spi-cpol and spi-cpha:
> > +Either SPI mode (0,0) or (1,1) must be used, so specify none or
> > both of
> > +spi-cpha, spi-cpol.
> For SPI properties it's a good idea to also reference the document for SPI
> bindings.
> Something like:
> See for more details:
> Documentation/devicetree/bindings/spi/spi-bus.txt
>
Thanks, Alex! I'll add that for v3.
Also, can you confirm AD2S90 works in both spi mode 0 and 3? It's not
explicitly stated in the datasheet, but that's what it seemed to me
and some colleagues.
Thanks,
Matheus
> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a
> > 600ns
> > +delay is expected between the application of a logic LO to CS and
> > the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be
> > at most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> > gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>
> --
> You received this message because you are subscribed to the Google Groups
> "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kernel-usp+unsubscr...@googlegroups.com.
> To post to this group, send email to kernel-...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kernel-usp/563614e00314ba92b9513645a82fde06504a42d5.camel%40analog.com.
> For more options, visit https://groups.google.com/d/optout.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/7] staging:iio:ad2s90: Move out of staging
Move ad2s90 resolver driver out of staging to the main tree.
Signed-off-by: Matheus Tavares
Signed-off-by: Victor Colombo
---
Changes in v2:
- Disabled git move detection, to see the whole code, as Jonathan
suggested
drivers/iio/resolver/Kconfig | 10 ++
drivers/iio/resolver/Makefile | 1 +
drivers/iio/resolver/ad2s90.c | 131 ++
drivers/staging/iio/resolver/Kconfig | 10 --
drivers/staging/iio/resolver/Makefile | 1 -
drivers/staging/iio/resolver/ad2s90.c | 131 --
6 files changed, 142 insertions(+), 142 deletions(-)
create mode 100644 drivers/iio/resolver/ad2s90.c
delete mode 100644 drivers/staging/iio/resolver/ad2s90.c
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
index 2ced9f22aa70..786801be54f6 100644
--- a/drivers/iio/resolver/Kconfig
+++ b/drivers/iio/resolver/Kconfig
@@ -3,6 +3,16 @@
#
menu "Resolver to digital converters"
+config AD2S90
+ tristate "Analog Devices ad2s90 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s90, provides direct access via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s90.
+
config AD2S1200
tristate "Analog Devices ad2s1200/ad2s1205 driver"
depends on SPI
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
index 4e1dccae07e7..398d82d50028 100644
--- a/drivers/iio/resolver/Makefile
+++ b/drivers/iio/resolver/Makefile
@@ -2,4 +2,5 @@
# Makefile for Resolver/Synchro drivers
#
+obj-$(CONFIG_AD2S90) += ad2s90.o
obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s90.c b/drivers/iio/resolver/ad2s90.c
new file mode 100644
index ..f04dc5dede00
--- /dev/null
+++ b/drivers/iio/resolver/ad2s90.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ 83
+
+struct ad2s90_state {
+ struct mutex lock; /* lock to protect rx buffer */
+ struct spi_device *sdev;
+ u8 rx[2] cacheline_aligned;
+};
+
+static int ad2s90_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long m)
+{
+ int ret;
+ struct ad2s90_state *st = iio_priv(indio_dev);
+
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / 2^12 */
+ *val = 6283; /* mV */
+ *val2 = 12;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
+ mutex_unlock(>lock);
+
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_info ad2s90_info = {
+ .read_raw = ad2s90_read_raw,
+};
+
+static const struct iio_chan_spec ad2s90_chan = {
+ .type = IIO_ANGL,
+ .indexed = 1,
+ .channel = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+};
+
+static int ad2s90_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct ad2s90_state *st;
+
+ if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+ dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+ spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+ return -EINVAL;
+ }
+
+ indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ mutex_init(>lock);
+ st->sdev = spi;
+ indio_dev->dev.parent = >dev;
+ indio_dev->info = _info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = _chan;
+ indio_dev->num_channels = 1;
+ indio_dev->name = spi_get_device_id(spi)->name;
+
+ return devm_iio_device_register(ind
[PATCH v2 0/7] staging:iio:ad2s90: Add dt support and move out of staging
This series adds device tree support to ad2s90, adds the respective dt-binding documentation, solves all remaining codestyle problems in the driver code and move it out of staging. This patch set completes all the remaining itens listed to be done before moving the driver out of staging, enumerated in this mail thread: https://marc.info/?l=linux-iio=154028966111330=2. Alexandru Ardelean (1): staging:iio:ad2s90: Add max frequency check at probe Matheus Tavares (5): staging:iio:ad2s90: Add device tree support staging:iio:ad2s90: Remove spi setup that should be done via dt dt-bindings:iio:resolver: Add docs for ad2s90 staging:iio:ad2s90: Replace license text w/ SPDX identifier staging:iio:ad2s90: Move out of staging Victor Colombo (1): staging:iio:ad2s90: Add comment to device state mutex .../bindings/iio/resolver/ad2s90.txt | 28 drivers/iio/resolver/Kconfig | 10 ++ drivers/iio/resolver/Makefile | 1 + drivers/iio/resolver/ad2s90.c | 131 ++ drivers/staging/iio/resolver/Kconfig | 10 -- drivers/staging/iio/resolver/Makefile | 1 - drivers/staging/iio/resolver/ad2s90.c | 127 - 7 files changed, 170 insertions(+), 138 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt create mode 100644 drivers/iio/resolver/ad2s90.c delete mode 100644 drivers/staging/iio/resolver/ad2s90.c -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/7] staging:iio:ad2s90: Add device tree support
This patch adds device tree support to ad2s90 with standard
device tree id table.
Signed-off-by: Matheus Tavares
---
Changes in v2:
- none
drivers/staging/iio/resolver/ad2s90.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 3e257ac46f7a..6ffbac66b837 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
+static const struct of_device_id ad2s90_of_match[] = {
+ { .compatible = "adi,ad2s90", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad2s90_of_match);
+
static const struct spi_device_id ad2s90_id[] = {
{ "ad2s90" },
{}
@@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
static struct spi_driver ad2s90_driver = {
.driver = {
.name = "ad2s90",
+ .of_match_table = of_match_ptr(ad2s90_of_match),
},
.probe = ad2s90_probe,
.id_table = ad2s90_id,
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/7] staging:iio:ad2s90: Add comment to device state mutex
From: Victor Colombo
Fix the checkpatch.pl issue:
"CHECK: struct mutex definition without comment".
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
Changes in v2:
- Patch added in v2
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 9aa229ba47e7..f04dc5dede00 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -22,7 +22,7 @@
#define AD2S90_MAX_SPI_FREQ_HZ 83
struct ad2s90_state {
- struct mutex lock;
+ struct mutex lock; /* lock to protect rx buffer */
struct spi_device *sdev;
u8 rx[2] cacheline_aligned;
};
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/7] staging:iio:ad2s90: Replace license text w/ SPDX identifier
This patch removes the license boilerplate text at the top of ad2s90.c and, instead, adds the SPDX GPL-2.0 license identifier, which solves the checkpatch.pl warning: "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1". Signed-off-by: Matheus Tavares --- Changes in v2: - Changed GPL-2.0-only identifier to GPL-2.0 - Removed license boilerplate text - Rewritten patch message to reflect these modifications drivers/staging/iio/resolver/ad2s90.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index fe90f2056bff..9aa229ba47e7 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -1,12 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90 * * Copyright (c) 2010-2010 Analog Devices Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * */ #include #include -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe
From: Alexandru Ardelean
This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.
The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.
Signed-off-by: Alexandru Ardelean
Signed-off-by: Matheus Tavares
---
Changes in v2:
- Correctly credit Alexandru as the patch's author
drivers/staging/iio/resolver/ad2s90.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 913d6fad2d4d..fe90f2056bff 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
#include
#include
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ 83
+
struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
+ if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+ dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+ spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+ return -EINVAL;
+ }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/7] staging:iio:ad2s90: Remove spi setup that should be done via dt
The ad2s90 driver currently sets some spi settings (max_speed_hz and
mode) at ad2s90_probe. Since the maximum frequency is a required element
in DT binding for spi slave devices and because the spi mode for the
device can be either (0,0) or (1,1), these settings should be handled
via device tree, not in the driver's code. This patch removes them from
the probe function.
Note: The way in which the mentioned spi settings need to be specified
on the ad2s90's node of a device tree will be documented in the future
patch "dt-bindings:iio:resolver: Add docs for ad2s90".
Signed-off-by: Matheus Tavares
---
Changes in v2:
- Rewritten patch message to better explain why the code snippet in
question should be removed.
drivers/staging/iio/resolver/ad2s90.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 6ffbac66b837..913d6fad2d4d 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- /* need 600ns between CS and the first falling edge of SCLK */
- spi->max_speed_hz = 83;
- spi->mode = SPI_MODE_3;
- ret = spi_setup(spi);
-
- if (ret < 0) {
- dev_err(>dev, "spi_setup failed!\n");
- return ret;
- }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90
This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.
Signed-off-by: Matheus Tavares
---
Changes in v2:
- Rewritten 'spi-cpol and spi-cpha' item to say that the device can
work in either mode (0,0) or (1,1) and explain how they should be
specified in DT.
.../bindings/iio/resolver/ad2s90.txt | 28 +++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
new file mode 100644
index ..594417539938
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
@@ -0,0 +1,28 @@
+Analog Devices AD2S90 Resolver-to-Digital Converter
+
+https://www.analog.com/en/products/ad2s90.html
+
+Required properties:
+ - compatible: should be "adi,ad2s90"
+ - reg: SPI chip select number for the device
+ - spi-max-frequency: set maximum clock frequency, must be 83
+ - spi-cpol and spi-cpha:
+Either SPI mode (0,0) or (1,1) must be used, so specify none or both of
+spi-cpha, spi-cpol.
+
+Note about max frequency:
+Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 600ns
+delay is expected between the application of a logic LO to CS and the
+application of SCLK, as also specified. And since the delay is not
+implemented in the spi code, to satisfy it, SCLK's period should be at most
+2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
+roughly 83Hz.
+
+Example:
+resolver@0 {
+ compatible = "adi,ad2s90";
+ reg = <0>;
+ spi-max-frequency = <83>;
+ spi-cpol;
+ spi-cpha;
+};
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] dt-bindings:iio:resolver: Add docs for ad2s90
On Sun, Nov 11, 2018 at 9:48 AM Jonathan Cameron wrote:
>
> On Fri, 9 Nov 2018 20:00:42 -0200
> Matheus Tavares wrote:
>
> > This patch adds the device tree binding documentation for the ad2s90
> > resolver-to-digital converter.
> >
> > Signed-off-by: Matheus Tavares
> > ---
> > .../bindings/iio/resolver/ad2s90.txt | 26 +++
> > 1 file changed, 26 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> >
> > diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > new file mode 100644
> > index ..b42cc7752ffd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
> > @@ -0,0 +1,26 @@
> > +Analog Devices AD2S90 Resolver-to-Digital Converter
> > +
> > +https://www.analog.com/en/products/ad2s90.html
> > +
> > +Required properties:
> > + - compatible : should be "adi,ad2s90"
> > + - reg : SPI chip select number for the device
> > + - spi-max-frequency : set maximum clock frequency, must be 83
> > + - spi-cpol and spi-cpha : must be defined to enable SPI mode 3
>
> As the part only works in mode 3, my gut feeling is that this belongs
> in the driver, not here. Rob, what do you think?
>
For this patch, I assumed the part only worked in mode 3 based on the
driver's code that set this at probe. But today I carefully looked for
it at the datasheet and now I'm unsure. It is never said, explicitly,
which SPI mode ad2s90 works with. But looking at the diagram that
shows the expected pins signals at each communication moment, it seems
to me that this chip can either work in mode 0 (CPOL=0, CPHA=0) or
mode 3 (CPOL=1, CPHA=1). Could someone help me to confirm this? And if
that is the case, them the SPI mode setting should be left in DT, as
adc/mcp320x and dac/ti-dac082s085 do, right?
Also, when I thought that ad2s90 only worked in mode 3, I wrote this
patch based on the dt-binding docs for the adxl345 accelerometer,
which only works in mode 3 but lets this setting to DT not in the
driver. Do you think, perhaps, it is wrong in adxl345, them?
Thanks,
Matheus.
> > +
> > +Note about max frequency:
> > +Chip's max frequency, as specified in its datasheet, is 2Mhz. But a
> > 600ns
> > +delay is expected between the application of a logic LO to CS and the
> > +application of SCLK, as also specified. And since the delay is not
> > +implemented in the spi code, to satisfy it, SCLK's period should be at
> > most
> > +2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
> > +roughly 83Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <83>;
> > + spi-cpol;
> > + spi-cpha;
> > +};
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging:iio:ad2s90: Remove spi setup that should be done via dt
On Sun, Nov 11, 2018 at 9:42 AM Jonathan Cameron wrote:
>
> On Fri, 9 Nov 2018 20:00:40 -0200
> Matheus Tavares wrote:
>
> > The ad2s90 driver currently sets some spi settings (max_speed_hz and
> > mode) at ad2s90_probe. This should, instead, be handled via device tree.
> > This patch removes these configurations from the probe function.
> >
> > Note: The way in which the mentioned spi settings need to be specified
> > on the ad2s90's node of a device tree will be documented in the future
> > patch "dt-bindings:iio:resolver: Add docs for ad2s90".
> >
> > Signed-off-by: Matheus Tavares
> I'd actually like to get Rob and Mark's views on this one. Previously
> I would just have applied it without thinking on the basis these can
> be easily specified from devicetree.
>
> Recent discussions with Rob have suggested that the settings in devicetree
> should perhaps be concerned with specifying constraints about the device
> that are not visible to the driver. The driver itself should apply
> the device constraints, but there are others such as wiring that
> might reduce the maximum frequency for example...
>
> So should a driver be clamping an over specified value from DT for
> example? Or given that is optional in DT, should it be making sure
> that a controller max frequency isn't too high for the sensor?
>
First of all, thanks for the review and comments.
By what you've said here and in the reviews for patches 3 and 4 of
this patch-set, it seems to me that the most reasonable thing would be
to keep the SPI mode 3 settings at the driver but the max frequency
setting at DT and check if it exceeds the maximum at the driver (as
patch 3 does). This makes sense to me, based on what you've said,
because mode 3 is a device constraint visible to the driver (as it
won't change) but max frequency is not (because of things such as
wiring, as you said).
What do you think, Jonathan, Rob, and Mark?
Matheus
> It seems to be unusual to do this, but to my mind it would make
> sense and might be worth pushing out into more drivers.
>
> Jonathan
> > ---
> > drivers/staging/iio/resolver/ad2s90.c | 11 ---
> > 1 file changed, 11 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index ff32ca76ca00..95c118c48400 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
> > {
> > struct iio_dev *indio_dev;
> > struct ad2s90_state *st;
> > - int ret;
> >
> > indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
> > if (!indio_dev)
> > @@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
> > indio_dev->num_channels = 1;
> > indio_dev->name = spi_get_device_id(spi)->name;
> >
> > - /* need 600ns between CS and the first falling edge of SCLK */
> > - spi->max_speed_hz = 83;
> > - spi->mode = SPI_MODE_3;
> > - ret = spi_setup(spi);
> > -
> > - if (ret < 0) {
> > - dev_err(>dev, "spi_setup failed!\n");
> > - return ret;
> > - }
> > -
> > return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> > }
> >
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier
On Sat, Nov 10, 2018 at 11:23 AM Fabio Estevam wrote:> > Hi Matheus, > > On Fri, Nov 9, 2018 at 10:27 PM Matheus Tavares Bernardino > wrote: > > > Got it, thanks for the explanation! I'll correct this in v2. > > One more suggestion: in v2 you could also consider to remove the legal > text that says GPL v2, as you are adding the SPDX tag. Okay, I'll do it! Thanks again for the review and suggestions! Matheus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier
On Fri, Nov 9, 2018 at 10:20 PM Greg Kroah-Hartman wrote: > > On Fri, Nov 09, 2018 at 09:19:52PM -0200, Matheus Tavares Bernardino wrote: > > On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam wrote: > > > > > > Hi Matheus, > > > > > > > Hi, Fabio > > > > > On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares > > > wrote: > > > > > > > > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c, > > > > which solves the checkpatch.pl warning: > > > > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1". > > > > > > > > Signed-off-by: Matheus Tavares > > > > --- > > > > drivers/staging/iio/resolver/ad2s90.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/staging/iio/resolver/ad2s90.c > > > > b/drivers/staging/iio/resolver/ad2s90.c > > > > index 949ff55ac6b0..f439da721df8 100644 > > > > --- a/drivers/staging/iio/resolver/ad2s90.c > > > > +++ b/drivers/staging/iio/resolver/ad2s90.c > > > > @@ -1,3 +1,4 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > > This should be: > > > // SPDX-License-Identifier: GPL-2.0 > > > > Hm, but it seems that the identifier "GPL-2.0" is deprecated, look: > > https://spdx.org/licenses/GPL-2.0.html. It has been updated to > > "GPL-2.0-only" in license list v3 > > (https://spdx.org/licenses/GPL-2.0-only.html). Is there some other > > reason to use the deprecated "GPL-2.0" that I'm not aware of? > > Yes, please read the in-kernel documentation for all of this at: > Documentation/process/license-rules.rst > > Long story short, we started the adding of these tags to the kernel > before the crazyness of the "-only" markings for GPL in spdx. Let's > keep it this way for now, if we ever get the whole kernel finished, then > we can revisit the markings and maybe do a wholesale conversion, if it's > really needed. > Got it, thanks for the explanation! I'll correct this in v2. Thanks, Matheus > thanks, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier
On Fri, Nov 9, 2018 at 8:13 PM Fabio Estevam wrote: > > Hi Matheus, > Hi, Fabio > On Fri, Nov 9, 2018 at 8:01 PM Matheus Tavares > wrote: > > > > This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c, > > which solves the checkpatch.pl warning: > > "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1". > > > > Signed-off-by: Matheus Tavares > > --- > > drivers/staging/iio/resolver/ad2s90.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/staging/iio/resolver/ad2s90.c > > b/drivers/staging/iio/resolver/ad2s90.c > > index 949ff55ac6b0..f439da721df8 100644 > > --- a/drivers/staging/iio/resolver/ad2s90.c > > +++ b/drivers/staging/iio/resolver/ad2s90.c > > @@ -1,3 +1,4 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > This should be: > // SPDX-License-Identifier: GPL-2.0 Hm, but it seems that the identifier "GPL-2.0" is deprecated, look: https://spdx.org/licenses/GPL-2.0.html. It has been updated to "GPL-2.0-only" in license list v3 (https://spdx.org/licenses/GPL-2.0-only.html). Is there some other reason to use the deprecated "GPL-2.0" that I'm not aware of? Thanks, Matheus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] staging:iio:ad2s90: Add dt support and move out of staging
This patch set adds device tree support to ad2s90, with standard
device tree id table, adds the respective dt-binding documentation,
solves a codestyle warning and move the driver out of staging.
This patch set completes all the remaining itens listed to be done
before moving the driver out of staging, enumerated in this mail thread:
https://marc.info/?l=linux-iio=154028966111330=2, except by one
codestyle problem: "CHECK: struct mutex definition without comment". It
seems to be a commonly ignored check for mutexes of device states. If I
am wrong, please, let me know and I will be happy to send a patch to
tackle it.
Matheus Tavares (6):
staging:iio:ad2s90: Add device tree support
staging:iio:ad2s90: Remove spi setup that should be done via dt
staging:iio:ad2s90: Add max frequency check at probe
dt-bindings:iio:resolver: Add docs for ad2s90
staging:iio:ad2s90: Add SPDX license identifier
staging:iio:ad2s90: Move out of staging
.../bindings/iio/resolver/ad2s90.txt | 26
drivers/iio/resolver/Kconfig | 10 ++
drivers/iio/resolver/Makefile | 1 +
drivers/{staging => }/iio/resolver/ad2s90.c | 31 ---
drivers/staging/iio/resolver/Kconfig | 10 --
drivers/staging/iio/resolver/Makefile | 1 -
6 files changed, 57 insertions(+), 22 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
rename drivers/{staging => }/iio/resolver/ad2s90.c (81%)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging:iio:ad2s90: Remove spi setup that should be done via dt
The ad2s90 driver currently sets some spi settings (max_speed_hz and
mode) at ad2s90_probe. This should, instead, be handled via device tree.
This patch removes these configurations from the probe function.
Note: The way in which the mentioned spi settings need to be specified
on the ad2s90's node of a device tree will be documented in the future
patch "dt-bindings:iio:resolver: Add docs for ad2s90".
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index ff32ca76ca00..95c118c48400 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- /* need 600ns between CS and the first falling edge of SCLK */
- spi->max_speed_hz = 83;
- spi->mode = SPI_MODE_3;
- ret = spi_setup(spi);
-
- if (ret < 0) {
- dev_err(>dev, "spi_setup failed!\n");
- return ret;
- }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging:iio:ad2s90: Add max frequency check at probe
This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.
The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.
Signed-off-by: Matheus Tavares
Signed-off-by: Alexandru Ardelean
---
Alex's S-o-B was added because he gave the code suggestion for this
patch.
drivers/staging/iio/resolver/ad2s90.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 95c118c48400..949ff55ac6b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
#include
#include
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ 83
+
struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
+ if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+ dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+ spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+ return -EINVAL;
+ }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging:iio:ad2s90: Move out of staging
Move ad2s90 resolver driver out of staging to the main tree.
Signed-off-by: Matheus Tavares
Signed-off-by: Victor Colombo
---
drivers/iio/resolver/Kconfig| 10 ++
drivers/iio/resolver/Makefile | 1 +
drivers/{staging => }/iio/resolver/ad2s90.c | 0
drivers/staging/iio/resolver/Kconfig| 10 --
drivers/staging/iio/resolver/Makefile | 1 -
5 files changed, 11 insertions(+), 11 deletions(-)
rename drivers/{staging => }/iio/resolver/ad2s90.c (100%)
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
index 2ced9f22aa70..786801be54f6 100644
--- a/drivers/iio/resolver/Kconfig
+++ b/drivers/iio/resolver/Kconfig
@@ -3,6 +3,16 @@
#
menu "Resolver to digital converters"
+config AD2S90
+ tristate "Analog Devices ad2s90 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s90, provides direct access via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s90.
+
config AD2S1200
tristate "Analog Devices ad2s1200/ad2s1205 driver"
depends on SPI
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
index 4e1dccae07e7..398d82d50028 100644
--- a/drivers/iio/resolver/Makefile
+++ b/drivers/iio/resolver/Makefile
@@ -2,4 +2,5 @@
# Makefile for Resolver/Synchro drivers
#
+obj-$(CONFIG_AD2S90) += ad2s90.o
obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/iio/resolver/ad2s90.c
similarity index 100%
rename from drivers/staging/iio/resolver/ad2s90.c
rename to drivers/iio/resolver/ad2s90.c
diff --git a/drivers/staging/iio/resolver/Kconfig
b/drivers/staging/iio/resolver/Kconfig
index 6a469ee6101f..4a727c17bb8f 100644
--- a/drivers/staging/iio/resolver/Kconfig
+++ b/drivers/staging/iio/resolver/Kconfig
@@ -3,16 +3,6 @@
#
menu "Resolver to digital converters"
-config AD2S90
- tristate "Analog Devices ad2s90 driver"
- depends on SPI
- help
- Say yes here to build support for Analog Devices spi resolver
- to digital converters, ad2s90, provides direct access via sysfs.
-
- To compile this driver as a module, choose M here: the
- module will be called ad2s90.
-
config AD2S1210
tristate "Analog Devices ad2s1210 driver"
depends on SPI
diff --git a/drivers/staging/iio/resolver/Makefile
b/drivers/staging/iio/resolver/Makefile
index 8d901dc7500b..b2049f2ce36e 100644
--- a/drivers/staging/iio/resolver/Makefile
+++ b/drivers/staging/iio/resolver/Makefile
@@ -2,5 +2,4 @@
# Makefile for Resolver/Synchro drivers
#
-obj-$(CONFIG_AD2S90) += ad2s90.o
obj-$(CONFIG_AD2S1210) += ad2s1210.o
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] dt-bindings:iio:resolver: Add docs for ad2s90
This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.
Signed-off-by: Matheus Tavares
---
.../bindings/iio/resolver/ad2s90.txt | 26 +++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
new file mode 100644
index ..b42cc7752ffd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
@@ -0,0 +1,26 @@
+Analog Devices AD2S90 Resolver-to-Digital Converter
+
+https://www.analog.com/en/products/ad2s90.html
+
+Required properties:
+ - compatible : should be "adi,ad2s90"
+ - reg : SPI chip select number for the device
+ - spi-max-frequency : set maximum clock frequency, must be 83
+ - spi-cpol and spi-cpha : must be defined to enable SPI mode 3
+
+Note about max frequency:
+Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 600ns
+delay is expected between the application of a logic LO to CS and the
+application of SCLK, as also specified. And since the delay is not
+implemented in the spi code, to satisfy it, SCLK's period should be at most
+2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
+roughly 83Hz.
+
+Example:
+resolver@0 {
+ compatible = "adi,ad2s90";
+ reg = <0>;
+ spi-max-frequency = <83>;
+ spi-cpol;
+ spi-cpha;
+};
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging:iio:ad2s90: Add SPDX license identifier
This patch adds the SPDX GPL-2.0-only license identifier to ad2s90.c, which solves the checkpatch.pl warning: "WARNING: Missing or malformed SPDX-License-Identifier tag in line 1". Signed-off-by: Matheus Tavares --- drivers/staging/iio/resolver/ad2s90.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c index 949ff55ac6b0..f439da721df8 100644 --- a/drivers/staging/iio/resolver/ad2s90.c +++ b/drivers/staging/iio/resolver/ad2s90.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-only /* * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90 * -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging:iio:ad2s90: Add device tree support
This patch adds device tree support to ad2s90 with standard
device tree id table.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 3e257ac46f7a..ff32ca76ca00 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
+static const struct of_device_id ad2s90_of_match[] = {
+ { .compatible = "adi,ad2s90", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ad2s90_of_match);
+
static const struct spi_device_id ad2s90_id[] = {
{ "ad2s90" },
{}
@@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
static struct spi_driver ad2s90_driver = {
.driver = {
.name = "ad2s90",
+ .of_match_table = of_match_ptr(ad2s90_of_match),
},
.probe = ad2s90_probe,
.id_table = ad2s90_id,
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 4908c8a95fad..54ad85bd9dc6 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -62,7 +62,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 9c168b7410d0..3e257ac46f7a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / 2^12 */
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 30 +++
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 8f79cccf4814..9c168b7410d0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,17 +34,29 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / 2^12 */
+ *val = 6283; /* mV */
+ *val2 = 12;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -55,7 +67,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..ba55de29ef36 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -36,11 +36,12 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
mutex_lock(>lock);
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code.
Note: The 'return ret' statement could be out of the 'if' block, but
this whole block will be moved up in the function in the patch:
'staging:iio:ad2s90: Move device registration to the end of probe'.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index ba55de29ef36..4908c8a95fad 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -86,7 +86,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 54ad85bd9dc6..8f79cccf4814 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -79,10 +79,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -93,7 +89,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v3: - Removed unconnected change in patch 1 (whitespace tidying up). - Added comment to patch 2's description regarding a code block that was moved in patch 4. - Corrected scale in patch 5, from 2Pi/(2^12 - 1) to 2Pi/2^12 and using IIO_VAL_FRACTIONAL_LOG2. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 53 ++- 1 file changed, 35 insertions(+), 18 deletions(-) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
On Sun, Oct 28, 2018 at 1:50 PM Jonathan Cameron wrote:
>
> On Fri, 26 Oct 2018 23:00:04 -0300
> Matheus Tavares wrote:
>
> > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
> > implements the relative read behavior at ad2s90_read_raw.
> >
> > Signed-off-by: Victor Colombo
> > Signed-off-by: Matheus Tavares
>
> Hi,
>
> A suggestion inline. This is a common case that we have infrastructure
> to simplify. + I think your scale factor is very slightly wrong.
>
> Jonathan
>
> > ---
> > drivers/staging/iio/resolver/ad2s90.c | 32 ++-
> > 1 file changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/iio/resolver/ad2s90.c
> > b/drivers/staging/iio/resolver/ad2s90.c
> > index b4a6a89c11b0..52b656875ed1 100644
> > --- a/drivers/staging/iio/resolver/ad2s90.c
> > +++ b/drivers/staging/iio/resolver/ad2s90.c
> > @@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
> > int ret;
> > struct ad2s90_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(>lock);
> > + switch (m) {
> > + case IIO_CHAN_INFO_SCALE:
> > + /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
> > + *val = 0;
> > + *val2 = 1534355;
> Definitely 2^12 - 1? That's a bit unusual if true as it would imply
> that 2^12 - 1 and 0 were the same value.
>
> Imagine a smaller version with on 2^2 bits so 0, 1, 2, 3
> Values of each are
>
> 0, M_PI/2, M_PI, 3*M_PI/2
>
> So the multiplier is 2*M_PI/(2^2) not 2*M_PI/(2^2 - 1)
> 1/2 vs 2/3 * M_PI
Oh, that makes a lot of sense! We used 2^12 - 1 here based on driver
drivers/iio/resolver/ad2s1200.c, whose resolution is also 12 bits, as
the ad2s90.c. Do you think this section is, perhaps, wrong on
ad2s1200.c too, or maybe there is some difference between these two
drivers that I didn't catch regarding the resolution?
Matheus.
> Now this is a very common case so we have the return type
> IIO_VAL_FRACTIONAL_LOG2 to give a more obvious and potentially
> more accurate representation.
>
> > + return IIO_VAL_INT_PLUS_NANO;
> > + case IIO_CHAN_INFO_RAW:
> > + mutex_lock(>lock);
> > +
> > + ret = spi_read(st->sdev, st->rx, 2);
> > + if (ret < 0) {
> > + mutex_unlock(>lock);
> > + return ret;
> > + }
> > +
> > + *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> >
> > - ret = spi_read(st->sdev, st->rx, 2);
> > - if (ret < 0) {
> > mutex_unlock(>lock);
> > - return ret;
> > - }
> >
> > - *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
> > -
> > - mutex_unlock(>lock);
> > + return IIO_VAL_INT;
> > + default:
> > + break;
> > + }
> >
> > - return IIO_VAL_INT;
> > + return -EINVAL;
> > }
> >
> > static const struct iio_info ad2s90_info = {
> > @@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
> > .type = IIO_ANGL,
> > .indexed = 1,
> > .channel = 0,
> > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > BIT(IIO_CHAN_INFO_SCALE),
> > };
> >
> > static int ad2s90_probe(struct spi_device *spi)
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
On 10/28/18 1:43 PM, Jonathan Cameron wrote:
On Fri, 26 Oct 2018 23:00:01 -0300
Matheus Tavares wrote:
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
I would have reordered this first to be before the iio_device_register call.
The reason being that it would avoid this comment.
Drop the return ret out of the block above and return ret unconditionally.
I don't mind too much as I know this is moving later, but I only know that
because of the earlier discussion ;) Few reviewers read the whole patch
set before responding to the early patches - it's just too much like hard
work. So if you can do things in an order that minimizes standard responses
then that's great.
Patch is fine though - could be solved by a comment in the intro that
says the code in question will move in patch X.
Ok! As a newcomer I'm not sure about the right way to add this comment.
Should I add it to the patch message or after the --- ? Also, how do I
reference patch X? Just giving the number of the patch in this series?
An alternative to adding the comment would be to drop the return ret,
and reinsert it in the next patch. Dosen't seem so good, to me, but
would avoid the problem you presented. What would be better?
Matheus.
Jonathan
return 0;
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
On 10/28/18 1:40 PM, Jonathan Cameron wrote:
On Fri, 26 Oct 2018 23:00:00 -0300
Matheus Tavares wrote:
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
Hi Matheus,
One quick process note is that it takes people a while to get around to
reviewing
a series, so whilst it's tempting to very quickly send out a fix the moment
someone points out something that needs fixing, it is perhaps better to wait
at least a few days to see if you can pick up a few more reviews before you
do a V2.
A few comments on this one inline. I think it can be done 'slightly'
(and I mean only slightly) nicer than the version you have. Result is the
same though.
Thanks,
Jonathan
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
Unconnected change. I'm not against the change in principle but please
group white space tidying up in it's own patch.
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
I'd actually prefer to keep the return path the same as before as then
it is easy (if the function gets more complex in future) to be sure
that all paths unlock the mutex.
Ok, got it! But then, in patch 5, when we add the switch for
IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_RAW, should I keep the goto and
label inside the switch case? I mean, should it be something like this:
switch (m) {
case IIO_CHAN_INFO_SCALE:
... // Does not use mutex
case IIO_CHAN_INFO_RAW:
mutex_lock(>lock);
ret = spi_read(st->sdev, st->rx, 2);
if (ret)
goto error_ret;
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
error_ret:
mutex_unlock(>lock);
return ret ? ret : IIO_VAL_INT;
default:
break;
}
Matheus
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
The 'standard' if slightly nasty way of doing this is:
return ret ? ret : IIO_VAL_INT;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
On Sun, Oct 28, 2018 at 1:52 PM Jonathan Cameron wrote: > > On Fri, 26 Oct 2018 22:59:59 -0300 > Matheus Tavares wrote: > > > This patch set adds scale info to ad2s90's single channel, improve > > error handling in it's functions and fix a possible race condition > > issue. > > > > The goal with this patch set is to address the points discussed in the > > mailing list in an effort to move ad2s90.c out of staging. > Thanks, > > A good series in general. A few suggested improvements. > If I haven't commented on a patch, usually it means I'm happy with it > and will pick it up with the rest of the series. > > Jonathan > Thanks for the review, Jonathan. We will address the necessary changes in v3! Matheus > > > > Changes in v2: > > - Added my S-o-B in patch 5. > > > > Matheus Tavares (5): > > staging:iio:ad2s90: Make read_raw return spi_read's error code > > staging:iio:ad2s90: Make probe handle spi_setup failure > > staging:iio:ad2s90: Remove always overwritten assignment > > staging:iio:ad2s90: Move device registration to the end of probe > > staging:iio:ad2s90: Check channel type at read_raw > > > > Victor Colombo (1): > > staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and > > read_raw > > > > drivers/staging/iio/resolver/ad2s90.c | 55 ++- > > 1 file changed, 37 insertions(+), 18 deletions(-) > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
On Fri, Oct 26, 2018 at 7:04 AM Dan Carpenter wrote: > > On Thu, Oct 25, 2018 at 09:45:11PM -0300, Matheus Tavares wrote: > > From: Victor Colombo > > > > This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and > > implements the relative read behavior at ad2s90_read_raw. > > > > Signed-off-by: Victor Colombo > > --- > > You should be adding your S-o-B here as well because the patch is > passing through your hands. Thanks for the review! I'll be sending a v2 with my S-o-B there. > regards, > dan carpenter > > -- > You received this message because you are subscribed to the Google Groups > "Kernel USP" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-usp+unsubscr...@googlegroups.com. > To post to this group, send email to kernel-...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/kernel-usp/20181026100422.lvz2avowd6ddix54%40mwanda. > For more options, visit https://groups.google.com/d/optout. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
From: Victor Colombo
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: iio: ad2s1210: fix 'assignment operator' style checks
This patch fixes all "Assignment operator '=' should be on the previous
line" checks found in ad2s1210.c with checkpatch.pl.
Signed-off-by: Matheus Tavares
---
Changes in v2: In v1, tabs were accidentally converted to whitespaces.
Now, tabs were preserved.
drivers/staging/iio/resolver/ad2s1210.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..d4b1c2c010f2 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -301,8 +301,8 @@ static ssize_t ad2s1210_store_control(struct device *dev,
"ad2s1210: write control register fail\n");
goto error_ret;
}
- st->resolution
- = ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+ st->resolution =
+ ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
if (st->pdata->gpioin) {
data = ad2s1210_read_resolution_pin(st);
if (data != st->resolution)
@@ -363,8 +363,8 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
dev_err(dev, "ad2s1210: setting resolution fail\n");
goto error_ret;
}
- st->resolution
- = ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+ st->resolution =
+ ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
if (st->pdata->gpioin) {
data = ad2s1210_read_resolution_pin(st);
if (data != st->resolution)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad2s1210: fix 'assignment operator' style checks
This patch fixes all "Assignment operator '=' should be on the previous
line" checks found in ad2s1210.c by checkpatch.pl.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s1210.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index ac13b99bd9cb..d4b1c2c010f2 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -301,8 +301,8 @@ static ssize_t ad2s1210_store_control(struct device *dev,
"ad2s1210: write control register fail\n");
goto error_ret;
}
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
if (st->pdata->gpioin) {
data = ad2s1210_read_resolution_pin(st);
if (data != st->resolution)
@@ -363,8 +363,8 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
dev_err(dev, "ad2s1210: setting resolution fail\n");
goto error_ret;
}
-st->resolution
-= ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
+st->resolution =
+ad2s1210_resolution_value[data & AD2S1210_SET_RESOLUTION];
if (st->pdata->gpioin) {
data = ad2s1210_read_resolution_pin(st);
if (data != st->resolution)
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
