* [PATCH v3 0/3] iio: adc: add support for ltc2496
@ 2019-11-21 21:00 Uwe Kleine-König
2019-11-21 21:00 ` [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2019-11-21 21:00 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
Michael Hennerich, Stefan Popa, Alexandru Ardelean
Cc: linux-iio, kernel
Hello,
this is v3 of a series generalizing the driver of the ltc2497 (an i2c
ADC) to be able to reuse most of the code for the ltc2496 (an spi
variant of the ltc2497).
Iteration v2 started with Message-Id:
20191114105159.14195-1-u.kleine-koenig@pengutronix.de.
The changes since v2 are based on feedback by Jonathan Cameron:
- rename common file to ltc2497-core.c (from ltc249x.c), also don't use
ltc249x in names for cpp symbols, functions and variables.
- properly align spi and i2c transfer buffers for DMA maintenance
- improve dt binding to mention spi specific properties
Best regards
Uwe
Uwe Kleine-König (3):
iio: adc: ltc2496: provide device tree binding document
iio: adc: ltc2497: split protocol independent part in a separate
module
iio: adc: new driver to support Linear technology's ltc2496
.../bindings/iio/adc/lltc,ltc2496.yaml | 37 +++
MAINTAINERS | 2 +-
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 3 +-
drivers/iio/adc/ltc2496.c | 108 ++++++++
drivers/iio/adc/ltc2497-core.c | 243 ++++++++++++++++++
drivers/iio/adc/ltc2497.c | 234 ++---------------
drivers/iio/adc/ltc2497.h | 18 ++
8 files changed, 445 insertions(+), 210 deletions(-)
create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
create mode 100644 drivers/iio/adc/ltc2496.c
create mode 100644 drivers/iio/adc/ltc2497-core.c
create mode 100644 drivers/iio/adc/ltc2497.h
--
2.24.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document 2019-11-21 21:00 [PATCH v3 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König @ 2019-11-21 21:00 ` Uwe Kleine-König 2019-12-01 11:28 ` Jonathan Cameron 2019-11-21 21:00 ` [PATCH v3 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König 2 siblings, 1 reply; 10+ messages in thread From: Uwe Kleine-König @ 2019-11-21 21:00 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean Cc: linux-iio, kernel The ADC only requires the standard stuff for spi devices and a reference voltage. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- .../bindings/iio/adc/lltc,ltc2496.yaml | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml new file mode 100644 index 000000000000..af485abeabd6 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml @@ -0,0 +1,37 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2496.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Linear Technology / Analog Devices LTC2496 ADC + +properties: + compatible: + enum: + - lltc,ltc2496 + + vref-supply: + description: phandle to an external regulator providing the reference voltage + allOf: + - $ref: /schemas/types.yaml#/definitions/phandle + + reg: + description: spi chipselect number according to the usual spi bindings + + spi-max-frequency: + description: maximal spi bus frequency supported by the chip + +required: + - compatible + - vref-supply + - reg + +examples: + - | + adc@0 { + compatible = "lltc,ltc2496"; + reg = <0>; + vref-supply = <<c2496_reg>; + spi-max-frequency = <2000000>; + }; -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document 2019-11-21 21:00 ` [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König @ 2019-12-01 11:28 ` Jonathan Cameron 2019-12-09 15:18 ` Uwe Kleine-König 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2019-12-01 11:28 UTC (permalink / raw) To: Uwe Kleine-König Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel On Thu, 21 Nov 2019 22:00:05 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > The ADC only requires the standard stuff for spi devices and a reference > voltage. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Rob, even with the issues below fixed I can't build test this and get 'no schema found in file'. I can't seem to figure out why so if you could take a look, that would be great. Thanks, Jonathan > --- > .../bindings/iio/adc/lltc,ltc2496.yaml | 37 +++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml > new file mode 100644 > index 000000000000..af485abeabd6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml > @@ -0,0 +1,37 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2496.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Linear Technology / Analog Devices LTC2496 ADC > + > +properties: > + compatible: > + enum: > + - lltc,ltc2496 > + > + vref-supply: > + description: phandle to an external regulator providing the reference voltage > + allOf: > + - $ref: /schemas/types.yaml#/definitions/phandle > + > + reg: > + description: spi chipselect number according to the usual spi bindings > + > + spi-max-frequency: > + description: maximal spi bus frequency supported by the chip > + > +required: > + - compatible > + - vref-supply > + - reg > + > +examples: > + - | Missed this before trying to build test. Spaces used in DT not tabs, and this should be in an spi block. Please check the example verifies using the instructions in Documentation/devicetree/bindings/writing-bindings.rst Thanks, Jonathan > + adc@0 { > + compatible = "lltc,ltc2496"; > + reg = <0>; > + vref-supply = <<c2496_reg>; > + spi-max-frequency = <2000000>; > + }; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document 2019-12-01 11:28 ` Jonathan Cameron @ 2019-12-09 15:18 ` Uwe Kleine-König 2019-12-09 20:34 ` Uwe Kleine-König 0 siblings, 1 reply; 10+ messages in thread From: Uwe Kleine-König @ 2019-12-09 15:18 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel Hello, On Sun, Dec 01, 2019 at 11:28:03AM +0000, Jonathan Cameron wrote: > On Thu, 21 Nov 2019 22:00:05 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > The ADC only requires the standard stuff for spi devices and a reference > > voltage. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Rob, even with the issues below fixed I can't build test this and get > 'no schema found in file'. > > I can't seem to figure out why so if you could take a look, that would > be great. > > > [...] > > Missed this before trying to build test. > > Spaces used in DT not tabs, and this should be in an spi block. > > Please check the example verifies using the instructions in > Documentation/devicetree/bindings/writing-bindings.rst I'm not sure if it's me or Rob you are waiting for now. I just setup dt-schema stuff locally to find out what to improve. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document 2019-12-09 15:18 ` Uwe Kleine-König @ 2019-12-09 20:34 ` Uwe Kleine-König 0 siblings, 0 replies; 10+ messages in thread From: Uwe Kleine-König @ 2019-12-09 20:34 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel On Mon, Dec 09, 2019 at 04:18:01PM +0100, Uwe Kleine-König wrote: > Hello, > > On Sun, Dec 01, 2019 at 11:28:03AM +0000, Jonathan Cameron wrote: > > On Thu, 21 Nov 2019 22:00:05 +0100 > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > The ADC only requires the standard stuff for spi devices and a reference > > > voltage. > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > Rob, even with the issues below fixed I can't build test this and get > > 'no schema found in file'. > > > > I can't seem to figure out why so if you could take a look, that would > > be great. > > > > > [...] > > > > Missed this before trying to build test. > > > > Spaces used in DT not tabs, and this should be in an spi block. > > > > Please check the example verifies using the instructions in > > Documentation/devicetree/bindings/writing-bindings.rst > > I'm not sure if it's me or Rob you are waiting for now. > > I just setup dt-schema stuff locally to find out what to improve. I managed to make the dt check target happy and created a v4 with the result. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] iio: adc: ltc2497: split protocol independent part in a separate module 2019-11-21 21:00 [PATCH v3 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König @ 2019-11-21 21:00 ` Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König 2 siblings, 0 replies; 10+ messages in thread From: Uwe Kleine-König @ 2019-11-21 21:00 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean Cc: linux-iio, kernel This allows to share most of this driver for the ltc2496 driver added in the next commit that is an SPI variant of the ltc2497. Initially I named the generic part ltc249x, but wild card names are frowned upon, so the generic part is called ltc2497-core even though it's not obvious that this is then to be reused for the ltc2496 driver. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/iio/adc/Makefile | 2 +- drivers/iio/adc/ltc2497-core.c | 243 +++++++++++++++++++++++++++++++++ drivers/iio/adc/ltc2497.c | 234 ++++--------------------------- drivers/iio/adc/ltc2497.h | 18 +++ 4 files changed, 288 insertions(+), 209 deletions(-) create mode 100644 drivers/iio/adc/ltc2497-core.c create mode 100644 drivers/iio/adc/ltc2497.h diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index ef9cc485fb67..ee0c8dcfb501 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -47,7 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o -obj-$(CONFIG_LTC2497) += ltc2497.o +obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o obj-$(CONFIG_MAX1027) += max1027.o obj-$(CONFIG_MAX11100) += max11100.o obj-$(CONFIG_MAX1118) += max1118.o diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c new file mode 100644 index 000000000000..f5f7039caacc --- /dev/null +++ b/drivers/iio/adc/ltc2497-core.c @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ltc2497-core.c - Common code for Analog Devices/Linear Technology + * LTC2496 and LTC2497 ADCs + * + * Copyright (C) 2017 Analog Devices Inc. + */ + +#include <linux/delay.h> +#include <linux/iio/iio.h> +#include <linux/iio/driver.h> +#include <linux/module.h> +#include <linux/regulator/consumer.h> + +#include "ltc2497.h" + +#define LTC2497_SGL BIT(4) +#define LTC2497_DIFF 0 +#define LTC2497_SIGN BIT(3) + +static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata) +{ + s64 time_elapsed; + + time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev); + + if (time_elapsed < LTC2497_CONVERSION_TIME_MS) { + /* delay if conversion time not passed + * since last read or write + */ + if (msleep_interruptible( + LTC2497_CONVERSION_TIME_MS - time_elapsed)) + return -ERESTARTSYS; + + return 0; + } + + if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) { + /* We're in automatic mode - + * so the last reading is still not outdated + */ + return 0; + } + + return 1; +} + +static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, int *val) +{ + int ret; + + ret = ltc2497core_wait_conv(ddata); + if (ret < 0) + return ret; + + if (ret || ddata->addr_prev != address) { + ret = ddata->result_and_measure(ddata, address, NULL); + if (ret < 0) + return ret; + ddata->addr_prev = address; + + if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS)) + return -ERESTARTSYS; + } + + ret = ddata->result_and_measure(ddata, address, val); + if (ret < 0) + return ret; + + ddata->time_prev = ktime_get(); + + return ret; +} + +static int ltc2497core_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct ltc2497core_driverdata *ddata = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + mutex_lock(&indio_dev->mlock); + ret = ltc2497core_read(ddata, chan->address, val); + mutex_unlock(&indio_dev->mlock); + if (ret < 0) + return ret; + + return IIO_VAL_INT; + + case IIO_CHAN_INFO_SCALE: + ret = regulator_get_voltage(ddata->ref); + if (ret < 0) + return ret; + + *val = ret / 1000; + *val2 = 17; + + return IIO_VAL_FRACTIONAL_LOG2; + + default: + return -EINVAL; + } +} + +#define LTC2497_CHAN(_chan, _addr, _ds_name) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (_chan), \ + .address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .datasheet_name = (_ds_name), \ +} + +#define LTC2497_CHAN_DIFF(_chan, _addr) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \ + .channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\ + .address = (_addr | _chan), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .differential = 1, \ +} + +static const struct iio_chan_spec ltc2497core_channel[] = { + LTC2497_CHAN(0, LTC2497_SGL, "CH0"), + LTC2497_CHAN(1, LTC2497_SGL, "CH1"), + LTC2497_CHAN(2, LTC2497_SGL, "CH2"), + LTC2497_CHAN(3, LTC2497_SGL, "CH3"), + LTC2497_CHAN(4, LTC2497_SGL, "CH4"), + LTC2497_CHAN(5, LTC2497_SGL, "CH5"), + LTC2497_CHAN(6, LTC2497_SGL, "CH6"), + LTC2497_CHAN(7, LTC2497_SGL, "CH7"), + LTC2497_CHAN(8, LTC2497_SGL, "CH8"), + LTC2497_CHAN(9, LTC2497_SGL, "CH9"), + LTC2497_CHAN(10, LTC2497_SGL, "CH10"), + LTC2497_CHAN(11, LTC2497_SGL, "CH11"), + LTC2497_CHAN(12, LTC2497_SGL, "CH12"), + LTC2497_CHAN(13, LTC2497_SGL, "CH13"), + LTC2497_CHAN(14, LTC2497_SGL, "CH14"), + LTC2497_CHAN(15, LTC2497_SGL, "CH15"), + LTC2497_CHAN_DIFF(0, LTC2497_DIFF), + LTC2497_CHAN_DIFF(1, LTC2497_DIFF), + LTC2497_CHAN_DIFF(2, LTC2497_DIFF), + LTC2497_CHAN_DIFF(3, LTC2497_DIFF), + LTC2497_CHAN_DIFF(4, LTC2497_DIFF), + LTC2497_CHAN_DIFF(5, LTC2497_DIFF), + LTC2497_CHAN_DIFF(6, LTC2497_DIFF), + LTC2497_CHAN_DIFF(7, LTC2497_DIFF), + LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN), + LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN), +}; + +static const struct iio_info ltc2497core_info = { + .read_raw = ltc2497core_read_raw, +}; + +int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev) +{ + struct ltc2497core_driverdata *ddata = iio_priv(indio_dev); + int ret; + + indio_dev->dev.parent = dev; + indio_dev->name = dev_name(dev); + indio_dev->info = <c2497core_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = ltc2497core_channel; + indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel); + + ret = ddata->result_and_measure(ddata, LTC2497_CONFIG_DEFAULT, NULL); + if (ret < 0) + return ret; + + ddata->ref = devm_regulator_get(dev, "vref"); + if (IS_ERR(ddata->ref)) { + if (PTR_ERR(ddata->ref) != -EPROBE_DEFER) + dev_err(dev, "Failed to get vref regulator: %pe\n", + ddata->ref); + + return PTR_ERR(ddata->ref); + } + + ret = regulator_enable(ddata->ref); + if (ret < 0) { + dev_err(dev, "Failed to enable vref regulator: %pe\n", + ERR_PTR(ret)); + return ret; + } + + if (dev->platform_data) { + struct iio_map *plat_data; + + plat_data = (struct iio_map *)dev->platform_data; + + ret = iio_map_array_register(indio_dev, plat_data); + if (ret) { + dev_err(&indio_dev->dev, "iio map err: %d\n", ret); + goto err_regulator_disable; + } + } + + ddata->addr_prev = LTC2497_CONFIG_DEFAULT; + ddata->time_prev = ktime_get(); + + ret = iio_device_register(indio_dev); + if (ret < 0) + goto err_array_unregister; + + return 0; + +err_array_unregister: + iio_map_array_unregister(indio_dev); + +err_regulator_disable: + regulator_disable(ddata->ref); + + return ret; +} +EXPORT_SYMBOL_NS(ltc2497core_probe, LTC2497); + +void ltc2497core_remove(struct iio_dev *indio_dev) +{ + struct ltc2497core_driverdata *ddata = iio_priv(indio_dev); + + iio_device_unregister(indio_dev); + + iio_map_array_unregister(indio_dev); + + regulator_disable(ddata->ref); +} +EXPORT_SYMBOL_NS(ltc2497core_remove, LTC2497); + +MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c index 470406032720..5db63d7c6bc5 100644 --- a/drivers/iio/adc/ltc2497.c +++ b/drivers/iio/adc/ltc2497.c @@ -7,27 +7,18 @@ * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf */ -#include <linux/delay.h> #include <linux/i2c.h> #include <linux/iio/iio.h> #include <linux/iio/driver.h> -#include <linux/iio/sysfs.h> #include <linux/module.h> #include <linux/of.h> -#include <linux/regulator/consumer.h> -#define LTC2497_ENABLE 0xA0 -#define LTC2497_SGL BIT(4) -#define LTC2497_DIFF 0 -#define LTC2497_SIGN BIT(3) -#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE -#define LTC2497_CONVERSION_TIME_MS 150ULL +#include "ltc2497.h" -struct ltc2497_st { +struct ltc2497_driverdata { + /* this must be the first member */ + struct ltc2497core_driverdata common_ddata; struct i2c_client *client; - struct regulator *ref; - ktime_t time_prev; - u8 addr_prev; /* * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache lines. @@ -35,232 +26,59 @@ struct ltc2497_st { __be32 buf ____cacheline_aligned; }; -static int ltc2497_wait_conv(struct ltc2497_st *st) +static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata, + u8 address, int *val) { - s64 time_elapsed; - - time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev); - - if (time_elapsed < LTC2497_CONVERSION_TIME_MS) { - /* delay if conversion time not passed - * since last read or write - */ - if (msleep_interruptible( - LTC2497_CONVERSION_TIME_MS - time_elapsed)) - return -ERESTARTSYS; - - return 0; - } - - if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) { - /* We're in automatic mode - - * so the last reading is stil not outdated - */ - return 0; - } - - return 1; -} - -static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val) -{ - struct i2c_client *client = st->client; - int ret; - - ret = ltc2497_wait_conv(st); - if (ret < 0) - return ret; - - if (ret || st->addr_prev != address) { - ret = i2c_smbus_write_byte(st->client, - LTC2497_ENABLE | address); - if (ret < 0) - return ret; - st->addr_prev = address; - if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS)) - return -ERESTARTSYS; - } - ret = i2c_master_recv(client, (char *)&st->buf, 3); - if (ret < 0) { - dev_err(&client->dev, "i2c_master_recv failed\n"); - return ret; - } - st->time_prev = ktime_get(); - - /* convert and shift the result, - * and finally convert from offset binary to signed integer - */ - *val = (be32_to_cpu(st->buf) >> 14) - (1 << 17); - - return ret; -} - -static int ltc2497_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, int *val2, long mask) -{ - struct ltc2497_st *st = iio_priv(indio_dev); + struct ltc2497_driverdata *st = + container_of(ddata, struct ltc2497_driverdata, common_ddata); int ret; - switch (mask) { - case IIO_CHAN_INFO_RAW: - mutex_lock(&indio_dev->mlock); - ret = ltc2497_read(st, chan->address, val); - mutex_unlock(&indio_dev->mlock); - if (ret < 0) - return ret; - - return IIO_VAL_INT; - - case IIO_CHAN_INFO_SCALE: - ret = regulator_get_voltage(st->ref); - if (ret < 0) + if (val) { + ret = i2c_master_recv(st->client, (char *)&st->buf, 3); + if (ret < 0) { + dev_err(&st->client->dev, "i2c_master_recv failed\n"); return ret; + } - *val = ret / 1000; - *val2 = 17; - - return IIO_VAL_FRACTIONAL_LOG2; - - default: - return -EINVAL; + *val = (be32_to_cpu(st->buf) >> 14) - (1 << 17); } -} -#define LTC2497_CHAN(_chan, _addr, _ds_name) { \ - .type = IIO_VOLTAGE, \ - .indexed = 1, \ - .channel = (_chan), \ - .address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \ - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ - .datasheet_name = (_ds_name), \ -} - -#define LTC2497_CHAN_DIFF(_chan, _addr) { \ - .type = IIO_VOLTAGE, \ - .indexed = 1, \ - .channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \ - .channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\ - .address = (_addr | _chan), \ - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ - .differential = 1, \ + ret = i2c_smbus_write_byte(st->client, + LTC2497_ENABLE | address); + if (ret) + dev_err(&st->client->dev, "i2c transfer failed: %pe\n", + ERR_PTR(ret)); + return ret; } -static const struct iio_chan_spec ltc2497_channel[] = { - LTC2497_CHAN(0, LTC2497_SGL, "CH0"), - LTC2497_CHAN(1, LTC2497_SGL, "CH1"), - LTC2497_CHAN(2, LTC2497_SGL, "CH2"), - LTC2497_CHAN(3, LTC2497_SGL, "CH3"), - LTC2497_CHAN(4, LTC2497_SGL, "CH4"), - LTC2497_CHAN(5, LTC2497_SGL, "CH5"), - LTC2497_CHAN(6, LTC2497_SGL, "CH6"), - LTC2497_CHAN(7, LTC2497_SGL, "CH7"), - LTC2497_CHAN(8, LTC2497_SGL, "CH8"), - LTC2497_CHAN(9, LTC2497_SGL, "CH9"), - LTC2497_CHAN(10, LTC2497_SGL, "CH10"), - LTC2497_CHAN(11, LTC2497_SGL, "CH11"), - LTC2497_CHAN(12, LTC2497_SGL, "CH12"), - LTC2497_CHAN(13, LTC2497_SGL, "CH13"), - LTC2497_CHAN(14, LTC2497_SGL, "CH14"), - LTC2497_CHAN(15, LTC2497_SGL, "CH15"), - LTC2497_CHAN_DIFF(0, LTC2497_DIFF), - LTC2497_CHAN_DIFF(1, LTC2497_DIFF), - LTC2497_CHAN_DIFF(2, LTC2497_DIFF), - LTC2497_CHAN_DIFF(3, LTC2497_DIFF), - LTC2497_CHAN_DIFF(4, LTC2497_DIFF), - LTC2497_CHAN_DIFF(5, LTC2497_DIFF), - LTC2497_CHAN_DIFF(6, LTC2497_DIFF), - LTC2497_CHAN_DIFF(7, LTC2497_DIFF), - LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN), - LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN), -}; - -static const struct iio_info ltc2497_info = { - .read_raw = ltc2497_read_raw, -}; - static int ltc2497_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct iio_dev *indio_dev; - struct ltc2497_st *st; - struct iio_map *plat_data; - int ret; + struct ltc2497_driverdata *st; + struct device *dev = &client->dev; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) return -EOPNOTSUPP; - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); if (!indio_dev) return -ENOMEM; st = iio_priv(indio_dev); i2c_set_clientdata(client, indio_dev); st->client = client; + st->common_ddata.result_and_measure = ltc2497_result_and_measure; - indio_dev->dev.parent = &client->dev; - indio_dev->name = id->name; - indio_dev->info = <c2497_info; - indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->channels = ltc2497_channel; - indio_dev->num_channels = ARRAY_SIZE(ltc2497_channel); - - st->ref = devm_regulator_get(&client->dev, "vref"); - if (IS_ERR(st->ref)) - return PTR_ERR(st->ref); - - ret = regulator_enable(st->ref); - if (ret < 0) - return ret; - - if (client->dev.platform_data) { - plat_data = ((struct iio_map *)client->dev.platform_data); - ret = iio_map_array_register(indio_dev, plat_data); - if (ret) { - dev_err(&indio_dev->dev, "iio map err: %d\n", ret); - goto err_regulator_disable; - } - } - - ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT); - if (ret < 0) - goto err_array_unregister; - - st->addr_prev = LTC2497_CONFIG_DEFAULT; - st->time_prev = ktime_get(); - - ret = iio_device_register(indio_dev); - if (ret < 0) - goto err_array_unregister; - - return 0; - -err_array_unregister: - iio_map_array_unregister(indio_dev); - -err_regulator_disable: - regulator_disable(st->ref); - - return ret; + return ltc2497core_probe(dev, indio_dev); } static int ltc2497_remove(struct i2c_client *client) { struct iio_dev *indio_dev = i2c_get_clientdata(client); - struct ltc2497_st *st = iio_priv(indio_dev); - iio_map_array_unregister(indio_dev); - iio_device_unregister(indio_dev); - regulator_disable(st->ref); + ltc2497core_remove(indio_dev); return 0; } diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h new file mode 100644 index 000000000000..d0b42dd6b8ad --- /dev/null +++ b/drivers/iio/adc/ltc2497.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#define LTC2497_ENABLE 0xA0 +#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE +#define LTC2497_CONVERSION_TIME_MS 150ULL + +struct ltc2497core_driverdata { + struct regulator *ref; + ktime_t time_prev; + u8 addr_prev; + int (*result_and_measure)(struct ltc2497core_driverdata *ddata, + u8 address, int *val); +}; + +int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev); +void ltc2497core_remove(struct iio_dev *indio_dev); + +MODULE_IMPORT_NS(LTC2497); -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 2019-11-21 21:00 [PATCH v3 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König @ 2019-11-21 21:00 ` Uwe Kleine-König 2019-11-23 17:12 ` Jonathan Cameron 2 siblings, 1 reply; 10+ messages in thread From: Uwe Kleine-König @ 2019-11-21 21:00 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean Cc: linux-iio, kernel This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C and so has a slightly different protocol. Only the actual hardware access is different. The spi protocol is different enough to not be able to map the differences via a regmap. Also generalize the entry in MAINTAINER to cover the newly introduced file. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- MAINTAINERS | 2 +- drivers/iio/adc/Kconfig | 10 ++++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ltc2496.c | 108 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/adc/ltc2496.c diff --git a/MAINTAINERS b/MAINTAINERS index eb19fad370d7..8b1211038617 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1029,7 +1029,7 @@ S: Supported F: Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9523 F: Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4350 F: drivers/iio/*/ad* -F: drivers/iio/adc/ltc2497* +F: drivers/iio/adc/ltc249* X: drivers/iio/*/adjd* F: drivers/staging/iio/*/ad* diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index f0af3a42f53c..deb86f6039b3 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -492,6 +492,16 @@ config LTC2485 To compile this driver as a module, choose M here: the module will be called ltc2485. +config LTC2496 + tristate "Linear Technology LTC2496 ADC driver" + depends on SPI + help + Say yes here to build support for Linear Technology LTC2496 + 16-Bit 8-/16-Channel Delta Sigma ADC. + + To compile this driver as a module, choose M here: the module will be + called ltc2496. + config LTC2497 tristate "Linear Technology LTC2497 ADC driver" depends on I2C diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index ee0c8dcfb501..c3dc2a12766a 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o obj-$(CONFIG_LTC2471) += ltc2471.o obj-$(CONFIG_LTC2485) += ltc2485.o +obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o obj-$(CONFIG_MAX1027) += max1027.o obj-$(CONFIG_MAX11100) += max11100.o diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c new file mode 100644 index 000000000000..a7659c8f9cc9 --- /dev/null +++ b/drivers/iio/adc/ltc2496.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ltc2496.c - Driver for Analog Devices/Linear Technology LTC2496 ADC + * + * Based on ltc2497.c which has + * Copyright (C) 2017 Analog Devices Inc. + * + * Licensed under the GPL-2. + * + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2496fc.pdf + */ + +#include <linux/spi/spi.h> +#include <linux/iio/iio.h> +#include <linux/iio/driver.h> +#include <linux/module.h> +#include <linux/of.h> + +#include "ltc2497.h" + +struct ltc2496_driverdata { + /* this must be the first member */ + struct ltc2497core_driverdata common_ddata; + struct spi_device *spi; + + /* + * DMA (thus cache coherency maintenance) requires the + * transfer buffers to live in their own cache lines. + */ + unsigned char rxbuf[3] ____cacheline_aligned; + unsigned char txbuf[3] ____cacheline_aligned; +}; + +static int ltc2496_result_and_measure(struct ltc2497core_driverdata *ddata, + u8 address, int *val) +{ + struct ltc2496_driverdata *st = + container_of(ddata, struct ltc2496_driverdata, common_ddata); + struct spi_transfer t = { + .tx_buf = st->txbuf, + .rx_buf = st->rxbuf, + .len = sizeof(st->txbuf), + }; + int ret; + + st->txbuf[0] = LTC2497_ENABLE | address; + + ret = spi_sync_transfer(st->spi, &t, 1); + if (ret < 0) { + dev_err(&st->spi->dev, "spi_sync_transfer failed: %pe\n", + ERR_PTR(ret)); + return ret; + } + + if (val) + *val = ((st->rxbuf[0] & 0x3f) << 12 | + st->rxbuf[1] << 4 | st->rxbuf[2] >> 4) - + (1 << 17); + + return 0; +} + +static int ltc2496_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct ltc2496_driverdata *st; + struct device *dev = &spi->dev; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + spi_set_drvdata(spi, indio_dev); + st->spi = spi; + st->common_ddata.result_and_measure = ltc2496_result_and_measure; + + return ltc2497core_probe(dev, indio_dev); +} + +static int ltc2496_remove(struct spi_device *spi) +{ + struct iio_dev *indio_dev = spi_get_drvdata(spi); + + ltc2497core_remove(indio_dev); + + return 0; +} + +static const struct of_device_id ltc2496_of_match[] = { + { .compatible = "lltc,ltc2496", }, + {}, +}; +MODULE_DEVICE_TABLE(of, ltc2496_of_match); + +static struct spi_driver ltc2496_driver = { + .driver = { + .name = "ltc2496", + .of_match_table = of_match_ptr(ltc2496_of_match), + }, + .probe = ltc2496_probe, + .remove = ltc2496_remove, +}; +module_spi_driver(ltc2496_driver); + +MODULE_AUTHOR("Uwe Kleine-König <u.kleine-könig@pengutronix.de>"); +MODULE_DESCRIPTION("Linear Technology LTC2496 ADC driver"); +MODULE_LICENSE("GPL v2"); -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 2019-11-21 21:00 ` [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König @ 2019-11-23 17:12 ` Jonathan Cameron 2019-11-24 19:11 ` Uwe Kleine-König 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2019-11-23 17:12 UTC (permalink / raw) To: Uwe Kleine-König Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel On Thu, 21 Nov 2019 22:00:07 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C > and so has a slightly different protocol. Only the actual hardware > access is different. The spi protocol is different enough to not be able > to map the differences via a regmap. > > Also generalize the entry in MAINTAINER to cover the newly introduced > file. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> looks good with the exception of the now overly protected DMA buffers. See inline. As that's all I'm seeing that needs fixing I'll just fix this up whilst applying. I'd like the series to sit a little longer on the list though to give devicetree maintainers time to look at the bindings. Thanks, Jonathan > --- > MAINTAINERS | 2 +- > drivers/iio/adc/Kconfig | 10 ++++ > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ltc2496.c | 108 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 120 insertions(+), 1 deletion(-) > create mode 100644 drivers/iio/adc/ltc2496.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index eb19fad370d7..8b1211038617 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1029,7 +1029,7 @@ S: Supported > F: Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9523 > F: Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4350 > F: drivers/iio/*/ad* > -F: drivers/iio/adc/ltc2497* > +F: drivers/iio/adc/ltc249* > X: drivers/iio/*/adjd* > F: drivers/staging/iio/*/ad* > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index f0af3a42f53c..deb86f6039b3 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -492,6 +492,16 @@ config LTC2485 > To compile this driver as a module, choose M here: the module will be > called ltc2485. > > +config LTC2496 > + tristate "Linear Technology LTC2496 ADC driver" > + depends on SPI > + help > + Say yes here to build support for Linear Technology LTC2496 > + 16-Bit 8-/16-Channel Delta Sigma ADC. > + > + To compile this driver as a module, choose M here: the module will be > + called ltc2496. > + > config LTC2497 > tristate "Linear Technology LTC2497 ADC driver" > depends on I2C > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index ee0c8dcfb501..c3dc2a12766a 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o > obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o > obj-$(CONFIG_LTC2471) += ltc2471.o > obj-$(CONFIG_LTC2485) += ltc2485.o > +obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o > obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o > obj-$(CONFIG_MAX1027) += max1027.o > obj-$(CONFIG_MAX11100) += max11100.o > diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c > new file mode 100644 > index 000000000000..a7659c8f9cc9 > --- /dev/null > +++ b/drivers/iio/adc/ltc2496.c > @@ -0,0 +1,108 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ltc2496.c - Driver for Analog Devices/Linear Technology LTC2496 ADC > + * > + * Based on ltc2497.c which has > + * Copyright (C) 2017 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2496fc.pdf > + */ > + > +#include <linux/spi/spi.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/driver.h> > +#include <linux/module.h> > +#include <linux/of.h> > + > +#include "ltc2497.h" > + > +struct ltc2496_driverdata { > + /* this must be the first member */ > + struct ltc2497core_driverdata common_ddata; > + struct spi_device *spi; > + > + /* > + * DMA (thus cache coherency maintenance) requires the > + * transfer buffers to live in their own cache lines. > + */ > + unsigned char rxbuf[3] ____cacheline_aligned; > + unsigned char txbuf[3] ____cacheline_aligned; Ah. I've not explained this clearly enough. Upshot is you only need to ensure that the buffers used for dma are not shared with any other usage. the __cacheline_aligned marker pads the structure to ensure the element so marked is at the start of a new cacheline. This means there is no sharing with non DMA related elements which may be accidentally reset when the DMA transfer ends. Imagine we had: struct bob { int a; //used for all sorts of fun things not related to dma and not //protected from happening concurrently with dma. unsigned char rx_buf[3]; unsigned char tx_buf[3] }; The buffers are used for DMA. The DMA engine takes a copy of the cacheline to start doing it's magic. Along comes other activity and writes to 'a'. DMA completes, then engine pushes the cacheline back to the memory including writing back what it had as a copy of a. Thus the update to 'a' is lost. Now the guarantee we make use of is that DMA engines are not allowed to copy cachelines that do not contain the buffers they are using (all sorts of things would break if they were). However, there is no need to separate rx_buf and tx_buf as they are being used by the same DMA engine and nothing else is going to update them whilst they are in use. A fun side note is that i2c never uses buffers provided to it for DMA transfers except when using the specific dmasafe functions (which isn't happening here). Thanks, Jonathan > +}; > + > +static int ltc2496_result_and_measure(struct ltc2497core_driverdata *ddata, > + u8 address, int *val) > +{ > + struct ltc2496_driverdata *st = > + container_of(ddata, struct ltc2496_driverdata, common_ddata); > + struct spi_transfer t = { > + .tx_buf = st->txbuf, > + .rx_buf = st->rxbuf, > + .len = sizeof(st->txbuf), > + }; > + int ret; > + > + st->txbuf[0] = LTC2497_ENABLE | address; > + > + ret = spi_sync_transfer(st->spi, &t, 1); > + if (ret < 0) { > + dev_err(&st->spi->dev, "spi_sync_transfer failed: %pe\n", > + ERR_PTR(ret)); > + return ret; > + } > + > + if (val) > + *val = ((st->rxbuf[0] & 0x3f) << 12 | > + st->rxbuf[1] << 4 | st->rxbuf[2] >> 4) - > + (1 << 17); > + > + return 0; > +} > + > +static int ltc2496_probe(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev; > + struct ltc2496_driverdata *st; > + struct device *dev = &spi->dev; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!indio_dev) > + return -ENOMEM; > + > + st = iio_priv(indio_dev); > + spi_set_drvdata(spi, indio_dev); > + st->spi = spi; > + st->common_ddata.result_and_measure = ltc2496_result_and_measure; > + > + return ltc2497core_probe(dev, indio_dev); > +} > + > +static int ltc2496_remove(struct spi_device *spi) > +{ > + struct iio_dev *indio_dev = spi_get_drvdata(spi); > + > + ltc2497core_remove(indio_dev); > + > + return 0; > +} > + > +static const struct of_device_id ltc2496_of_match[] = { > + { .compatible = "lltc,ltc2496", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, ltc2496_of_match); > + > +static struct spi_driver ltc2496_driver = { > + .driver = { > + .name = "ltc2496", > + .of_match_table = of_match_ptr(ltc2496_of_match), > + }, > + .probe = ltc2496_probe, > + .remove = ltc2496_remove, > +}; > +module_spi_driver(ltc2496_driver); > + > +MODULE_AUTHOR("Uwe Kleine-König <u.kleine-könig@pengutronix.de>"); > +MODULE_DESCRIPTION("Linear Technology LTC2496 ADC driver"); > +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 2019-11-23 17:12 ` Jonathan Cameron @ 2019-11-24 19:11 ` Uwe Kleine-König 2019-12-01 11:00 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Uwe Kleine-König @ 2019-11-24 19:11 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel On Sat, Nov 23, 2019 at 05:12:04PM +0000, Jonathan Cameron wrote: > On Thu, 21 Nov 2019 22:00:07 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C > > and so has a slightly different protocol. Only the actual hardware > > access is different. The spi protocol is different enough to not be able > > to map the differences via a regmap. > > > > Also generalize the entry in MAINTAINER to cover the newly introduced > > file. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > looks good with the exception of the now overly protected DMA buffers. > > See inline. As that's all I'm seeing that needs fixing I'll just > fix this up whilst applying. > > I'd like the series to sit a little longer on the list though to give > devicetree maintainers time to look at the bindings. ok. > > +struct ltc2496_driverdata { > > + /* this must be the first member */ > > + struct ltc2497core_driverdata common_ddata; > > + struct spi_device *spi; > > + > > + /* > > + * DMA (thus cache coherency maintenance) requires the > > + * transfer buffers to live in their own cache lines. > > + */ > > + unsigned char rxbuf[3] ____cacheline_aligned; > > + unsigned char txbuf[3] ____cacheline_aligned; > Ah. I've not explained this clearly enough. Upshot is you only need > to ensure that the buffers used for dma are not shared with any other > usage. the __cacheline_aligned marker pads the structure to ensure > the element so marked is at the start of a new cacheline. This means > there is no sharing with non DMA related elements which may be accidentally > reset when the DMA transfer ends. > > Imagine we had: > struct bob { > int a; //used for all sorts of fun things not related to dma and not > //protected from happening concurrently with dma. > unsigned char rx_buf[3]; > unsigned char tx_buf[3] > }; > > The buffers are used for DMA. The DMA engine takes a copy of the cacheline > to start doing it's magic. > > Along comes other activity and writes to 'a'. > > DMA completes, then engine pushes the cacheline back to the memory including > writing back what it had as a copy of a. Thus the update to 'a' is lost. > > Now the guarantee we make use of is that DMA engines are not allowed to > copy cachelines that do not contain the buffers they are using (all sorts > of things would break if they were). > > However, there is no need to separate rx_buf and tx_buf as they are being > used by the same DMA engine and nothing else is going to update them whilst > they are in use. Yeah, I thought about that when adding the second annotation but then forgot to mention that in my cover letter. So I assume you will just drop the 2nd ____cacheline_aligned? That's fine for me; thanks. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 2019-11-24 19:11 ` Uwe Kleine-König @ 2019-12-01 11:00 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2019-12-01 11:00 UTC (permalink / raw) To: Uwe Kleine-König Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa, Alexandru Ardelean, linux-iio, kernel On Sun, 24 Nov 2019 20:11:01 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Sat, Nov 23, 2019 at 05:12:04PM +0000, Jonathan Cameron wrote: > > On Thu, 21 Nov 2019 22:00:07 +0100 > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C > > > and so has a slightly different protocol. Only the actual hardware > > > access is different. The spi protocol is different enough to not be able > > > to map the differences via a regmap. > > > > > > Also generalize the entry in MAINTAINER to cover the newly introduced > > > file. > > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > looks good with the exception of the now overly protected DMA buffers. > > > > See inline. As that's all I'm seeing that needs fixing I'll just > > fix this up whilst applying. > > > > I'd like the series to sit a little longer on the list though to give > > devicetree maintainers time to look at the bindings. > > ok. > > > > +struct ltc2496_driverdata { > > > + /* this must be the first member */ > > > + struct ltc2497core_driverdata common_ddata; > > > + struct spi_device *spi; > > > + > > > + /* > > > + * DMA (thus cache coherency maintenance) requires the > > > + * transfer buffers to live in their own cache lines. > > > + */ > > > + unsigned char rxbuf[3] ____cacheline_aligned; > > > + unsigned char txbuf[3] ____cacheline_aligned; > > Ah. I've not explained this clearly enough. Upshot is you only need > > to ensure that the buffers used for dma are not shared with any other > > usage. the __cacheline_aligned marker pads the structure to ensure > > the element so marked is at the start of a new cacheline. This means > > there is no sharing with non DMA related elements which may be accidentally > > reset when the DMA transfer ends. > > > > Imagine we had: > > struct bob { > > int a; //used for all sorts of fun things not related to dma and not > > //protected from happening concurrently with dma. > > unsigned char rx_buf[3]; > > unsigned char tx_buf[3] > > }; > > > > The buffers are used for DMA. The DMA engine takes a copy of the cacheline > > to start doing it's magic. > > > > Along comes other activity and writes to 'a'. > > > > DMA completes, then engine pushes the cacheline back to the memory including > > writing back what it had as a copy of a. Thus the update to 'a' is lost. > > > > Now the guarantee we make use of is that DMA engines are not allowed to > > copy cachelines that do not contain the buffers they are using (all sorts > > of things would break if they were). > > > > However, there is no need to separate rx_buf and tx_buf as they are being > > used by the same DMA engine and nothing else is going to update them whilst > > they are in use. > > Yeah, I thought about that when adding the second annotation but then > forgot to mention that in my cover letter. > > So I assume you will just drop the 2nd ____cacheline_aligned? That's > fine for me; thanks. Yup. Jonathan > > Best regards > Uwe > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-09 20:34 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-21 21:00 [PATCH v3 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König 2019-12-01 11:28 ` Jonathan Cameron 2019-12-09 15:18 ` Uwe Kleine-König 2019-12-09 20:34 ` Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König 2019-11-21 21:00 ` [PATCH v3 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König 2019-11-23 17:12 ` Jonathan Cameron 2019-11-24 19:11 ` Uwe Kleine-König 2019-12-01 11:00 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).