Re: [PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
Hi,
On Thu, Feb 25, 2021 at 09:09:30AM +0200, Kalle Valo wrote:
> writes:
>
> > On 24/02/21 10:13 pm, Kalle Valo wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you
> >> know the content is safe
> >>
> >> Marcus Folkesson writes:
> >>
> >>> Write the value instead of reading it twice.
> >>>
> >>> Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for
> >>> function in wilc_sdio.c file")
> >>>
> >>> Signed-off-by: Marcus Folkesson
> >>> ---
> >>> drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> b/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> index 351ff909ab1c..e14b9fc2c67a 100644
> >>> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> @@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int
> >>> nint)
> >>> for (i = 0; (i < 3) && (nint > 0); i++, nint--)
> >>> reg |= BIT(i);
> >>>
> >>> - ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE,
> >>> );
> >>> + ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE,
> >>> reg);
> >>
> >> To me it looks like the bug existed before commit 5e63a598441a:
> >
> >
> > Yes, you are correct. The bug existed from commit c5c77ba18ea6:
> >
> > https://git.kernel.org/linus/c5c77ba18ea6
>
> So the fixes tag should be:
>
> Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
You are right.
>
> I can change that during commit, ok?
Please do, thanks!
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
Write the value instead of reading it twice.
Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for function in
wilc_sdio.c file")
Signed-off-by: Marcus Folkesson
---
drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 351ff909ab1c..e14b9fc2c67a 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int nint)
for (i = 0; (i < 3) && (nint > 0); i++, nint--)
reg |= BIT(i);
- ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE, );
+ ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE, reg);
if (ret) {
dev_err(>dev,
"Failed write reg (%08x)...\n",
--
2.30.0
Re: [PATCH 1/2] watchdog: sama5d4: fix timeout-sec usage
On Fri, Sep 14, 2018 at 12:13:38PM +0200, Romain Izard wrote:
> When using watchdog_init_timeout to update the default timeout value,
> an error means that there is no "timeout-sec" in the relevant device
> tree node.
>
> This should not prevent binding of the driver to the device.
>
> Fixes: 976932e40036 ("watchdog: sama5d4: make use of timeout-secs provided in
> devicetree")
> Signed-off-by: Romain Izard
Reviewed-by: Marcus Folkesson
Re: [PATCH 1/2] watchdog: sama5d4: fix timeout-sec usage
On Fri, Sep 14, 2018 at 12:13:38PM +0200, Romain Izard wrote:
> When using watchdog_init_timeout to update the default timeout value,
> an error means that there is no "timeout-sec" in the relevant device
> tree node.
>
> This should not prevent binding of the driver to the device.
>
> Fixes: 976932e40036 ("watchdog: sama5d4: make use of timeout-secs provided in
> devicetree")
> Signed-off-by: Romain Izard
Reviewed-by: Marcus Folkesson
[PATCH] iio: dac: ti-dac5571: make vref regulator optional
The `vref` regulator is declared as optional in the device-tree binding,
but the driver does require it.
Go for the device-tree binding and make the `vref` regulator optional.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index f6dcd8bce2b0..bf21cc312096 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -251,6 +251,9 @@ static int dac5571_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
+ if (!data->vref)
+ return -EOPNOTSUPP;
+
ret = regulator_get_voltage(data->vref);
if (ret < 0)
return ret;
@@ -335,13 +338,21 @@ static int dac5571_probe(struct i2c_client *client,
indio_dev->num_channels = spec->num_channels;
data->spec = spec;
- data->vref = devm_regulator_get(dev, "vref");
- if (IS_ERR(data->vref))
- return PTR_ERR(data->vref);
+ data->vref = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(data->vref)) {
+ if (PTR_ERR(data->vref) == -ENODEV) {
+ data->vref = NULL;
+ } else {
+ dev_err(dev, "failed to get regulator (%ld)\n",
+ PTR_ERR(data->vref));
+ return PTR_ERR(data->vref);
+ }
- ret = regulator_enable(data->vref);
- if (ret < 0)
- return ret;
+ } else {
+ ret = regulator_enable(data->vref);
+ if (ret)
+ return ret;
+ }
mutex_init(>lock);
@@ -373,7 +384,9 @@ static int dac5571_probe(struct i2c_client *client,
return 0;
err:
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
+
return ret;
}
@@ -383,7 +396,8 @@ static int dac5571_remove(struct i2c_client *i2c)
struct dac5571_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
return 0;
}
--
2.18.0
[PATCH] iio: dac: ti-dac5571: provide of_match_table to driver
Use the created list of of_device_id's as a match table.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index e39d1e901353..f6dcd8bce2b0 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -421,6 +421,7 @@ MODULE_DEVICE_TABLE(i2c, dac5571_id);
static struct i2c_driver dac5571_driver = {
.driver = {
.name = "ti-dac5571",
+ .of_match_table = of_match_ptr(dac5571_of_id),
},
.probe= dac5571_probe,
.remove = dac5571_remove,
--
2.18.0
[PATCH] iio: dac: mcp4922: fix error handling in mcp4922_write_raw
Do not try to write negative values and make sure that the write goes well.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4922.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/dac/mcp4922.c b/drivers/iio/dac/mcp4922.c
index bf9aa3fc0534..b5190d1dae8e 100644
--- a/drivers/iio/dac/mcp4922.c
+++ b/drivers/iio/dac/mcp4922.c
@@ -94,17 +94,22 @@ static int mcp4922_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct mcp4922_state *state = iio_priv(indio_dev);
+ int ret;
if (val2 != 0)
return -EINVAL;
switch (mask) {
case IIO_CHAN_INFO_RAW:
- if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
return -EINVAL;
val <<= chan->scan_type.shift;
- state->value[chan->channel] = val;
- return mcp4922_spi_write(state, chan->channel, val);
+
+ ret = mcp4922_spi_write(state, chan->channel, val);
+ if (!ret)
+ state->value[chan->channel] = val;
+ return ret;
+
default:
return -EINVAL;
}
--
2.18.0
[PATCH] iio: dac: ti-dac5571: make vref regulator optional
The `vref` regulator is declared as optional in the device-tree binding,
but the driver does require it.
Go for the device-tree binding and make the `vref` regulator optional.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index f6dcd8bce2b0..bf21cc312096 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -251,6 +251,9 @@ static int dac5571_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
+ if (!data->vref)
+ return -EOPNOTSUPP;
+
ret = regulator_get_voltage(data->vref);
if (ret < 0)
return ret;
@@ -335,13 +338,21 @@ static int dac5571_probe(struct i2c_client *client,
indio_dev->num_channels = spec->num_channels;
data->spec = spec;
- data->vref = devm_regulator_get(dev, "vref");
- if (IS_ERR(data->vref))
- return PTR_ERR(data->vref);
+ data->vref = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(data->vref)) {
+ if (PTR_ERR(data->vref) == -ENODEV) {
+ data->vref = NULL;
+ } else {
+ dev_err(dev, "failed to get regulator (%ld)\n",
+ PTR_ERR(data->vref));
+ return PTR_ERR(data->vref);
+ }
- ret = regulator_enable(data->vref);
- if (ret < 0)
- return ret;
+ } else {
+ ret = regulator_enable(data->vref);
+ if (ret)
+ return ret;
+ }
mutex_init(>lock);
@@ -373,7 +384,9 @@ static int dac5571_probe(struct i2c_client *client,
return 0;
err:
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
+
return ret;
}
@@ -383,7 +396,8 @@ static int dac5571_remove(struct i2c_client *i2c)
struct dac5571_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
return 0;
}
--
2.18.0
[PATCH] iio: dac: ti-dac5571: provide of_match_table to driver
Use the created list of of_device_id's as a match table.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index e39d1e901353..f6dcd8bce2b0 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -421,6 +421,7 @@ MODULE_DEVICE_TABLE(i2c, dac5571_id);
static struct i2c_driver dac5571_driver = {
.driver = {
.name = "ti-dac5571",
+ .of_match_table = of_match_ptr(dac5571_of_id),
},
.probe= dac5571_probe,
.remove = dac5571_remove,
--
2.18.0
[PATCH] iio: dac: mcp4922: fix error handling in mcp4922_write_raw
Do not try to write negative values and make sure that the write goes well.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4922.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/dac/mcp4922.c b/drivers/iio/dac/mcp4922.c
index bf9aa3fc0534..b5190d1dae8e 100644
--- a/drivers/iio/dac/mcp4922.c
+++ b/drivers/iio/dac/mcp4922.c
@@ -94,17 +94,22 @@ static int mcp4922_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct mcp4922_state *state = iio_priv(indio_dev);
+ int ret;
if (val2 != 0)
return -EINVAL;
switch (mask) {
case IIO_CHAN_INFO_RAW:
- if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
return -EINVAL;
val <<= chan->scan_type.shift;
- state->value[chan->channel] = val;
- return mcp4922_spi_write(state, chan->channel, val);
+
+ ret = mcp4922_spi_write(state, chan->channel, val);
+ if (!ret)
+ state->value[chan->channel] = val;
+ return ret;
+
default:
return -EINVAL;
}
--
2.18.0
Re: [PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
On Tue, Aug 21, 2018 at 09:31:25PM +0200, Marcus Folkesson wrote:
> LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
> with eight individual channels.
>
> Signed-off-by: Marcus Folkesson
Rob, sorry I missed your tag.
Reviewed-by: Rob Herring
> ---
>
> Notes:
> v2:
> - rename file, ltc166x -> ltc1660
>
> .../devicetree/bindings/iio/dac/ltc1660.txt | 21
> +
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> new file mode 100644
> index ..c5b5f22d6c64
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> @@ -0,0 +1,21 @@
> +* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
> +
> +Required properties:
> + - compatible: Must be one of the following:
> + "lltc,ltc1660"
> + "lltc,ltc1665"
> + - reg: SPI chip select number for the device
> + - vref-supply: Phandle to the voltage reference supply
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> + Documentation/devicetree/bindings/spi/spi-bus.txt.
> + Max frequency for this chip is 5 MHz.
> +
> +Example:
> +dac@0 {
> + compatible = "lltc,ltc1660";
> + reg = <0>;
> + spi-max-frequency = <500>;
> + vref-supply = <_reg>;
> +};
> --
> 2.11.0.rc2
>
Re: [PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
On Tue, Aug 21, 2018 at 09:31:25PM +0200, Marcus Folkesson wrote:
> LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
> with eight individual channels.
>
> Signed-off-by: Marcus Folkesson
Rob, sorry I missed your tag.
Reviewed-by: Rob Herring
> ---
>
> Notes:
> v2:
> - rename file, ltc166x -> ltc1660
>
> .../devicetree/bindings/iio/dac/ltc1660.txt | 21
> +
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> new file mode 100644
> index ..c5b5f22d6c64
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> @@ -0,0 +1,21 @@
> +* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
> +
> +Required properties:
> + - compatible: Must be one of the following:
> + "lltc,ltc1660"
> + "lltc,ltc1665"
> + - reg: SPI chip select number for the device
> + - vref-supply: Phandle to the voltage reference supply
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> + Documentation/devicetree/bindings/spi/spi-bus.txt.
> + Max frequency for this chip is 5 MHz.
> +
> +Example:
> +dac@0 {
> + compatible = "lltc,ltc1660";
> + reg = <0>;
> + spi-max-frequency = <500>;
> + vref-supply = <_reg>;
> +};
> --
> 2.11.0.rc2
>
[PATCH v2 3/3] MAINTAINERS: add entry for ltc1660 DAC driver
Add entry for ltc1660 DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- Notes: v2: - rename enumerated files, ltc166x* -> ltc1660* MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..3db1edaf68bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC1660 DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc1660.txt +F: drivers/iio/dac/ltc1660.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 3/3] MAINTAINERS: add entry for ltc1660 DAC driver
Add entry for ltc1660 DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- Notes: v2: - rename enumerated files, ltc166x* -> ltc1660* MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..3db1edaf68bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC1660 DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc1660.txt +F: drivers/iio/dac/ltc1660.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename file, ltc166x -> ltc1660
.../devicetree/bindings/iio/dac/ltc1660.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH v2 1/3] iio: dac: add support for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename all instances of ltc166x to ltc1660
- read regulator value "in-place" instead of cache voltage
- fix error handling in ltc1660_write_raw
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc1660.c | 250 ++
3 files changed, 261 insertions(+)
create mode 100644 drivers/iio/dac/ltc1660.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..cbee80bd111e 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC1660
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc1660.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..e446eeb09c85 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC1660) += ltc1660.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc1660.c b/drivers/iio/dac/ltc1660.c
new file mode 100644
index ..10866838c72a
--- /dev/null
+++ b/drivers/iio/dac/ltc1660.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC1660_REG_WAKE 0x0
+#define LTC1660_REG_DAC_A 0x1
+#define LTC1660_REG_DAC_B 0x2
+#define LTC1660_REG_DAC_C 0x3
+#define LTC1660_REG_DAC_D 0x4
+#define LTC1660_REG_DAC_E 0x5
+#define LTC1660_REG_DAC_F 0x6
+#define LTC1660_REG_DAC_G 0x7
+#define LTC1660_REG_DAC_H 0x8
+#define LTC1660_REG_SLEEP 0xe
+
+#define LTC1660_NUM_CHANNELS 8
+
+static const struct regmap_config ltc1660_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc1660_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc1660_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC1660_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc1660_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = regulator_get_voltage(priv->vref_reg);
+ if (*val < 0) {
+ dev_err(>spi->dev, "failed to read vref
regulator: %d\n",
+ *val);
+ return *val;
+ }
+
+ /* Convert to mV */
+ *val /= 1000;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc1660_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
+ return -EINVAL;
+
+ ret = regmap_write(priv->regmap, chan->channel,
+ (val << chan->scan_type.shift));
+ if (!ret)
+ priv->value[chan->channel] = val;
+
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC1660_CHAN(chan, bits) {
[PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename file, ltc166x -> ltc1660
.../devicetree/bindings/iio/dac/ltc1660.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH v2 1/3] iio: dac: add support for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename all instances of ltc166x to ltc1660
- read regulator value "in-place" instead of cache voltage
- fix error handling in ltc1660_write_raw
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc1660.c | 250 ++
3 files changed, 261 insertions(+)
create mode 100644 drivers/iio/dac/ltc1660.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..cbee80bd111e 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC1660
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc1660.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..e446eeb09c85 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC1660) += ltc1660.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc1660.c b/drivers/iio/dac/ltc1660.c
new file mode 100644
index ..10866838c72a
--- /dev/null
+++ b/drivers/iio/dac/ltc1660.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC1660_REG_WAKE 0x0
+#define LTC1660_REG_DAC_A 0x1
+#define LTC1660_REG_DAC_B 0x2
+#define LTC1660_REG_DAC_C 0x3
+#define LTC1660_REG_DAC_D 0x4
+#define LTC1660_REG_DAC_E 0x5
+#define LTC1660_REG_DAC_F 0x6
+#define LTC1660_REG_DAC_G 0x7
+#define LTC1660_REG_DAC_H 0x8
+#define LTC1660_REG_SLEEP 0xe
+
+#define LTC1660_NUM_CHANNELS 8
+
+static const struct regmap_config ltc1660_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc1660_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc1660_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC1660_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc1660_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = regulator_get_voltage(priv->vref_reg);
+ if (*val < 0) {
+ dev_err(>spi->dev, "failed to read vref
regulator: %d\n",
+ *val);
+ return *val;
+ }
+
+ /* Convert to mV */
+ *val /= 1000;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc1660_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
+ return -EINVAL;
+
+ ret = regmap_write(priv->regmap, chan->channel,
+ (val << chan->scan_type.shift));
+ if (!ret)
+ priv->value[chan->channel] = val;
+
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC1660_CHAN(chan, bits) {
Re: [PATCH v6 1/2] iio: light: Add support for vishay vcnl4035
vm_request_threaded_irq(>client->dev, data->client->irq,
> + NULL, vcnl4035_drdy_irq_thread,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + VCNL4035_IRQ_NAME, indio_dev);
> + if (ret < 0) {
> + dev_err(>client->dev, "request irq %d for trigger0
> failed\n",
> + data->client->irq);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vcnl4035_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct vcnl4035_data *data;
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, _regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(>dev, "regmap_init failed!\n");
> + return PTR_ERR(regmap);
> + }
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + data->regmap = regmap;
> +
> + indio_dev->dev.parent = >dev;
> + indio_dev->info = _info;
> + indio_dev->name = VCNL4035_DRV_NAME;
> + indio_dev->channels = vcnl4035_channels;
> + indio_dev->num_channels = ARRAY_SIZE(vcnl4035_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = vcnl4035_init(data);
> + if (ret < 0) {
> + dev_err(>dev, "vcnl4035 chip init failed\n");
> + return ret;
> + }
> +
> + if (client->irq > 0) {
> + ret = vcnl4035_probe_trigger(indio_dev);
> + if (ret < 0) {
> + dev_err(>dev, "vcnl4035 unable init trigger\n");
> + goto fail_poweroff;
> + }
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto fail_poweroff;
> +
> + ret = pm_runtime_set_active(>dev);
> + if (ret < 0)
> + goto fail_unregister;
> +
> + pm_runtime_enable(>dev);
> + pm_runtime_set_autosuspend_delay(>dev, VCNL4035_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(>dev);
> +
> + return 0;
> +
> +fail_unregister:
> + iio_device_unregister(indio_dev);
> +fail_poweroff:
> + vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + return ret;
> +}
> +
> +static int vcnl4035_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> + iio_device_unregister(indio_dev);
> +
> + pm_runtime_dont_use_autosuspend(>dev);
> + pm_runtime_disable(>dev);
> + pm_runtime_set_suspended(>dev);
> + pm_runtime_put_noidle(>dev);
> +
> + return vcnl4035_set_als_power_state(iio_priv(indio_dev),
> + VCNL4035_MODE_ALS_DISABLE);
> +}
> +
> +#ifdef CONFIG_PM
> +static int __maybe_unused vcnl4035_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + regcache_mark_dirty(data->regmap);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused vcnl4035_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + regcache_sync(data->regmap);
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + /* wait for 1 ALS integration cycle */
> + msleep(data->als_it_val * 100);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops vcnl4035_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(vcnl4035_runtime_suspend,
> +vcnl4035_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id vcnl4035_of_match[] = {
> + { .compatible = "vishay,vcnl4035", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, vcnl4035_of_match);
> +
> +static struct i2c_driver vcnl4035_driver = {
> + .driver = {
> + .name = VCNL4035_DRV_NAME,
> + .pm = _pm_ops,
> + .of_match_table = of_match_ptr(vcnl4035_of_match),
Please correct me if I'm wrong here.
Use of_match_ptr() and not provide an ACPI match table will not make
this driver work on ACPI systems.
The reason is that the ACPI match function will fall back on
of_match_table if no acpi_match_table is provided, and the
of_match_ptr() macro will simple evaluate to NULL if not CONFIG_OF
is set.
In other words, I think we should change this to just
.of_match_table = _of_match,
> + },
> + .probe = vcnl4035_probe,
> + .remove = vcnl4035_remove,
> +};
> +
> +module_i2c_driver(vcnl4035_driver);
> +
> +MODULE_AUTHOR("Parthiban Nallathambi ");
> +MODULE_DESCRIPTION("VCNL4035 Ambient Light Sensor driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v6 1/2] iio: light: Add support for vishay vcnl4035
vm_request_threaded_irq(>client->dev, data->client->irq,
> + NULL, vcnl4035_drdy_irq_thread,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + VCNL4035_IRQ_NAME, indio_dev);
> + if (ret < 0) {
> + dev_err(>client->dev, "request irq %d for trigger0
> failed\n",
> + data->client->irq);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vcnl4035_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct vcnl4035_data *data;
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, _regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(>dev, "regmap_init failed!\n");
> + return PTR_ERR(regmap);
> + }
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + data->regmap = regmap;
> +
> + indio_dev->dev.parent = >dev;
> + indio_dev->info = _info;
> + indio_dev->name = VCNL4035_DRV_NAME;
> + indio_dev->channels = vcnl4035_channels;
> + indio_dev->num_channels = ARRAY_SIZE(vcnl4035_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = vcnl4035_init(data);
> + if (ret < 0) {
> + dev_err(>dev, "vcnl4035 chip init failed\n");
> + return ret;
> + }
> +
> + if (client->irq > 0) {
> + ret = vcnl4035_probe_trigger(indio_dev);
> + if (ret < 0) {
> + dev_err(>dev, "vcnl4035 unable init trigger\n");
> + goto fail_poweroff;
> + }
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto fail_poweroff;
> +
> + ret = pm_runtime_set_active(>dev);
> + if (ret < 0)
> + goto fail_unregister;
> +
> + pm_runtime_enable(>dev);
> + pm_runtime_set_autosuspend_delay(>dev, VCNL4035_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(>dev);
> +
> + return 0;
> +
> +fail_unregister:
> + iio_device_unregister(indio_dev);
> +fail_poweroff:
> + vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + return ret;
> +}
> +
> +static int vcnl4035_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> + iio_device_unregister(indio_dev);
> +
> + pm_runtime_dont_use_autosuspend(>dev);
> + pm_runtime_disable(>dev);
> + pm_runtime_set_suspended(>dev);
> + pm_runtime_put_noidle(>dev);
> +
> + return vcnl4035_set_als_power_state(iio_priv(indio_dev),
> + VCNL4035_MODE_ALS_DISABLE);
> +}
> +
> +#ifdef CONFIG_PM
> +static int __maybe_unused vcnl4035_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + regcache_mark_dirty(data->regmap);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused vcnl4035_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + regcache_sync(data->regmap);
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + /* wait for 1 ALS integration cycle */
> + msleep(data->als_it_val * 100);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops vcnl4035_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(vcnl4035_runtime_suspend,
> +vcnl4035_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id vcnl4035_of_match[] = {
> + { .compatible = "vishay,vcnl4035", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, vcnl4035_of_match);
> +
> +static struct i2c_driver vcnl4035_driver = {
> + .driver = {
> + .name = VCNL4035_DRV_NAME,
> + .pm = _pm_ops,
> + .of_match_table = of_match_ptr(vcnl4035_of_match),
Please correct me if I'm wrong here.
Use of_match_ptr() and not provide an ACPI match table will not make
this driver work on ACPI systems.
The reason is that the ACPI match function will fall back on
of_match_table if no acpi_match_table is provided, and the
of_match_ptr() macro will simple evaluate to NULL if not CONFIG_OF
is set.
In other words, I think we should change this to just
.of_match_table = _of_match,
> + },
> + .probe = vcnl4035_probe,
> + .remove = vcnl4035_remove,
> +};
> +
> +module_i2c_driver(vcnl4035_driver);
> +
> +MODULE_AUTHOR("Parthiban Nallathambi ");
> +MODULE_DESCRIPTION("VCNL4035 Ambient Light Sensor driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:29:43PM +0100, Jonathan Cameron wrote: > On Sun, 19 Aug 2018 21:17:51 +0200 > Marcus Folkesson wrote: > > > On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > > > On Wed, 8 Aug 2018 10:09:15 +0200 > > > Marcus Folkesson wrote: > > > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > > > > > Co-Developed-by: Kent Gustavsson > > > checkpatch points out.. > > > Co-developed-by is the 'official' form of the tag. I'll fixup. > > > (first time I've seen one of these ;) > > > > Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. > > I tried to fix it, but Perl is just such a terrible language that I > > considered it impossible. > > The version in mainline seems to be aware, it just wants > Co-developed-by not Co-Developed-by! > > (lower case d) I just noticed that the tag was renamed from Co-Developed-by to Co-developed-by in e474785a12b46230ecf9f3663166b0de1175bcdc. I must have been reading the documentation from an older tree... /Marcus signature.asc Description: PGP signature
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:29:43PM +0100, Jonathan Cameron wrote: > On Sun, 19 Aug 2018 21:17:51 +0200 > Marcus Folkesson wrote: > > > On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > > > On Wed, 8 Aug 2018 10:09:15 +0200 > > > Marcus Folkesson wrote: > > > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > > > > > Co-Developed-by: Kent Gustavsson > > > checkpatch points out.. > > > Co-developed-by is the 'official' form of the tag. I'll fixup. > > > (first time I've seen one of these ;) > > > > Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. > > I tried to fix it, but Perl is just such a terrible language that I > > considered it impossible. > > The version in mainline seems to be aware, it just wants > Co-developed-by not Co-Developed-by! > > (lower case d) I just noticed that the tag was renamed from Co-Developed-by to Co-developed-by in e474785a12b46230ecf9f3663166b0de1175bcdc. I must have been reading the documentation from an older tree... /Marcus signature.asc Description: PGP signature
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > On Wed, 8 Aug 2018 10:09:15 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Co-Developed-by: Kent Gustavsson > checkpatch points out.. > Co-developed-by is the 'official' form of the tag. I'll fixup. > (first time I've seen one of these ;) Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. I tried to fix it, but Perl is just such a terrible language that I considered it impossible. > > > Signed-off-by: Kent Gustavsson > > Signed-off-by: Marcus Folkesson > Applied to the togreg branch of iio.git and pushed out as testing for the > autobuilders to play with it. > > A few really trivial comments inline. Also some slightly odd alignment > I'll fix up that I haven't noted, but not every parameter second line > lines up with the opening brackets correctly. Thank you. > > Jonathan > > > --- > > > > Notes: > > v4: > > - remove defined(CONFIG_OF) and of_match_ptr() macro > > - do not check adc->clki as clock api is NULL-aware > > - add Kent as co-developer > > v3: > > - rename adc_clk to clki > > - add error handling/cleanup for clock > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 361 > > ++ > > 3 files changed, 372 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..540c8fbcce89 > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,361 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > On Wed, 8 Aug 2018 10:09:15 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Co-Developed-by: Kent Gustavsson > checkpatch points out.. > Co-developed-by is the 'official' form of the tag. I'll fixup. > (first time I've seen one of these ;) Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. I tried to fix it, but Perl is just such a terrible language that I considered it impossible. > > > Signed-off-by: Kent Gustavsson > > Signed-off-by: Marcus Folkesson > Applied to the togreg branch of iio.git and pushed out as testing for the > autobuilders to play with it. > > A few really trivial comments inline. Also some slightly odd alignment > I'll fix up that I haven't noted, but not every parameter second line > lines up with the opening brackets correctly. Thank you. > > Jonathan > > > --- > > > > Notes: > > v4: > > - remove defined(CONFIG_OF) and of_match_ptr() macro > > - do not check adc->clki as clock api is NULL-aware > > - add Kent as co-developer > > v3: > > - rename adc_clk to clki > > - add error handling/cleanup for clock > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 361 > > ++ > > 3 files changed, 372 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..540c8fbcce89 > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,361 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH
Re: [PATCH 1/3] iio: dac: add support for ltc166x
Hi Jonathan,
Thanks for your comments!
On Sun, Aug 19, 2018 at 05:38:50PM +0100, Jonathan Cameron wrote:
> On Sat, 11 Aug 2018 22:02:24 +0200
> Marcus Folkesson wrote:
>
> > LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
> > (DAC) with eight individual channels.
> >
> > Signed-off-by: Marcus Folkesson
>
> So first rule we try to stick to that this breaks is never use wild cards
> in drivers. You probably wouldn't believe how often this has gone wrong with
> a manufacturer deciding to use other values that wild card covers...
Hah, I can imagine. I will switch do use ltc1660 all over the place.
>
> Please pick a part and name everything after that. Either one is fine.
>
> A few really minor things inline. Nice little driver.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/iio/dac/Kconfig | 10 ++
> > drivers/iio/dac/Makefile | 1 +
> > drivers/iio/dac/ltc166x.c | 244
> > ++
> > 3 files changed, 255 insertions(+)
> > create mode 100644 drivers/iio/dac/ltc166x.c
> >
> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> > index 76db0768e454..04cfa6bb9dc1 100644
> > --- a/drivers/iio/dac/Kconfig
> > +++ b/drivers/iio/dac/Kconfig
> > @@ -120,6 +120,16 @@ config AD5624R_SPI
> > Say yes here to build support for Analog Devices AD5624R, AD5644R and
> > AD5664R converters (DAC). This driver uses the common SPI interface.
> >
> > +config LTC166X
> > + tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
> > + depends on SPI
> > + help
> > + Say yes here to build support for Linear Technology
> > + LTC1660 and LTC1665 Digital to Analog Converters.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called ltc166x.
> > +
> > config LTC2632
> > tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> > depends on SPI
> > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> > index 81e710ed7491..380749c87c26 100644
> > --- a/drivers/iio/dac/Makefile
> > +++ b/drivers/iio/dac/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
> > obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
> > obj-$(CONFIG_DS4424) += ds4424.o
> > obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> > +obj-$(CONFIG_LTC166X) += ltc166x.o
> > obj-$(CONFIG_LTC2632) += ltc2632.o
> > obj-$(CONFIG_M62332) += m62332.o
> > obj-$(CONFIG_MAX517) += max517.o
> > diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
> > new file mode 100644
> > index ..0031f2b50f14
> > --- /dev/null
> > +++ b/drivers/iio/dac/ltc166x.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
> > + *
> > + * Copyright (C) 2018 Marcus Folkesson
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define LTC166X_REG_WAKE 0x0
> > +#define LTC166X_REG_DAC_A 0x1
> > +#define LTC166X_REG_DAC_B 0x2
> > +#define LTC166X_REG_DAC_C 0x3
> > +#define LTC166X_REG_DAC_D 0x4
> > +#define LTC166X_REG_DAC_E 0x5
> > +#define LTC166X_REG_DAC_F 0x6
> > +#define LTC166X_REG_DAC_G 0x7
> > +#define LTC166X_REG_DAC_H 0x8
> > +#define LTC166X_REG_SLEEP 0xe
> > +
> > +#define LTC166X_NUM_CHANNELS 8
> > +
> > +static const struct regmap_config ltc166x_regmap_config = {
> > + .reg_bits = 4,
> > + .val_bits = 12,
> > +};
> > +
> > +enum ltc166x_supported_device_ids {
> > + ID_LTC1660,
> > + ID_LTC1665,
> > +};
> > +
> > +struct ltc166x_priv {
> > + struct spi_device *spi;
> > + struct regmap *regmap;
> > + struct regulator *vref_reg;
> > + unsigned int value[LTC166X_NUM_CHANNELS];
> > + unsigned int vref_mv;
> > +};
> > +
> > +static int ltc166x_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val,
> > + int *val2,
> > + long mask)
> > +{
> > + struct ltc166x_priv *priv = iio_priv(indio_dev);
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + *val = priv->value[chan->channel];
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
&g
Re: [PATCH 1/3] iio: dac: add support for ltc166x
Hi Jonathan,
Thanks for your comments!
On Sun, Aug 19, 2018 at 05:38:50PM +0100, Jonathan Cameron wrote:
> On Sat, 11 Aug 2018 22:02:24 +0200
> Marcus Folkesson wrote:
>
> > LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
> > (DAC) with eight individual channels.
> >
> > Signed-off-by: Marcus Folkesson
>
> So first rule we try to stick to that this breaks is never use wild cards
> in drivers. You probably wouldn't believe how often this has gone wrong with
> a manufacturer deciding to use other values that wild card covers...
Hah, I can imagine. I will switch do use ltc1660 all over the place.
>
> Please pick a part and name everything after that. Either one is fine.
>
> A few really minor things inline. Nice little driver.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/iio/dac/Kconfig | 10 ++
> > drivers/iio/dac/Makefile | 1 +
> > drivers/iio/dac/ltc166x.c | 244
> > ++
> > 3 files changed, 255 insertions(+)
> > create mode 100644 drivers/iio/dac/ltc166x.c
> >
> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> > index 76db0768e454..04cfa6bb9dc1 100644
> > --- a/drivers/iio/dac/Kconfig
> > +++ b/drivers/iio/dac/Kconfig
> > @@ -120,6 +120,16 @@ config AD5624R_SPI
> > Say yes here to build support for Analog Devices AD5624R, AD5644R and
> > AD5664R converters (DAC). This driver uses the common SPI interface.
> >
> > +config LTC166X
> > + tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
> > + depends on SPI
> > + help
> > + Say yes here to build support for Linear Technology
> > + LTC1660 and LTC1665 Digital to Analog Converters.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called ltc166x.
> > +
> > config LTC2632
> > tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> > depends on SPI
> > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> > index 81e710ed7491..380749c87c26 100644
> > --- a/drivers/iio/dac/Makefile
> > +++ b/drivers/iio/dac/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
> > obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
> > obj-$(CONFIG_DS4424) += ds4424.o
> > obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> > +obj-$(CONFIG_LTC166X) += ltc166x.o
> > obj-$(CONFIG_LTC2632) += ltc2632.o
> > obj-$(CONFIG_M62332) += m62332.o
> > obj-$(CONFIG_MAX517) += max517.o
> > diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
> > new file mode 100644
> > index ..0031f2b50f14
> > --- /dev/null
> > +++ b/drivers/iio/dac/ltc166x.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
> > + *
> > + * Copyright (C) 2018 Marcus Folkesson
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define LTC166X_REG_WAKE 0x0
> > +#define LTC166X_REG_DAC_A 0x1
> > +#define LTC166X_REG_DAC_B 0x2
> > +#define LTC166X_REG_DAC_C 0x3
> > +#define LTC166X_REG_DAC_D 0x4
> > +#define LTC166X_REG_DAC_E 0x5
> > +#define LTC166X_REG_DAC_F 0x6
> > +#define LTC166X_REG_DAC_G 0x7
> > +#define LTC166X_REG_DAC_H 0x8
> > +#define LTC166X_REG_SLEEP 0xe
> > +
> > +#define LTC166X_NUM_CHANNELS 8
> > +
> > +static const struct regmap_config ltc166x_regmap_config = {
> > + .reg_bits = 4,
> > + .val_bits = 12,
> > +};
> > +
> > +enum ltc166x_supported_device_ids {
> > + ID_LTC1660,
> > + ID_LTC1665,
> > +};
> > +
> > +struct ltc166x_priv {
> > + struct spi_device *spi;
> > + struct regmap *regmap;
> > + struct regulator *vref_reg;
> > + unsigned int value[LTC166X_NUM_CHANNELS];
> > + unsigned int vref_mv;
> > +};
> > +
> > +static int ltc166x_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val,
> > + int *val2,
> > + long mask)
> > +{
> > + struct ltc166x_priv *priv = iio_priv(indio_dev);
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + *val = priv->value[chan->channel];
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCALE:
&g
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi Gutavo,
Sorry for the delay.
On Wed, Aug 15, 2018 at 12:50:10PM -0500, Gustavo A. R. Silva wrote:
> Hi Marcus,
>
> On 8/15/18 12:27 PM, Marcus Folkesson wrote:
> > Hi,
> >
> > On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> >> where we are expecting to fall through.
> >>
> >> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> >> Signed-off-by: Gustavo A. R. Silva
> >> ---
> >> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> index 063e89e..d609654 100644
> >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> >> platform_device *pdev)
> >>switch (i) {
> >>case X:
> >>ec_accel_channels[X].scan_index = Y;
> >> + /* fall through */
> >>case Y:
> >>ec_accel_channels[Y].scan_index = X;
> >> + /* fall through */
> >>case Z:
> >>ec_accel_channels[Z].scan_index = Z;
> >>}
> >
> > Hum, I'm not sure we are supposed to fall through here, even if it does
> > not hurt to do so.
>
> Yeah. You're right. It doesn't hurt but is actually redundant. I think
> the original intention was to break instead of falling through.
>
> > I even think we can remove the switch and put that outside the for-loop,
> > e.g:
> >
> > ec_accel_channels[X].scan_index = Y;
> > ec_accel_channels[Y].scan_index = X;
> > ec_accel_channels[Z].scan_index = Z;
> >
> > for (i = X ; i < MAX_AXIS; i++) {
> > if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> > state->sign[i] = -1;
> > else
> > state->sign[i] = 1;
> > }
> >
>
> I like this, but the code clearly depends on MAX_AXIS. So, if MAX_AXIS
> will be always 3, then the change you suggest is just fine. Otherwise,
> it seems that adding a break to each case is the right way to go.
>
> What do you think?
Well, I guess it is a matter of taste after all.
I don't think the number of axis will change, but just put the break in
place is good enough.
Anyway, If we choose to not use the switch, I think we should remove the
for-loop as well, eg:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
if (state->sensor_num == MOTIONSENSE_LOC_LID) {
state->sign[X] = -1;
state->sign[Y] = 1;
state->sign[Z] = -1;
} else {
state->sign[X] = 1;
state->sign[Y] = 1;
state->sign[Z] = 1;
}
But someone else may like to give their point of view on this change.
>
> Thanks for the feedback.
> --
> Gustavo
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi Gutavo,
Sorry for the delay.
On Wed, Aug 15, 2018 at 12:50:10PM -0500, Gustavo A. R. Silva wrote:
> Hi Marcus,
>
> On 8/15/18 12:27 PM, Marcus Folkesson wrote:
> > Hi,
> >
> > On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> >> where we are expecting to fall through.
> >>
> >> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> >> Signed-off-by: Gustavo A. R. Silva
> >> ---
> >> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> index 063e89e..d609654 100644
> >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> >> platform_device *pdev)
> >>switch (i) {
> >>case X:
> >>ec_accel_channels[X].scan_index = Y;
> >> + /* fall through */
> >>case Y:
> >>ec_accel_channels[Y].scan_index = X;
> >> + /* fall through */
> >>case Z:
> >>ec_accel_channels[Z].scan_index = Z;
> >>}
> >
> > Hum, I'm not sure we are supposed to fall through here, even if it does
> > not hurt to do so.
>
> Yeah. You're right. It doesn't hurt but is actually redundant. I think
> the original intention was to break instead of falling through.
>
> > I even think we can remove the switch and put that outside the for-loop,
> > e.g:
> >
> > ec_accel_channels[X].scan_index = Y;
> > ec_accel_channels[Y].scan_index = X;
> > ec_accel_channels[Z].scan_index = Z;
> >
> > for (i = X ; i < MAX_AXIS; i++) {
> > if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> > state->sign[i] = -1;
> > else
> > state->sign[i] = 1;
> > }
> >
>
> I like this, but the code clearly depends on MAX_AXIS. So, if MAX_AXIS
> will be always 3, then the change you suggest is just fine. Otherwise,
> it seems that adding a break to each case is the right way to go.
>
> What do you think?
Well, I guess it is a matter of taste after all.
I don't think the number of axis will change, but just put the break in
place is good enough.
Anyway, If we choose to not use the switch, I think we should remove the
for-loop as well, eg:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
if (state->sensor_num == MOTIONSENSE_LOC_LID) {
state->sign[X] = -1;
state->sign[Y] = 1;
state->sign[Z] = -1;
} else {
state->sign[X] = 1;
state->sign[Y] = 1;
state->sign[Z] = 1;
}
But someone else may like to give their point of view on this change.
>
> Thanks for the feedback.
> --
> Gustavo
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi,
On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 063e89e..d609654 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> platform_device *pdev)
> switch (i) {
> case X:
> ec_accel_channels[X].scan_index = Y;
> + /* fall through */
> case Y:
> ec_accel_channels[Y].scan_index = X;
> + /* fall through */
> case Z:
> ec_accel_channels[Z].scan_index = Z;
> }
Hum, I'm not sure we are supposed to fall through here, even if it does
not hurt to do so.
I even think we can remove the switch and put that outside the for-loop,
e.g:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
for (i = X ; i < MAX_AXIS; i++) {
if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
state->sign[i] = -1;
else
state->sign[i] = 1;
}
Best regards,
Marcus Folkesson
> --
> 2.7.4
>
signature.asc
Description: PGP signature
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi,
On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 063e89e..d609654 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> platform_device *pdev)
> switch (i) {
> case X:
> ec_accel_channels[X].scan_index = Y;
> + /* fall through */
> case Y:
> ec_accel_channels[Y].scan_index = X;
> + /* fall through */
> case Z:
> ec_accel_channels[Z].scan_index = Z;
> }
Hum, I'm not sure we are supposed to fall through here, even if it does
not hurt to do so.
I even think we can remove the switch and put that outside the for-loop,
e.g:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
for (i = X ; i < MAX_AXIS; i++) {
if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
state->sign[i] = -1;
else
state->sign[i] = 1;
}
Best regards,
Marcus Folkesson
> --
> 2.7.4
>
signature.asc
Description: PGP signature
[PATCH 1/3] iio: dac: add support for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc166x.c | 244 ++
3 files changed, 255 insertions(+)
create mode 100644 drivers/iio/dac/ltc166x.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..04cfa6bb9dc1 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC166X
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc166x.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..380749c87c26 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC166X) += ltc166x.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
new file mode 100644
index ..0031f2b50f14
--- /dev/null
+++ b/drivers/iio/dac/ltc166x.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC166X_REG_WAKE 0x0
+#define LTC166X_REG_DAC_A 0x1
+#define LTC166X_REG_DAC_B 0x2
+#define LTC166X_REG_DAC_C 0x3
+#define LTC166X_REG_DAC_D 0x4
+#define LTC166X_REG_DAC_E 0x5
+#define LTC166X_REG_DAC_F 0x6
+#define LTC166X_REG_DAC_G 0x7
+#define LTC166X_REG_DAC_H 0x8
+#define LTC166X_REG_SLEEP 0xe
+
+#define LTC166X_NUM_CHANNELS 8
+
+static const struct regmap_config ltc166x_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc166x_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc166x_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC166X_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc166x_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = priv->vref_mv;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc166x_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+ if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ return -EINVAL;
+ priv->value[chan->channel] = val;
+ val <<= chan->scan_type.shift;
+ return regmap_write(priv->regmap, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC166X_CHAN(chan, bits) { \
+ .type = IIO_VOLTAGE,\
+ .indexed = 1, \
+ .output = 1,\
+ .channel = chan,\
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u',\
+ .realbits = (bits), \
+ .storagebits = 16,
[PATCH 2/3] dt-bindings: iio: dac: add bindings for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
.../devicetree/bindings/iio/dac/ltc166x.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc166x.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH 1/3] iio: dac: add support for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc166x.c | 244 ++
3 files changed, 255 insertions(+)
create mode 100644 drivers/iio/dac/ltc166x.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..04cfa6bb9dc1 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC166X
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc166x.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..380749c87c26 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC166X) += ltc166x.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
new file mode 100644
index ..0031f2b50f14
--- /dev/null
+++ b/drivers/iio/dac/ltc166x.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC166X_REG_WAKE 0x0
+#define LTC166X_REG_DAC_A 0x1
+#define LTC166X_REG_DAC_B 0x2
+#define LTC166X_REG_DAC_C 0x3
+#define LTC166X_REG_DAC_D 0x4
+#define LTC166X_REG_DAC_E 0x5
+#define LTC166X_REG_DAC_F 0x6
+#define LTC166X_REG_DAC_G 0x7
+#define LTC166X_REG_DAC_H 0x8
+#define LTC166X_REG_SLEEP 0xe
+
+#define LTC166X_NUM_CHANNELS 8
+
+static const struct regmap_config ltc166x_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc166x_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc166x_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC166X_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc166x_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = priv->vref_mv;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc166x_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+ if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ return -EINVAL;
+ priv->value[chan->channel] = val;
+ val <<= chan->scan_type.shift;
+ return regmap_write(priv->regmap, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC166X_CHAN(chan, bits) { \
+ .type = IIO_VOLTAGE,\
+ .indexed = 1, \
+ .output = 1,\
+ .channel = chan,\
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u',\
+ .realbits = (bits), \
+ .storagebits = 16,
[PATCH 2/3] dt-bindings: iio: dac: add bindings for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
.../devicetree/bindings/iio/dac/ltc166x.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc166x.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH 3/3] MAINTAINERS: add entry for ltc166x DAC driver
Add entry for ltc166x DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..2dc4c773fb2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC166X DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc166x.txt +F: drivers/iio/dac/ltc166x.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[PATCH 3/3] MAINTAINERS: add entry for ltc166x DAC driver
Add entry for ltc166x DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..2dc4c773fb2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC166X DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc166x.txt +F: drivers/iio/dac/ltc166x.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[RESEND PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), , 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), , 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = _pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = _pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
Re: [PATCH v4 5/6] iio:adxl372: Add sampling frequency support
0 6400");
> +
> +static struct attribute *adxl372_attributes[] = {
> + _const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group adxl372_attrs_group = {
> + .attrs = adxl372_attributes,
> +};
> +
> static const struct iio_info adxl372_info = {
> + .attrs = _attrs_group,
> .read_raw = adxl372_read_raw,
> + .write_raw = adxl372_write_raw,
> .debugfs_reg_access = _reg_access,
> .hwfifo_set_watermark = adxl372_set_watermark,
> };
> --
> 2.7.4
Best regards,
Marcus Folkesson
>
signature.asc
Description: PGP signature
Re: [PATCH v4 5/6] iio:adxl372: Add sampling frequency support
0 6400");
> +
> +static struct attribute *adxl372_attributes[] = {
> + _const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group adxl372_attrs_group = {
> + .attrs = adxl372_attributes,
> +};
> +
> static const struct iio_info adxl372_info = {
> + .attrs = _attrs_group,
> .read_raw = adxl372_read_raw,
> + .write_raw = adxl372_write_raw,
> .debugfs_reg_access = _reg_access,
> .hwfifo_set_watermark = adxl372_set_watermark,
> };
> --
> 2.7.4
Best regards,
Marcus Folkesson
>
signature.asc
Description: PGP signature
Re: [PATCH v4 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Mon, Aug 06, 2018 at 03:04:42PM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ADXL372 3-Axis Digital Accelerometer SPI driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
Please make the SPDX-identifier and MODULE_LICENCE match here as well.
Either
SPDX-License-Identifier: GPL-2.0+
MODULE_LICENSE("GPL")
or
SPDX-License-Identifier: GPL-2.0
MODULE_LICENSE("GPL v2")
Thanks!
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v4 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Mon, Aug 06, 2018 at 03:04:42PM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ADXL372 3-Axis Digital Accelerometer SPI driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
Please make the SPDX-identifier and MODULE_LICENCE match here as well.
Either
SPDX-License-Identifier: GPL-2.0+
MODULE_LICENSE("GPL")
or
SPDX-License-Identifier: GPL-2.0
MODULE_LICENSE("GPL v2")
Thanks!
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), , 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = _pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
[PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), , 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = _pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = _pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
Re: [PATCH v6 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Fri, Aug 10, 2018 at 11:46:18AM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
The SPDX identifier "GPL-2.0+" is "GPL v2 or later", and the MODULE_LICENSE
"GPL v2" is "GPL v2 only"
See include/linux/module.h ::
* "GPL" [GNU Public License v2 or later]
* "GPL v2"[GNU Public License v2]
Please make them match :-)
[snip]
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v6 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Fri, Aug 10, 2018 at 11:46:18AM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
The SPDX identifier "GPL-2.0+" is "GPL v2 or later", and the MODULE_LICENSE
"GPL v2" is "GPL v2 only"
See include/linux/module.h ::
* "GPL" [GNU Public License v2 or later]
* "GPL v2"[GNU Public License v2]
Please make them match :-)
[snip]
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v4 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
---
Notes:
v4:
- remove defined(CONFIG_OF) and of_match_ptr() macro
- do not check adc->clki as clock api is NULL-aware
- add Kent as co-developer
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 361 ++
3 files changed, 372 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..540c8fbcce89
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
[PATCH v4 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Co-Developed-by: Kent Gustavsson Signed-off-by: Kent Gustavsson Signed-off-by: Marcus Folkesson --- Notes: v4: - add Kent as co-developer v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v4 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
Reviewed-by: Rob Herring
---
Notes:
v4:
- remove reference to Documentation/.../interrupts.txt
- add Kent as co-developer
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 30 ++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..3071f48fb30b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,30 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ - interrupts: IRQ line for the ADC
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+};
--
2.11.0.rc2
[PATCH v4 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
---
Notes:
v4:
- remove defined(CONFIG_OF) and of_match_ptr() macro
- do not check adc->clki as clock api is NULL-aware
- add Kent as co-developer
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 361 ++
3 files changed, 372 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..540c8fbcce89
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
[PATCH v4 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Co-Developed-by: Kent Gustavsson Signed-off-by: Kent Gustavsson Signed-off-by: Marcus Folkesson --- Notes: v4: - add Kent as co-developer v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v4 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
Reviewed-by: Rob Herring
---
Notes:
v4:
- remove reference to Documentation/.../interrupts.txt
- add Kent as co-developer
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 30 ++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..3071f48fb30b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,30 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ - interrupts: IRQ line for the ADC
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+};
--
2.11.0.rc2
Re: [PATCH v3 1/3] iio: adc: add support for mcp3911
Hi Andy and Jonathan,
On Fri, Aug 03, 2018 at 11:09:22PM +0100, Jonathan Cameron wrote:
> On Thu, 2 Aug 2018 22:52:00 +0300
> Andy Shevchenko wrote:
>
> > On Thu, Aug 2, 2018 at 10:15 PM, Marcus Folkesson
> > wrote:
> > > MCP3911 is a dual channel Analog Front End (AFE) containing two
> > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
> > >
> > > Signed-off-by: Marcus Folkesson
> >
> > > Signed-off-by: Kent Gustavsson
> >
> > What is this? Why it's here (presense and location in the message)?
> To clarify... If Kent wrote the patch and you are simply acting
> as gatekeeper / upstreamer you should set the mail to be from Kent
> and put your own Signed-off after his to basically act as a submaintainer
> certifying you believe his sign off and all that entails.
>
> If it is a bit of a joint effort then that's fine but for copyright
> purposes there should be some indication of the split.
First, thank you Andy for noticing.
I actually intended to use Co-Developed-by (a pretty new tag)
in combination with Signed-off-by.
But the tag must have disappeared in some preparation stage..
From Documentation/process/submitting-patches.rst ::
A Co-Developed-by: states that the patch was also created by another
developer
along with the original author. This is useful at times when multiple
people
work on a single patch. Note, this person also needs to have a
Signed-off-by:
line in the patch as well.
I will switch order and add the Co-Developed-by-tag.
Is this correct?
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
>
>
> >
> > > + * Copyright (C) 2018 Kent Gustavsson
> >
> > > + *
> >
> > Redundant.
> >
> > > +static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
> > > +{
> > > + int ret;
> > > +
> > > + reg = MCP3911_REG_READ(reg, adc->dev_addr);
> > > + ret = spi_write_then_read(adc->spi, , 1, val, len);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + be32_to_cpus(val);
> > > + *val >>= ((4 - len) * 8);
> > > + dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
> > > + reg>>1);
> > > + return ret;
> > > +}
> > > +
> > > +static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
> > > +{
> > > + dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val,
> > > reg);
> > > +
> > > + val <<= (3 - len) * 8;
> > > + cpu_to_be32s();
> > > + val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
> > > +
> > > + return spi_write(adc->spi, , len + 1);
> > > +}
> >
> > Can't you use REGMAP_SPI ?
> My instinct is not really. The magic device-addr doesn't help.
> You could work around it but it's not that nice and you'd have
> to create the regmap mapping on the fly or carry static ones
> for each value of htat.
>
>
>
> >
> > > + of_property_read_u32(of_node, "device-addr", >dev_addr);
> > > + if (adc->dev_addr > 3) {
> > > + dev_err(>spi->dev,
> > > + "invalid device address (%i). Must be in
> > > range 0-3.\n",
> > > + adc->dev_addr);
> > > + return -EINVAL;
> > > + }
> > > + dev_dbg(>spi->dev, "use device address %i\n",
> > > adc->dev_addr);
> >
> > Isn't what we called CS (chip select)?
> Nope. We went around this in an earlier revision. It's an address transmitted
> in the control byte to allow you to 'share' a chip select line between
> multiple
> chips (crazy but true).
>
> >
> > > + if (IS_ERR(adc->vref)) {
> >
> > > +
> >
> > Redundant.
> >
> > > + if (adc->clki)
> >
> > Seems to be redundant if clock is optional (it should be NULL and API
> > is NULL-aware).
> >
> > > + clk_disable_unprepare(adc->clki);
> >
> > > + if (adc->clki)
> > > + clk_disable_unprepare(adc->clki);
> >
> > Ditto.
> >
> > > +#if defined(CONFIG_OF)
> >
> > This prevents playing with the device on ACPI enabled platforms.
I will remove the defined(CONFIG_OF) and not use the of_match_ptr()
macro.
> Yeah, that trickery is still little known (and I forget about it from
> time to time).
>
> The upshot for those who have missed this is you can use a magic
> acpi device to instantiate based on device tree bindings :)
>
> So we need to strip all this 'obvious' protection stuff out of drivers.
Jonathan,
Do you want me to do the same changes for drivers in iio/ ?
I'm probably not the only one looking at other drivers when writing my
own, so I guess this is a rather frequent issue for new drivers?
>
> >
> > > + .of_match_table = of_match_ptr(mcp3911_dt_ids),
> >
> > Ditto for macro.
> >
>
Best regards,
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v3 1/3] iio: adc: add support for mcp3911
Hi Andy and Jonathan,
On Fri, Aug 03, 2018 at 11:09:22PM +0100, Jonathan Cameron wrote:
> On Thu, 2 Aug 2018 22:52:00 +0300
> Andy Shevchenko wrote:
>
> > On Thu, Aug 2, 2018 at 10:15 PM, Marcus Folkesson
> > wrote:
> > > MCP3911 is a dual channel Analog Front End (AFE) containing two
> > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
> > >
> > > Signed-off-by: Marcus Folkesson
> >
> > > Signed-off-by: Kent Gustavsson
> >
> > What is this? Why it's here (presense and location in the message)?
> To clarify... If Kent wrote the patch and you are simply acting
> as gatekeeper / upstreamer you should set the mail to be from Kent
> and put your own Signed-off after his to basically act as a submaintainer
> certifying you believe his sign off and all that entails.
>
> If it is a bit of a joint effort then that's fine but for copyright
> purposes there should be some indication of the split.
First, thank you Andy for noticing.
I actually intended to use Co-Developed-by (a pretty new tag)
in combination with Signed-off-by.
But the tag must have disappeared in some preparation stage..
From Documentation/process/submitting-patches.rst ::
A Co-Developed-by: states that the patch was also created by another
developer
along with the original author. This is useful at times when multiple
people
work on a single patch. Note, this person also needs to have a
Signed-off-by:
line in the patch as well.
I will switch order and add the Co-Developed-by-tag.
Is this correct?
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
>
>
> >
> > > + * Copyright (C) 2018 Kent Gustavsson
> >
> > > + *
> >
> > Redundant.
> >
> > > +static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
> > > +{
> > > + int ret;
> > > +
> > > + reg = MCP3911_REG_READ(reg, adc->dev_addr);
> > > + ret = spi_write_then_read(adc->spi, , 1, val, len);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + be32_to_cpus(val);
> > > + *val >>= ((4 - len) * 8);
> > > + dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
> > > + reg>>1);
> > > + return ret;
> > > +}
> > > +
> > > +static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
> > > +{
> > > + dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val,
> > > reg);
> > > +
> > > + val <<= (3 - len) * 8;
> > > + cpu_to_be32s();
> > > + val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
> > > +
> > > + return spi_write(adc->spi, , len + 1);
> > > +}
> >
> > Can't you use REGMAP_SPI ?
> My instinct is not really. The magic device-addr doesn't help.
> You could work around it but it's not that nice and you'd have
> to create the regmap mapping on the fly or carry static ones
> for each value of htat.
>
>
>
> >
> > > + of_property_read_u32(of_node, "device-addr", >dev_addr);
> > > + if (adc->dev_addr > 3) {
> > > + dev_err(>spi->dev,
> > > + "invalid device address (%i). Must be in
> > > range 0-3.\n",
> > > + adc->dev_addr);
> > > + return -EINVAL;
> > > + }
> > > + dev_dbg(>spi->dev, "use device address %i\n",
> > > adc->dev_addr);
> >
> > Isn't what we called CS (chip select)?
> Nope. We went around this in an earlier revision. It's an address transmitted
> in the control byte to allow you to 'share' a chip select line between
> multiple
> chips (crazy but true).
>
> >
> > > + if (IS_ERR(adc->vref)) {
> >
> > > +
> >
> > Redundant.
> >
> > > + if (adc->clki)
> >
> > Seems to be redundant if clock is optional (it should be NULL and API
> > is NULL-aware).
> >
> > > + clk_disable_unprepare(adc->clki);
> >
> > > + if (adc->clki)
> > > + clk_disable_unprepare(adc->clki);
> >
> > Ditto.
> >
> > > +#if defined(CONFIG_OF)
> >
> > This prevents playing with the device on ACPI enabled platforms.
I will remove the defined(CONFIG_OF) and not use the of_match_ptr()
macro.
> Yeah, that trickery is still little known (and I forget about it from
> time to time).
>
> The upshot for those who have missed this is you can use a magic
> acpi device to instantiate based on device tree bindings :)
>
> So we need to strip all this 'obvious' protection stuff out of drivers.
Jonathan,
Do you want me to do the same changes for drivers in iio/ ?
I'm probably not the only one looking at other drivers when writing my
own, so I guess this is a rather frequent issue for new drivers?
>
> >
> > > + .of_match_table = of_match_ptr(mcp3911_dt_ids),
> >
> > Ditto for macro.
> >
>
Best regards,
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v3 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v3 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 32 ++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..432da7a28783
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,32 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - interrupts: IRQ line for the ADC
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+};
--
2.11.0.rc2
[PATCH v3 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v3 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 32 ++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..432da7a28783
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,32 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - interrupts: IRQ line for the ADC
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+};
--
2.11.0.rc2
[PATCH v3 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 367 ++
3 files changed, 378 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..fca8e6a304ec
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s();
+ val |= MCP3911_REG_WRITE(reg, adc->dev
[PATCH v3 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 367 ++
3 files changed, 378 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..fca8e6a304ec
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s();
+ val |= MCP3911_REG_WRITE(reg, adc->dev
Re: [PATCH v2 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Sorry for the delay, I've been away from keyboard for a few days. On Sun, Jul 29, 2018 at 12:44:22PM +0100, Jonathan Cameron wrote: > On Tue, 24 Jul 2018 20:30:02 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > Hi > > I think you missed cleaning up the clock enable in the remove. > If it is safe to not do so it should also be safe in the error path > of probe. I've actually noticed this and had it prepared in v4 :-) > > A couple of trivial other things jumped out at me whilst reading. Thank you so much, I will fix these. > > Thanks, > > Jonathan > > --- > > > > Notes: > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 366 > > ++ > > 3 files changed, 377 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..29aa39930ead > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,366 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14 > > +#de
Re: [PATCH v2 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Sorry for the delay, I've been away from keyboard for a few days. On Sun, Jul 29, 2018 at 12:44:22PM +0100, Jonathan Cameron wrote: > On Tue, 24 Jul 2018 20:30:02 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > Hi > > I think you missed cleaning up the clock enable in the remove. > If it is safe to not do so it should also be safe in the error path > of probe. I've actually noticed this and had it prepared in v4 :-) > > A couple of trivial other things jumped out at me whilst reading. Thank you so much, I will fix these. > > Thanks, > > Jonathan > > --- > > > > Notes: > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 366 > > ++ > > 3 files changed, 377 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..29aa39930ead > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,366 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14 > > +#de
Re: [PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hello Rob, Thank you for the review. On Wed, Jul 25, 2018 at 11:51:17AM -0600, Rob Herring wrote: > On Tue, Jul 24, 2018 at 08:30:03PM +0200, Marcus Folkesson wrote: > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > > > Notes: > > v2: > > - drop channel width > > - drop `external_vref` > > - replace `external-clock` with proper clock bindings > > > > .../devicetree/bindings/iio/adc/mcp3911.txt| 28 > > ++ > > 1 file changed, 28 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > new file mode 100644 > > index ..af5472f51a84 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > @@ -0,0 +1,28 @@ > > +* Microchip MCP3911 Dual channel analog front end (ADC) > > + > > +Required properties: > > + - compatible: Should be "microchip,mcp3911" > > + - reg: SPI chip select number for the device > > + > > +Recommended properties: > > + - spi-max-frequency: Definition as per > > +Documentation/devicetree/bindings/spi/spi-bus.txt. > > +Max frequency for this chip is 20MHz. > > + > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > Isn't this the reg value? > The reg value defines which chip select the chip hangs on. The chip has support to connect up to four chips on the same SPI bus, including the same chip select signal. The chips are separated by the device-addr as it is part of the communication protocol. When reading the description for device-addr, I agree that it fits well for the reg property as well.. > > + - vref-supply: Phandle to the external reference voltage supply. > > + - clocks: Phandle and clock identifier (see clock-names) > > + - clock-names: "adc_clk" for the ADC (sampling) clock > > Datasheet calls this clki (or mclk internally). Or just drop clock-names > as it is pointless > when there is only 1 clock. I will drop the clock-names, thanks! > > Also DR handling as an IRQ is missing. I have considered using the DR signal, but as we not support buffering yet I did not see any point in using it. From the datasheet, section 7.1 :: These registers [channel registers] are latched when an ADC read communication occurs. When a data ready event occurs during a read communication, the most current ADC data is also latched to avoid data corruption issues. The three bytes of each channel are updated synchronously at a DRCLK rate. The three bytes can be accessed separately if needed but are refreshed synchronously. So it should be safe to read the values independently of the DR signal. Or maybe I'm missing something. Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hello Rob, Thank you for the review. On Wed, Jul 25, 2018 at 11:51:17AM -0600, Rob Herring wrote: > On Tue, Jul 24, 2018 at 08:30:03PM +0200, Marcus Folkesson wrote: > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > > > Notes: > > v2: > > - drop channel width > > - drop `external_vref` > > - replace `external-clock` with proper clock bindings > > > > .../devicetree/bindings/iio/adc/mcp3911.txt| 28 > > ++ > > 1 file changed, 28 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > new file mode 100644 > > index ..af5472f51a84 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > @@ -0,0 +1,28 @@ > > +* Microchip MCP3911 Dual channel analog front end (ADC) > > + > > +Required properties: > > + - compatible: Should be "microchip,mcp3911" > > + - reg: SPI chip select number for the device > > + > > +Recommended properties: > > + - spi-max-frequency: Definition as per > > +Documentation/devicetree/bindings/spi/spi-bus.txt. > > +Max frequency for this chip is 20MHz. > > + > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > Isn't this the reg value? > The reg value defines which chip select the chip hangs on. The chip has support to connect up to four chips on the same SPI bus, including the same chip select signal. The chips are separated by the device-addr as it is part of the communication protocol. When reading the description for device-addr, I agree that it fits well for the reg property as well.. > > + - vref-supply: Phandle to the external reference voltage supply. > > + - clocks: Phandle and clock identifier (see clock-names) > > + - clock-names: "adc_clk" for the ADC (sampling) clock > > Datasheet calls this clki (or mclk internally). Or just drop clock-names > as it is pointless > when there is only 1 clock. I will drop the clock-names, thanks! > > Also DR handling as an IRQ is missing. I have considered using the DR signal, but as we not support buffering yet I did not see any point in using it. From the datasheet, section 7.1 :: These registers [channel registers] are latched when an ADC read communication occurs. When a data ready event occurs during a read communication, the most current ADC data is also latched to avoid data corruption issues. The three bytes of each channel are updated synchronously at a DRCLK rate. The three bytes can be accessed separately if needed but are refreshed synchronously. So it should be safe to read the values independently of the DR signal. Or maybe I'm missing something. Best regards Marcus Folkesson signature.asc Description: PGP signature
[PATCH v2 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 366 ++
3 files changed, 377 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..29aa39930ead
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *adc_clk;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s();
+ val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, , len+1);
+}
+
+static int mcp3911_update(struct m
[PATCH v2 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 366 ++
3 files changed, 377 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..29aa39930ead
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *adc_clk;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(>spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s();
+ val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, , len+1);
+}
+
+static int mcp3911_update(struct m
[PATCH v2 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 28 ++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..af5472f51a84
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,28 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+ - clocks: Phandle and clock identifier (see clock-names)
+ - clock-names: "adc_clk" for the ADC (sampling) clock
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+ clock-names = "adc_clk";
+};
--
2.11.0.rc2
[PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 28 ++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..af5472f51a84
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,28 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+ - clocks: Phandle and clock identifier (see clock-names)
+ - clock-names: "adc_clk" for the ADC (sampling) clock
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ vref-supply = <_reg>;
+ clocks = <>;
+ clock-names = "adc_clk";
+};
--
2.11.0.rc2
Re: [PATCH] input: pxrc - do not store USB device in private struct
Hello Dmitry, On Tue, Jul 24, 2018 at 02:38:04AM +, Dmitry Torokhov wrote: > Hi Marcus, > > On Mon, Jul 16, 2018 at 04:40:14PM +0200, Marcus Folkesson wrote: > > The USB device is only needed during setup, so put it back after > > initialization and do not store it in our private struct. > > > > Also, the USB device is a parent of USB interface so our driver > > model rules ensure that USB device should not disappear while > > interface device is still there. > > So not keep a refcount on the device is safe. > > > > Reported-by: Alexey Khoroshilov > > Signed-off-by: Marcus Folkesson > > --- > > drivers/input/joystick/pxrc.c | 22 -- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > > index 07a0dbd3ced2..46a7acb747bf 100644 > > --- a/drivers/input/joystick/pxrc.c > > +++ b/drivers/input/joystick/pxrc.c > ... > > > @@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf, > > return -ENOMEM; > > > > mutex_init(>pm_mutex); > > - pxrc->udev = usb_get_dev(interface_to_usbdev(intf)); > > + udev = usb_get_dev(interface_to_usbdev(intf)); > > There is really no need to "get" device for the probe duration, or in > general, when you are not storing the reference to it. > > I posted series with an updated version of this patch plus couple more > cleanups/fixes, and would appreciate if you could give it a spin. Thank you for doing this. I have reviewed the patchset and tested on real hardware, and it looks good to me. For what it's worth: Reviewed-by: Marcus Folkesson Tested-by: Marcus Folkesson On the whole patchset. > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH] input: pxrc - do not store USB device in private struct
Hello Dmitry, On Tue, Jul 24, 2018 at 02:38:04AM +, Dmitry Torokhov wrote: > Hi Marcus, > > On Mon, Jul 16, 2018 at 04:40:14PM +0200, Marcus Folkesson wrote: > > The USB device is only needed during setup, so put it back after > > initialization and do not store it in our private struct. > > > > Also, the USB device is a parent of USB interface so our driver > > model rules ensure that USB device should not disappear while > > interface device is still there. > > So not keep a refcount on the device is safe. > > > > Reported-by: Alexey Khoroshilov > > Signed-off-by: Marcus Folkesson > > --- > > drivers/input/joystick/pxrc.c | 22 -- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > > index 07a0dbd3ced2..46a7acb747bf 100644 > > --- a/drivers/input/joystick/pxrc.c > > +++ b/drivers/input/joystick/pxrc.c > ... > > > @@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf, > > return -ENOMEM; > > > > mutex_init(>pm_mutex); > > - pxrc->udev = usb_get_dev(interface_to_usbdev(intf)); > > + udev = usb_get_dev(interface_to_usbdev(intf)); > > There is really no need to "get" device for the probe duration, or in > general, when you are not storing the reference to it. > > I posted series with an updated version of this patch plus couple more > cleanups/fixes, and would appreciate if you could give it a spin. Thank you for doing this. I have reviewed the patchset and tested on real hardware, and it looks good to me. For what it's worth: Reviewed-by: Marcus Folkesson Tested-by: Marcus Folkesson On the whole patchset. > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hi Jonathan, On Sun, Jul 22, 2018 at 09:11:11AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 21:59:22 +0200 > Marcus Folkesson wrote: > [snip] > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > + - external-clock: Use external clock instead of crystal oscillator. > As mentioned, in the code, can we use the standard fixed clock bindings here. > We don't actually care what the value is, but it might be nice to be able to > power down the clock if we are suspending or something.. > > > + - external-vref: Use external voltage reference > > + - vref-supply:Phandle to the external reference voltage supply. (only > > valid in combination with `external-vref`) > > Just use the optional regulator stuff and get rid of the bool. > > > + - ch0-width: width for channel0. Valid widths are 16 and 24bits. > > + - ch1-width: width for channel1. Valid widths are 16 and 24bits. > > As I asked in the code, are these a function of the wiring etc or are > they something we should really be leaving up to userspace (with a sensible > default). > I agree with all of your comments. I will remove the channel width properties and fix the regulator/clock bindings. Thanks, Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hi Jonathan, On Sun, Jul 22, 2018 at 09:11:11AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 21:59:22 +0200 > Marcus Folkesson wrote: > [snip] > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > + - external-clock: Use external clock instead of crystal oscillator. > As mentioned, in the code, can we use the standard fixed clock bindings here. > We don't actually care what the value is, but it might be nice to be able to > power down the clock if we are suspending or something.. > > > + - external-vref: Use external voltage reference > > + - vref-supply:Phandle to the external reference voltage supply. (only > > valid in combination with `external-vref`) > > Just use the optional regulator stuff and get rid of the bool. > > > + - ch0-width: width for channel0. Valid widths are 16 and 24bits. > > + - ch1-width: width for channel1. Valid widths are 16 and 24bits. > > As I asked in the code, are these a function of the wiring etc or are > they something we should really be leaving up to userspace (with a sensible > default). > I agree with all of your comments. I will remove the channel width properties and fix the regulator/clock bindings. Thanks, Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Thanks, all good catches. On Sun, Jul 22, 2018 at 09:08:38AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 23:19:48 +0200 (CEST) > Peter Meerwald-Stadler wrote: > > > Hello, > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > some comments below... > > +CC Mark for the unusual SPI addressing stuff. I'm mostly interested in what > precedent there is for bindings etc. > Yep, I'm not entirely sure that the SPI framework can handle multiple clients on the same CS. The reason why we created device-addr is that the chip supports that and may have hardcoded chip address from factory. The chip address is also part of the protocol so we have to specify it. > > > > > Signed-off-by: Marcus Folkesson > > > Signed-off-by: Kent Gustavsson > > > --- > > > drivers/iio/adc/Kconfig | 10 ++ > > > drivers/iio/adc/Makefile | 1 + > > > drivers/iio/adc/mcp3911.c | 444 > > > ++ > > > 3 files changed, 455 insertions(+) > > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > > index 15606f237480..f9a41fa96fcc 100644 > > > --- a/drivers/iio/adc/Kconfig > > > +++ b/drivers/iio/adc/Kconfig > > > @@ -501,6 +501,16 @@ config MCP3422 > > > This driver can also be built as a module. If so, the module will be > > > called mcp3422. > > > > > > +config MCP3911 > > > + tristate "Microchip Technology MCP3911 driver" > > > + depends on SPI > > > + help > > > + Say yes here to build support for Microchip Technology's MCP3911 > > > + analog to digital converter. > > > + > > > + This driver can also be built as a module. If so, the module will be > > > + called mcp3911. > > > + > > > config MEDIATEK_MT6577_AUXADC > > > tristate "MediaTek AUXADC driver" > > > depends on ARCH_MEDIATEK || COMPILE_TEST > > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > > index 28a9423997f3..3cfebfff7d26 100644 > > > --- a/drivers/iio/adc/Makefile > > > +++ b/drivers/iio/adc/Makefile > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > > obj-$(CONFIG_MAX9611) += max9611.o > > > obj-$(CONFIG_MCP320X) += mcp320x.o > > > obj-$(CONFIG_MCP3422) += mcp3422.o > > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > > new file mode 100644 > > > index ..be74cb15827b > > > --- /dev/null > > > +++ b/drivers/iio/adc/mcp3911.c > > > @@ -0,0 +1,444 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > > + * > > > + * Copyright (C) 2018 Marcus Folkesson > > > + * Copyright (C) 2018 Kent Gustavsson > > > + * > > No need for blank line here. > Ok > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define MCP3911_REG_CHANNEL0 0x00 > > > +#define MCP3911_REG_CHANNEL1 0x03 > > > +#define MCP3911_REG_MOD 0x06 > > > +#define MCP3911_REG_PHASE0x07 > > > + > > > +#define MCP3911_REG_GAIN 0x09 > > > +#define MCP3911_GAIN_MASK(ch)(0x7 << 3*ch) > > > > space around * operator, maybe parenthesis around variable, i.e > > (0x07 << (3 * (ch))) > > > > > +#define MCP3911_GAIN_VAL(ch, val)((val << 3*ch) & > > > MCP3911_GAIN_MASK(ch)) > > > + > > > +#define MCP3911_REG_STATUSCOM0x0a > > > +#define MCP3911_STATUSCOM_CH1_24WIDTHBIT(4) > > > +#define MCP3911_STATUSCOM_CH0_24WIDTHBIT(3) > > > +#define MCP3911_STATUSCOM_EN_OFFCAL BIT(2) > > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > > + > > > +#define MCP3911_REG_CONFIG 0x0c > > > +#define MCP3911_CONFIG_
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Thanks, all good catches. On Sun, Jul 22, 2018 at 09:08:38AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 23:19:48 +0200 (CEST) > Peter Meerwald-Stadler wrote: > > > Hello, > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > some comments below... > > +CC Mark for the unusual SPI addressing stuff. I'm mostly interested in what > precedent there is for bindings etc. > Yep, I'm not entirely sure that the SPI framework can handle multiple clients on the same CS. The reason why we created device-addr is that the chip supports that and may have hardcoded chip address from factory. The chip address is also part of the protocol so we have to specify it. > > > > > Signed-off-by: Marcus Folkesson > > > Signed-off-by: Kent Gustavsson > > > --- > > > drivers/iio/adc/Kconfig | 10 ++ > > > drivers/iio/adc/Makefile | 1 + > > > drivers/iio/adc/mcp3911.c | 444 > > > ++ > > > 3 files changed, 455 insertions(+) > > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > > index 15606f237480..f9a41fa96fcc 100644 > > > --- a/drivers/iio/adc/Kconfig > > > +++ b/drivers/iio/adc/Kconfig > > > @@ -501,6 +501,16 @@ config MCP3422 > > > This driver can also be built as a module. If so, the module will be > > > called mcp3422. > > > > > > +config MCP3911 > > > + tristate "Microchip Technology MCP3911 driver" > > > + depends on SPI > > > + help > > > + Say yes here to build support for Microchip Technology's MCP3911 > > > + analog to digital converter. > > > + > > > + This driver can also be built as a module. If so, the module will be > > > + called mcp3911. > > > + > > > config MEDIATEK_MT6577_AUXADC > > > tristate "MediaTek AUXADC driver" > > > depends on ARCH_MEDIATEK || COMPILE_TEST > > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > > index 28a9423997f3..3cfebfff7d26 100644 > > > --- a/drivers/iio/adc/Makefile > > > +++ b/drivers/iio/adc/Makefile > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > > obj-$(CONFIG_MAX9611) += max9611.o > > > obj-$(CONFIG_MCP320X) += mcp320x.o > > > obj-$(CONFIG_MCP3422) += mcp3422.o > > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > > new file mode 100644 > > > index ..be74cb15827b > > > --- /dev/null > > > +++ b/drivers/iio/adc/mcp3911.c > > > @@ -0,0 +1,444 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > > + * > > > + * Copyright (C) 2018 Marcus Folkesson > > > + * Copyright (C) 2018 Kent Gustavsson > > > + * > > No need for blank line here. > Ok > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define MCP3911_REG_CHANNEL0 0x00 > > > +#define MCP3911_REG_CHANNEL1 0x03 > > > +#define MCP3911_REG_MOD 0x06 > > > +#define MCP3911_REG_PHASE0x07 > > > + > > > +#define MCP3911_REG_GAIN 0x09 > > > +#define MCP3911_GAIN_MASK(ch)(0x7 << 3*ch) > > > > space around * operator, maybe parenthesis around variable, i.e > > (0x07 << (3 * (ch))) > > > > > +#define MCP3911_GAIN_VAL(ch, val)((val << 3*ch) & > > > MCP3911_GAIN_MASK(ch)) > > > + > > > +#define MCP3911_REG_STATUSCOM0x0a > > > +#define MCP3911_STATUSCOM_CH1_24WIDTHBIT(4) > > > +#define MCP3911_STATUSCOM_CH0_24WIDTHBIT(3) > > > +#define MCP3911_STATUSCOM_EN_OFFCAL BIT(2) > > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > > + > > > +#define MCP3911_REG_CONFIG 0x0c > > > +#define MCP3911_CONFIG_
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Peter, Thanks for the comments! On Sat, Jul 21, 2018 at 11:19:48PM +0200, Peter Meerwald-Stadler wrote: > Hello, > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > some comments below... > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 444 > > ++ > > 3 files changed, 455 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..be74cb15827b > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,444 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > + > > +#define MCP3911_REG_GAIN 0x09 > > +#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch) > > space around * operator, maybe parenthesis around variable, i.e > (0x07 << (3 * (ch))) > Will do > > +#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch)) > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14 > > +#define MCP3911_REG_GAINCAL_CH10x17 > > +#define MCP3911_REG_VREFCAL0x1a > > + > > +#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3) > > +#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6) > > +#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6) > > + > > delete newline > Will do > > + > > +/* Internal voltage reference in uV */ > > +#define MCP3911_INT_VREF_UV120 > > + > > +#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff) > > +#define REG_WRITE(reg, id) (((reg << 1) | (id &
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Peter, Thanks for the comments! On Sat, Jul 21, 2018 at 11:19:48PM +0200, Peter Meerwald-Stadler wrote: > Hello, > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > some comments below... > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 444 > > ++ > > 3 files changed, 455 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..be74cb15827b > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,444 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > + > > +#define MCP3911_REG_GAIN 0x09 > > +#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch) > > space around * operator, maybe parenthesis around variable, i.e > (0x07 << (3 * (ch))) > Will do > > +#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch)) > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14 > > +#define MCP3911_REG_GAINCAL_CH10x17 > > +#define MCP3911_REG_VREFCAL0x1a > > + > > +#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3) > > +#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6) > > +#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6) > > + > > delete newline > Will do > > + > > +/* Internal voltage reference in uV */ > > +#define MCP3911_INT_VREF_UV120 > > + > > +#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff) > > +#define REG_WRITE(reg, id) (((reg << 1) | (id &
[PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
.../devicetree/bindings/iio/adc/mcp3911.txt| 33 ++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..e233ee94ad96
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,33 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - external-clock: Use external clock instead of crystal oscillator.
+ - external-vref: Use external voltage reference
+ - vref-supply:Phandle to the external reference voltage supply. (only
valid in combination with `external-vref`)
+ - ch0-width: width for channel0. Valid widths are 16 and 24bits.
+ - ch1-width: width for channel1. Valid widths are 16 and 24bits.
+
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ ch0-width = <16>;
+ ch1-width = <24>;
+ external-clock;
+ external-vref;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
.../devicetree/bindings/iio/adc/mcp3911.txt| 33 ++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..e233ee94ad96
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,33 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - external-clock: Use external clock instead of crystal oscillator.
+ - external-vref: Use external voltage reference
+ - vref-supply:Phandle to the external reference voltage supply. (only
valid in combination with `external-vref`)
+ - ch0-width: width for channel0. Valid widths are 16 and 24bits.
+ - ch1-width: width for channel1. Valid widths are 16 and 24bits.
+
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ ch0-width = <16>;
+ ch1-width = <24>;
+ external-clock;
+ external-vref;
+ vref-supply = <_reg>;
+};
--
2.11.0.rc2
[PATCH 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 444 ++
3 files changed, 455 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..be74cb15827b
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+
+#define MCP3911_REG_GAIN 0x09
+#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch)
+#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch))
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6)
+
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff)
+#define REG_WRITE(reg, id) (((reg << 1) | (id << 5) | (0 << 0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct device_node *np;
+ struct mutex lock;
+
+ u32 gain[MCP3911_NUM_CHANNELS];
+ u32 width[MCP3911_NUM_CHANNELS];
+
+ u32 dev_addr;
+ bool vrefext;
+ struct regulator *vref;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ *val <<= ((4-len)*8);
+ be32_to_cpus(val);
+ dev_dbg(>spi->dev, "Reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "Writing 0x%x to register 0x%x\n", val, reg);
+
+ cpu_to_be32s();
+ val >>= (3-len)*8;
+ val |= REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, , len+1);
+}
+
+static int mcp3911_update(struct mcp3911 *adc, u8 reg, u32 mask,
+ u32 val, u8 len)
+{
+ u32 tmp;
+ int ret;
+
+ ret = mcp3911_read(adc, reg, ,
[PATCH 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 444 ++
3 files changed, 455 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..be74cb15827b
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+
+#define MCP3911_REG_GAIN 0x09
+#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch)
+#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch))
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6)
+
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff)
+#define REG_WRITE(reg, id) (((reg << 1) | (id << 5) | (0 << 0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct device_node *np;
+ struct mutex lock;
+
+ u32 gain[MCP3911_NUM_CHANNELS];
+ u32 width[MCP3911_NUM_CHANNELS];
+
+ u32 dev_addr;
+ bool vrefext;
+ struct regulator *vref;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, , 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ *val <<= ((4-len)*8);
+ be32_to_cpus(val);
+ dev_dbg(>spi->dev, "Reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(>spi->dev, "Writing 0x%x to register 0x%x\n", val, reg);
+
+ cpu_to_be32s();
+ val >>= (3-len)*8;
+ val |= REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, , len+1);
+}
+
+static int mcp3911_update(struct mcp3911 *adc, u8 reg, u32 mask,
+ u32 val, u8 len)
+{
+ u32 tmp;
+ int ret;
+
+ ret = mcp3911_read(adc, reg, ,
[PATCH] input: pxrc - do not store USB device in private struct
The USB device is only needed during setup, so put it back after
initialization and do not store it in our private struct.
Also, the USB device is a parent of USB interface so our driver
model rules ensure that USB device should not disappear while
interface device is still there.
So not keep a refcount on the device is safe.
Reported-by: Alexey Khoroshilov
Signed-off-by: Marcus Folkesson
---
drivers/input/joystick/pxrc.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
index 07a0dbd3ced2..46a7acb747bf 100644
--- a/drivers/input/joystick/pxrc.c
+++ b/drivers/input/joystick/pxrc.c
@@ -27,7 +27,6 @@ MODULE_DEVICE_TABLE(usb, pxrc_table);
struct pxrc {
struct input_dev*input;
- struct usb_device *udev;
struct usb_interface*intf;
struct urb *urb;
struct mutexpm_mutex;
@@ -120,7 +119,7 @@ static void pxrc_close(struct input_dev *input)
mutex_unlock(>pm_mutex);
}
-static int pxrc_usb_init(struct pxrc *pxrc)
+static int pxrc_usb_init(struct pxrc *pxrc, struct usb_device *udev)
{
struct usb_endpoint_descriptor *epirq;
unsigned int pipe;
@@ -145,7 +144,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
usb_set_intfdata(pxrc->intf, pxrc);
- usb_make_path(pxrc->udev, pxrc->phys, sizeof(pxrc->phys));
+ usb_make_path(udev, pxrc->phys, sizeof(pxrc->phys));
strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys));
pxrc->urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -154,8 +153,8 @@ static int pxrc_usb_init(struct pxrc *pxrc)
goto error;
}
- pipe = usb_rcvintpipe(pxrc->udev, pxrc->epaddr),
- usb_fill_int_urb(pxrc->urb, pxrc->udev, pipe, pxrc->data, pxrc->bsize,
+ pipe = usb_rcvintpipe(udev, pxrc->epaddr),
+ usb_fill_int_urb(pxrc->urb, udev, pipe, pxrc->data, pxrc->bsize,
pxrc_usb_irq, pxrc, 1);
error:
@@ -164,7 +163,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
-static int pxrc_input_init(struct pxrc *pxrc)
+static int pxrc_input_init(struct pxrc *pxrc, struct usb_device *udev)
{
pxrc->input = devm_input_allocate_device(>intf->dev);
if (pxrc->input == NULL) {
@@ -174,7 +173,7 @@ static int pxrc_input_init(struct pxrc *pxrc)
pxrc->input->name = "PXRC Flight Controller Adapter";
pxrc->input->phys = pxrc->phys;
- usb_to_input_id(pxrc->udev, >input->id);
+ usb_to_input_id(udev, >input->id);
pxrc->input->open = pxrc_open;
pxrc->input->close = pxrc_close;
@@ -197,6 +196,7 @@ static int pxrc_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct pxrc *pxrc;
+ struct usb_device *udev;
int retval;
pxrc = devm_kzalloc(>dev, sizeof(*pxrc), GFP_KERNEL);
@@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf,
return -ENOMEM;
mutex_init(>pm_mutex);
- pxrc->udev = usb_get_dev(interface_to_usbdev(intf));
+ udev = usb_get_dev(interface_to_usbdev(intf));
pxrc->intf = intf;
- retval = pxrc_usb_init(pxrc);
+ retval = pxrc_usb_init(pxrc, udev);
if (retval)
goto error;
- retval = pxrc_input_init(pxrc);
+ retval = pxrc_input_init(pxrc, udev);
if (retval)
goto err_free_urb;
+ usb_put_dev(udev);
return 0;
err_free_urb:
usb_free_urb(pxrc->urb);
error:
+ usb_put_dev(udev);
return retval;
}
--
2.18.0
[PATCH] input: pxrc - do not store USB device in private struct
The USB device is only needed during setup, so put it back after
initialization and do not store it in our private struct.
Also, the USB device is a parent of USB interface so our driver
model rules ensure that USB device should not disappear while
interface device is still there.
So not keep a refcount on the device is safe.
Reported-by: Alexey Khoroshilov
Signed-off-by: Marcus Folkesson
---
drivers/input/joystick/pxrc.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
index 07a0dbd3ced2..46a7acb747bf 100644
--- a/drivers/input/joystick/pxrc.c
+++ b/drivers/input/joystick/pxrc.c
@@ -27,7 +27,6 @@ MODULE_DEVICE_TABLE(usb, pxrc_table);
struct pxrc {
struct input_dev*input;
- struct usb_device *udev;
struct usb_interface*intf;
struct urb *urb;
struct mutexpm_mutex;
@@ -120,7 +119,7 @@ static void pxrc_close(struct input_dev *input)
mutex_unlock(>pm_mutex);
}
-static int pxrc_usb_init(struct pxrc *pxrc)
+static int pxrc_usb_init(struct pxrc *pxrc, struct usb_device *udev)
{
struct usb_endpoint_descriptor *epirq;
unsigned int pipe;
@@ -145,7 +144,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
usb_set_intfdata(pxrc->intf, pxrc);
- usb_make_path(pxrc->udev, pxrc->phys, sizeof(pxrc->phys));
+ usb_make_path(udev, pxrc->phys, sizeof(pxrc->phys));
strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys));
pxrc->urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -154,8 +153,8 @@ static int pxrc_usb_init(struct pxrc *pxrc)
goto error;
}
- pipe = usb_rcvintpipe(pxrc->udev, pxrc->epaddr),
- usb_fill_int_urb(pxrc->urb, pxrc->udev, pipe, pxrc->data, pxrc->bsize,
+ pipe = usb_rcvintpipe(udev, pxrc->epaddr),
+ usb_fill_int_urb(pxrc->urb, udev, pipe, pxrc->data, pxrc->bsize,
pxrc_usb_irq, pxrc, 1);
error:
@@ -164,7 +163,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
-static int pxrc_input_init(struct pxrc *pxrc)
+static int pxrc_input_init(struct pxrc *pxrc, struct usb_device *udev)
{
pxrc->input = devm_input_allocate_device(>intf->dev);
if (pxrc->input == NULL) {
@@ -174,7 +173,7 @@ static int pxrc_input_init(struct pxrc *pxrc)
pxrc->input->name = "PXRC Flight Controller Adapter";
pxrc->input->phys = pxrc->phys;
- usb_to_input_id(pxrc->udev, >input->id);
+ usb_to_input_id(udev, >input->id);
pxrc->input->open = pxrc_open;
pxrc->input->close = pxrc_close;
@@ -197,6 +196,7 @@ static int pxrc_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct pxrc *pxrc;
+ struct usb_device *udev;
int retval;
pxrc = devm_kzalloc(>dev, sizeof(*pxrc), GFP_KERNEL);
@@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf,
return -ENOMEM;
mutex_init(>pm_mutex);
- pxrc->udev = usb_get_dev(interface_to_usbdev(intf));
+ udev = usb_get_dev(interface_to_usbdev(intf));
pxrc->intf = intf;
- retval = pxrc_usb_init(pxrc);
+ retval = pxrc_usb_init(pxrc, udev);
if (retval)
goto error;
- retval = pxrc_input_init(pxrc);
+ retval = pxrc_input_init(pxrc, udev);
if (retval)
goto err_free_urb;
+ usb_put_dev(udev);
return 0;
err_free_urb:
usb_free_urb(pxrc->urb);
error:
+ usb_put_dev(udev);
return retval;
}
--
2.18.0
Re: [PATCH] Input: pxrc - fix leak of usb_device
On Sat, Jul 14, 2018 at 08:51:09AM +, Dmitry Torokhov wrote: > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote: > > Hi Alexey, > > > > Good catch! > > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > > > anywhere in the driver. > > > > > > The patch adds one to error handling code and to pxrc_disconnect(). > > > > > > Found by Linux Driver Verification project (linuxtesting.org). > > > > > > Signed-off-by: Alexey Khoroshilov > > > > Reviewed-by: Marcus Folkesson > > Hmm, the biggest question however if we need to "take" the device, as I > do not think interface can outlive the device, and whether we actually > need to store it in pxrc, as we only need it during set up, as far as I > can see. Yep, the device is only used during setup. I interpret the comments for usb_get_dev() as you should take a reference count on the device even if you only use the interface, but I could be wrong. >From usb_get_dev():: * usb_get_dev - increments the reference count of the usb device structure * @dev: the device being referenced * * Each live reference to a device should be refcounted. * * Drivers for USB interfaces should normally record such references in * their probe() methods, when they bind to an interface, and release * them by calling usb_put_dev(), in their disconnect() methods. I can fix the driver to not take the device if that is what we want. If not Alexey want to fix it of course, it is his catch :-) > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson
Re: [PATCH] Input: pxrc - fix leak of usb_device
On Sat, Jul 14, 2018 at 08:51:09AM +, Dmitry Torokhov wrote: > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote: > > Hi Alexey, > > > > Good catch! > > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > > > anywhere in the driver. > > > > > > The patch adds one to error handling code and to pxrc_disconnect(). > > > > > > Found by Linux Driver Verification project (linuxtesting.org). > > > > > > Signed-off-by: Alexey Khoroshilov > > > > Reviewed-by: Marcus Folkesson > > Hmm, the biggest question however if we need to "take" the device, as I > do not think interface can outlive the device, and whether we actually > need to store it in pxrc, as we only need it during set up, as far as I > can see. Yep, the device is only used during setup. I interpret the comments for usb_get_dev() as you should take a reference count on the device even if you only use the interface, but I could be wrong. >From usb_get_dev():: * usb_get_dev - increments the reference count of the usb device structure * @dev: the device being referenced * * Each live reference to a device should be refcounted. * * Drivers for USB interfaces should normally record such references in * their probe() methods, when they bind to an interface, and release * them by calling usb_put_dev(), in their disconnect() methods. I can fix the driver to not take the device if that is what we want. If not Alexey want to fix it of course, it is his catch :-) > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson
Re: [PATCH] Input: pxrc - fix leak of usb_device
Hi Alexey, Good catch! On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > anywhere in the driver. > > The patch adds one to error handling code and to pxrc_disconnect(). > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov Reviewed-by: Marcus Folkesson > --- > drivers/input/joystick/pxrc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > index 07a0dbd3ced2..0a31de63ac8e 100644 > --- a/drivers/input/joystick/pxrc.c > +++ b/drivers/input/joystick/pxrc.c > @@ -221,6 +221,7 @@ static int pxrc_probe(struct usb_interface *intf, > usb_free_urb(pxrc->urb); > > error: > + usb_put_dev(pxrc->udev); > return retval; > } > > @@ -229,6 +230,7 @@ static void pxrc_disconnect(struct usb_interface *intf) > struct pxrc *pxrc = usb_get_intfdata(intf); > > usb_free_urb(pxrc->urb); > + usb_put_dev(pxrc->udev); > usb_set_intfdata(intf, NULL); > } > > -- > 2.7.4 >
Re: [PATCH] Input: pxrc - fix leak of usb_device
Hi Alexey, Good catch! On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > anywhere in the driver. > > The patch adds one to error handling code and to pxrc_disconnect(). > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov Reviewed-by: Marcus Folkesson > --- > drivers/input/joystick/pxrc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > index 07a0dbd3ced2..0a31de63ac8e 100644 > --- a/drivers/input/joystick/pxrc.c > +++ b/drivers/input/joystick/pxrc.c > @@ -221,6 +221,7 @@ static int pxrc_probe(struct usb_interface *intf, > usb_free_urb(pxrc->urb); > > error: > + usb_put_dev(pxrc->udev); > return retval; > } > > @@ -229,6 +230,7 @@ static void pxrc_disconnect(struct usb_interface *intf) > struct pxrc *pxrc = usb_get_intfdata(intf); > > usb_free_urb(pxrc->urb); > + usb_put_dev(pxrc->udev); > usb_set_intfdata(intf, NULL); > } > > -- > 2.7.4 >
[PATCH v4 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Add documentation to give a brief description on how to use the CCID Gadget Device. This includes a description for all attributes followed by an example on how to setup the device with ConfigFS. Reviewed-by: Randy Dunlap Signed-off-by: Marcus Folkesson --- Notes: v4: - clarify how to use /dev/ccidg and BULK endpoind (thanks Randy) v3: - correct the grammer (thanks Randy) v2: - add the missing changelog text Documentation/usb/gadget_ccid.rst | 269 ++ 1 file changed, 269 insertions(+) create mode 100644 Documentation/usb/gadget_ccid.rst diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst new file mode 100644 index ..c8f25080c9e7 --- /dev/null +++ b/Documentation/usb/gadget_ccid.rst @@ -0,0 +1,269 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +CCID Gadget + + +:Author: Marcus Folkesson + +Introduction + + +The CCID Gadget will present itself as a CCID device to the host system. +The device supports two endpoints for now; BULK IN and BULK OUT. +These endpoints are exposed to userspace via /dev/ccidgX where `X` is +the device enumeration number. +Use read(2) and write(3) to operate on BULK_IN and BULK_OUT respectively. + +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID +has an associated ending response. Some commands can also have intermediate +responses. The response is sent on the BULK-IN endpoint. +See Figure 3-3 in the CCID Specification [1]_ for more details. + +The CCID commands must be handled in userspace since the driver is only working +as a transport layer for the TPDUs. + + +CCID Commands +-- + +All CCID commands begins with a 10-byte header followed by an optional +data field depending on message type. + +++--+---+--+ +| Offset | Field| Size | Description | +++==+===+==+ +| 0 | bMessageType | 1 | Type of message | +++--+---+--+ +| 1 | dwLength | 4 | Message specific data length | +|| | | | +++--+---+--+ +| 5 | bSlot| 1 | Identifies the slot number | +|| | | for this command | +++--+---+--+ +| 6 | bSeq | 1 | Sequence number for command | +++--+---+--+ +| 7 | ... | 3 | Fields depends on message type | +++--+---+--+ +| 10 | abData | array | Message specific data (OPTIONAL) | +++--+---+--+ + + +Multiple CCID gadgets +-- + +It is possible to create multiple instances of the CCID gadget, however, +a much more flexible way is to create one gadget and set the `nslots` attribute +to the number of desired CCID devices. + +All CCID commands specify which slot is the receiver in the `bSlot` field +of the CCID header. + +Usage += + +Access from userspace +-- +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device. +Only one file descriptor is allowed to be open to the device at a time. + +The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + +The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + + +Configuration with configfs + + +ConfigFS is used to create and configure the CCID gadget. +In order to get a device to work as intended, a few attributes must +be considered. + +The attributes are described below followed by an example. + +features +~ + +The `feature` attribute writes to the dwFeatures field in the class descriptor. +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_. + +The value indicates what intelligent features the CCID has. +These values are available to user application as defined in ccid.h [2]_. +The default value is 0x. + +The value is a bitwise OR operation performed on the following values: + ++++ +| Value | Description| ++++ +| 0x | No special characteristics
[PATCH v4 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Add documentation to give a brief description on how to use the CCID Gadget Device. This includes a description for all attributes followed by an example on how to setup the device with ConfigFS. Reviewed-by: Randy Dunlap Signed-off-by: Marcus Folkesson --- Notes: v4: - clarify how to use /dev/ccidg and BULK endpoind (thanks Randy) v3: - correct the grammer (thanks Randy) v2: - add the missing changelog text Documentation/usb/gadget_ccid.rst | 269 ++ 1 file changed, 269 insertions(+) create mode 100644 Documentation/usb/gadget_ccid.rst diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst new file mode 100644 index ..c8f25080c9e7 --- /dev/null +++ b/Documentation/usb/gadget_ccid.rst @@ -0,0 +1,269 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +CCID Gadget + + +:Author: Marcus Folkesson + +Introduction + + +The CCID Gadget will present itself as a CCID device to the host system. +The device supports two endpoints for now; BULK IN and BULK OUT. +These endpoints are exposed to userspace via /dev/ccidgX where `X` is +the device enumeration number. +Use read(2) and write(3) to operate on BULK_IN and BULK_OUT respectively. + +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID +has an associated ending response. Some commands can also have intermediate +responses. The response is sent on the BULK-IN endpoint. +See Figure 3-3 in the CCID Specification [1]_ for more details. + +The CCID commands must be handled in userspace since the driver is only working +as a transport layer for the TPDUs. + + +CCID Commands +-- + +All CCID commands begins with a 10-byte header followed by an optional +data field depending on message type. + +++--+---+--+ +| Offset | Field| Size | Description | +++==+===+==+ +| 0 | bMessageType | 1 | Type of message | +++--+---+--+ +| 1 | dwLength | 4 | Message specific data length | +|| | | | +++--+---+--+ +| 5 | bSlot| 1 | Identifies the slot number | +|| | | for this command | +++--+---+--+ +| 6 | bSeq | 1 | Sequence number for command | +++--+---+--+ +| 7 | ... | 3 | Fields depends on message type | +++--+---+--+ +| 10 | abData | array | Message specific data (OPTIONAL) | +++--+---+--+ + + +Multiple CCID gadgets +-- + +It is possible to create multiple instances of the CCID gadget, however, +a much more flexible way is to create one gadget and set the `nslots` attribute +to the number of desired CCID devices. + +All CCID commands specify which slot is the receiver in the `bSlot` field +of the CCID header. + +Usage += + +Access from userspace +-- +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device. +Only one file descriptor is allowed to be open to the device at a time. + +The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + +The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + + +Configuration with configfs + + +ConfigFS is used to create and configure the CCID gadget. +In order to get a device to work as intended, a few attributes must +be considered. + +The attributes are described below followed by an example. + +features +~ + +The `feature` attribute writes to the dwFeatures field in the class descriptor. +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_. + +The value indicates what intelligent features the CCID has. +These values are available to user application as defined in ccid.h [2]_. +The default value is 0x. + +The value is a bitwise OR operation performed on the following values: + ++++ +| Value | Description| ++++ +| 0x | No special characteristics
