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&m=154028966111330&w=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
This patch adds device tree support to ad2s90 with standard
device tree id table.
Signed-off-by: Matheus Tavares <[email protected]>
---
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
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 <[email protected]>
---
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(&spi->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 = 830000;
- spi->mode = SPI_MODE_3;
- ret = spi_setup(spi);
-
- if (ret < 0) {
- dev_err(&spi->dev, "spi_setup failed!\n");
- return ret;
- }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
--
2.18.0
From: Alexandru Ardelean <[email protected]>
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 830000Hz.
Signed-off-by: Alexandru Ardelean <[email protected]>
Signed-off-by: Matheus Tavares <[email protected]>
---
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 <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+/*
+ * 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 830000
+
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(&spi->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(&spi->dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
--
2.18.0
This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.
Signed-off-by: Matheus Tavares <[email protected]>
---
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 000000000000..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 830000
+ - 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 830000Hz.
+
+Example:
+resolver@0 {
+ compatible = "adi,ad2s90";
+ reg = <0>;
+ spi-max-frequency = <830000>;
+ spi-cpol;
+ spi-cpha;
+};
--
2.18.0
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 <[email protected]>
---
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 <linux/types.h>
#include <linux/mutex.h>
--
2.18.0
From: Victor Colombo <[email protected]>
Fix the checkpatch.pl issue:
"CHECK: struct mutex definition without comment".
Signed-off-by: Victor Colombo <[email protected]>
Signed-off-by: Matheus Tavares <[email protected]>
---
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 830000
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
Move ad2s90 resolver driver out of staging to the main tree.
Signed-off-by: Matheus Tavares <[email protected]>
Signed-off-by: Victor Colombo <[email protected]>
---
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 000000000000..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 <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+#include <linux/spi/spi.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/module.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/*
+ * 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 830000
+
+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(&st->lock);
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(&st->lock);
+ return ret;
+ }
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
+ mutex_unlock(&st->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(&spi->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(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ mutex_init(&st->lock);
+ st->sdev = spi;
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->info = &ad2s90_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = &ad2s90_chan;
+ indio_dev->num_channels = 1;
+ indio_dev->name = spi_get_device_id(spi)->name;
+
+ 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" },
+ {}
+};
+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,
+};
+module_spi_driver(ad2s90_driver);
+
+MODULE_AUTHOR("Graff Yang <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD2S90 Resolver to Digital SPI driver");
+MODULE_LICENSE("GPL v2");
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
diff --git a/drivers/staging/iio/resolver/ad2s90.c b/drivers/staging/iio/resolver/ad2s90.c
deleted file mode 100644
index f04dc5dede00..000000000000
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ /dev/null
@@ -1,131 +0,0 @@
-// 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 <linux/types.h>
-#include <linux/mutex.h>
-#include <linux/device.h>
-#include <linux/spi/spi.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
-#include <linux/module.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-
-/*
- * 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 830000
-
-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(&st->lock);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
- mutex_unlock(&st->lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(&st->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(&spi->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(&spi->dev, sizeof(*st));
- if (!indio_dev)
- return -ENOMEM;
- st = iio_priv(indio_dev);
- spi_set_drvdata(spi, indio_dev);
-
- mutex_init(&st->lock);
- st->sdev = spi;
- indio_dev->dev.parent = &spi->dev;
- indio_dev->info = &ad2s90_info;
- indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = &ad2s90_chan;
- indio_dev->num_channels = 1;
- indio_dev->name = spi_get_device_id(spi)->name;
-
- 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" },
- {}
-};
-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,
-};
-module_spi_driver(ad2s90_driver);
-
-MODULE_AUTHOR("Graff Yang <[email protected]>");
-MODULE_DESCRIPTION("Analog Devices AD2S90 Resolver to Digital SPI driver");
-MODULE_LICENSE("GPL v2");
--
2.18.0
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 <[email protected]>
> ---
> 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/
So,
+ .of_match_table = of_match_ptr(ad2s90_of_match),
becomes
> + .of_match_table = ad2s90_of_match,
> },
> .probe = ad2s90_probe,
> .id_table = ad2s90_id,
On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> From: Alexandru Ardelean <[email protected]>
>
> 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 830000Hz.
>
> Signed-off-by: Alexandru Ardelean <[email protected]>
I think you can use "Suggested-by:" instead.
But this is also fine.
> Signed-off-by: Matheus Tavares <[email protected]>
> ---
> 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 <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
>
> +/*
> + * 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 830000
> +
> 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(&spi->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(&spi->dev, sizeof(*st));
> if (!indio_dev)
> return -ENOMEM;
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 <[email protected]>
> ---
> 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 000000000000..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 830000
> + - 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
> +
> +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 830000Hz.
> +
> +Example:
> +resolver@0 {
> + compatible = "adi,ad2s90";
> + reg = <0>;
> + spi-max-frequency = <830000>;
> + spi-cpol;
> + spi-cpha;
> +};
On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
<[email protected]> 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 <[email protected]>
> > ---
> > 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 [email protected].
> To post to this group, send email to [email protected].
> 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.
On Mon, Nov 19, 2018 at 6:22 AM Ardelean, Alexandru
<[email protected]> 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 <[email protected]>
> > ---
> > 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 000000000000..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 830000
> > + - 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 830000Hz.
> > +
> > +Example:
> > +resolver@0 {
> > + compatible = "adi,ad2s90";
> > + reg = <0>;
> > + spi-max-frequency = <830000>;
> > + 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 [email protected].
> To post to this group, send email to [email protected].
> 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.
On Tue, 20 Nov 2018 21:59:19 -0200
Matheus Tavares Bernardino <[email protected]> wrote:
> On Mon, Nov 19, 2018 at 6:09 AM Ardelean, Alexandru
> <[email protected]> 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 <[email protected]>
> > > ---
> > > 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?
>
It's an odd one. The ACPI route allows you to basically provide
a temporary (these are really just for debugging purposes)
https://lwn.net/Articles/612062/
https://www.kernel.org/doc/Documentation/acpi/enumeration.txt
search for prp0001
So of_match_ptr is sort of obsolete, but conversely for some
types of driver it might never make sense to use the prp0001
route. For the stuff I play with in my day job for example
we can just change the firmware to do it right whenever we
need to and have appropriate ACPI ids to be able to issue
unique ones.
Jonathan
> 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 [email protected].
> > To post to this group, send email to [email protected].
> > 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.