* Re: [PATCH 2/2] iio:imu:mpu6050 add explicit mpu9250 support
From: Jonathan Cameron @ 2017-04-02 9:12 UTC (permalink / raw)
To: Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mark.Rutland-5wv7dgnIgG8
In-Reply-To: <20170330231846.j45psrzast3pjc5b@rob-hp-laptop>
On 31/03/17 00:18, Rob Herring wrote:
> On Sun, Mar 26, 2017 at 12:11:00PM +0100, Jonathan Cameron wrote:
>> The mpu9250 is a SIP containing an mpu6500 and an ak8975. If this was all
>> there was too it there would be no need for explicit handling in the driver.
>> Arguably the bindings would also only reflect the presence of an mpu6500 with
>> the ak8975 hanging off it, as the kernel doesn't care that they are in one
>> package.
>>
>> However, the WHOAMI value changes as well so best to add explicit support.
>>
>> Signed-off-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 2 +-
>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 ++++++
>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 5 +++++
>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 ++
>> drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 1 +
>> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> Same one per line comment, but
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
Fixed up and applied. Thanks Rob.
Jonathan
^ permalink raw reply
* Re: [PATCH v2] iio:adc: Driver for Linear Technology LTC2497 ADC
From: Jonathan Cameron @ 2017-04-02 9:30 UTC (permalink / raw)
To: michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
lars-Qo5EllUWu/uELgA04lAiVw, knaack.h-Mmb7MZpHnFY,
pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490599433-6103-1-git-send-email-michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
On 27/03/17 08:23, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org wrote:
> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>
> This patch adds support for the Analog Devices / Linear Technology
> LTC2497 ADCs. The LTC2497 is a 16-channel (eight differential),
> 16-bit, high precision, delta-sigma ADC with an automatic, differential,
> input current cancellation front end and a 2-wire, I2C interface.
>
> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Looks good to me.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Bindings are trivial so happy to take without pestering Rob or Mark.
(of course their comments are always welcome!)
Thanks,
Jonathan
>
> ---
>
> Changes since v1:
>
> - remove confusing kconfig help text
> - use proper defines
> - add descriptive comment
> - fix removal order
> - add poper commit mssage
> ---
> .../devicetree/bindings/iio/adc/ltc2497.txt | 13 +
> MAINTAINERS | 1 +
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ltc2497.c | 269 +++++++++++++++++++++
> 5 files changed, 294 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> create mode 100644 drivers/iio/adc/ltc2497.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> new file mode 100644
> index 0000000..c2829c19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> @@ -0,0 +1,13 @@
> +* Linear Technology / Analog Devices LTC2497 ADC
> +
> +Required properties:
> + - compatible: Should be "lltc,ltc2497"
> + - reg: Should contain the ADC I2C address
> + - vref-supply: The regulator supply for ADC reference voltage
> +
> +Example:
> + ltc2497: adc@76 {
> + compatible = "lltc,ltc2497";
> + reg = <0x76>;
> + vref-supply = <<c2497_reg>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7d6f9a..173043c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -813,6 +813,7 @@ W: http://wiki.analog.com/
> W: http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/iio/*/ad*
> +F: drivers/iio/adc/ltc2497*
> X: drivers/iio/*/adjd*
> F: drivers/staging/iio/*/ad*
> F: drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2268a6f..acc115b 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -326,6 +326,16 @@ config LTC2485
> To compile this driver as a module, choose M here: the module will be
> called ltc2485.
>
> +config LTC2497
> + tristate "Linear Technology LTC2497 ADC driver"
> + depends on I2C
> + help
> + Say yes here to build support for Linear Technology LTC2497
> + 16-Bit 8-/16-Channel Delta Sigma ADC.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ltc2497.
> +
> config MAX1027
> tristate "Maxim max1027 ADC driver"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 73dbe39..9d626b5 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
> obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
> obj-$(CONFIG_LTC2485) += ltc2485.o
> +obj-$(CONFIG_LTC2497) += ltc2497.o
> obj-$(CONFIG_MAX1027) += max1027.o
> obj-$(CONFIG_MAX11100) += max11100.o
> obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> new file mode 100644
> index 0000000..7907534
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -0,0 +1,269 @@
> +/*
> + * ltc2497.c - Driver for Linear Technology LTC2497 ADC
> + *
> + * Copyright (C) 2017 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define LTC2497_ENABLE 0xA0
> +#define LTC2497_SGL (1 << 4)
> +#define LTC2497_DIFF (0 << 4)
> +#define LTC2497_SIGN (1 << 3)
> +#define LTC2497_CONFIG_DEFAULT LTC2497_ENABLE
> +#define LTC2497_CONVERSION_TIME_MS 150ULL
> +
> +struct ltc2497_st {
> + struct i2c_client *client;
> + struct regulator *ref;
> + ktime_t time_prev;
> + u8 addr_prev;
> +};
> +
> +static int ltc2497_wait_conv(struct ltc2497_st *st)
> +{
> + 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
> + */
> + msleep(LTC2497_CONVERSION_TIME_MS - time_elapsed);
> + 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 -ETIMEDOUT;
> +}
> +
> +static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
> +{
> + struct i2c_client *client = st->client;
> + __be32 buf = 0;
> + int ret;
> +
> + ret = ltc2497_wait_conv(st);
> + if (ret < 0 || st->addr_prev != address) {
> + ret = i2c_smbus_write_byte(st->client,
> + LTC2497_ENABLE | address);
> + if (ret < 0)
> + return ret;
> + st->addr_prev = address;
> + msleep(LTC2497_CONVERSION_TIME_MS);
> + }
> + ret = i2c_master_recv(client, (char *)&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(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);
> + 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)
> + return ret;
> +
> + *val = ret / 1000;
> + *val2 = 17;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +#define LTC2497_CHAN(_chan, _addr) { \
> + .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), \
> +}
> +
> +#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 ltc2497_channel[] = {
> + LTC2497_CHAN(0, LTC2497_SGL),
> + LTC2497_CHAN(1, LTC2497_SGL),
> + LTC2497_CHAN(2, LTC2497_SGL),
> + LTC2497_CHAN(3, LTC2497_SGL),
> + LTC2497_CHAN(4, LTC2497_SGL),
> + LTC2497_CHAN(5, LTC2497_SGL),
> + LTC2497_CHAN(6, LTC2497_SGL),
> + LTC2497_CHAN(7, LTC2497_SGL),
> + LTC2497_CHAN(8, LTC2497_SGL),
> + LTC2497_CHAN(9, LTC2497_SGL),
> + LTC2497_CHAN(10, LTC2497_SGL),
> + LTC2497_CHAN(11, LTC2497_SGL),
> + LTC2497_CHAN(12, LTC2497_SGL),
> + LTC2497_CHAN(13, LTC2497_SGL),
> + LTC2497_CHAN(14, LTC2497_SGL),
> + LTC2497_CHAN(15, LTC2497_SGL),
> + 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,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static int ltc2497_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct iio_dev *indio_dev;
> + struct ltc2497_st *st;
> + int ret;
> +
> + 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));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + st->client = client;
> +
> + 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;
> +
> + ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT);
> + if (ret < 0)
> + goto err_regulator_disable;
> +
> + st->addr_prev = LTC2497_CONFIG_DEFAULT;
> + st->time_prev = ktime_get();
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0)
> + goto err_regulator_disable;
> +
> + return 0;
> +
> +err_regulator_disable:
> + regulator_disable(st->ref);
> +
> + return ret;
> +}
> +
> +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_device_unregister(indio_dev);
> + regulator_disable(st->ref);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ltc2497_id[] = {
> + { "ltc2497", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2497_id);
> +
> +static const struct of_device_id ltc2497_of_match[] = {
> + { .compatible = "lltc,ltc2497", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ltc2497_of_match);
> +
> +static struct i2c_driver ltc2497_driver = {
> + .driver = {
> + .name = "ltc2497",
> + .of_match_table = of_match_ptr(ltc2497_of_match),
> + },
> + .probe = ltc2497_probe,
> + .remove = ltc2497_remove,
> + .id_table = ltc2497_id,
> +};
> +module_i2c_driver(ltc2497_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Linear Technology LTC2497 ADC driver");
> +MODULE_LICENSE("GPL v2");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
From: Jonathan Cameron @ 2017-04-02 9:35 UTC (permalink / raw)
To: Jacopo Mondi, geert, wsa+renesas, magnus.damm, laurent.pinchart,
knaack.h, lars, pmeerw, robh+dt, mark.rutland
Cc: linux-iio, linux-renesas-soc, devicetree
In-Reply-To: <1490627578-15938-2-git-send-email-jacopo+renesas@jmondi.org>
On 27/03/17 16:12, Jacopo Mondi wrote:
> Add device tree bindings documentation for Maxim max9611/max9612 current
> sense amplifier.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> .../devicetree/bindings/iio/adc/max9611.txt | 26 ++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/max9611.txt b/Documentation/devicetree/bindings/iio/adc/max9611.txt
> new file mode 100644
> index 0000000..8430d3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt
> @@ -0,0 +1,26 @@
> +* Maxim max9611/max9612 current sense amplifier with 12-bits ADC interface
> +
> +Maxim max9611/max9612 is an high-side current sense amplifier with integrated
> +12-bits ADC communicating over I2c bus.
> +The device node for this driver shall be a child of a I2c controller.
> +
> +Required properties
> + - compatible: Should be "maxim,max9611" or "maxim,max9612"
> + - reg: The 7-bits long I2c address of the device
> + - shunt-resistor-uohm: Value, in uOhm, of the current sense shunt resistor
Just did a quick grep for this see if I felt comfortable enough to
not wait for Rob / Mark to have a chance to look at it.
Grep gave me:
shunt-resistor-micro-ohms for the ina2xx.
Feels rather like we should match that.
Otherwise looks good to me.
Jonathan
> +
> +Example:
> +
> +&i2c4 {
> + csa: adc@7c {
> + compatible = "maxim,max9611";
> + reg = <0x7c>;
> +
> + shunt-resistor-uohm = <5000>;
> + };
> +};
> +
> +This device node describes a current sense amplifier sitting on I2c4 bus
> +with address 0x7c (read address is 0xf9, write address is 0xf8).
> +A sense resistor of 0,005 Ohm is installed between RS+ and RS- current-sensing
> +inputs.
>
^ permalink raw reply
* Re: [PATCH v4 2/4] iio: Documentation: Add max9611 sysfs documentation
From: Jonathan Cameron @ 2017-04-02 9:36 UTC (permalink / raw)
To: Jacopo Mondi, geert, wsa+renesas, magnus.damm, laurent.pinchart,
knaack.h, lars, pmeerw, robh+dt, mark.rutland
Cc: linux-iio, linux-renesas-soc, devicetree
In-Reply-To: <1490627578-15938-3-git-send-email-jacopo+renesas@jmondi.org>
On 27/03/17 16:12, Jacopo Mondi wrote:
> Add documentation for max9611 driver.
> Document attributes describing value of shunt resistor installed between
> RS+ and RS- voltage sense inputs.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Good. Will pick up once the binding is sorted.
Kick me if I don't!
> ---
> Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
> new file mode 100644
> index 0000000..5020d00
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
> @@ -0,0 +1,17 @@
> +What: /sys/bus/iio/devices/iio:deviceX/in_power_shunt_resistor
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: linux-iio@vger.kernel.org
> +Description: The value of the shunt resistor used to compute power drain on
> + common input voltage pin (RS+). In micro Ohms.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_current_shunt_resistor
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: linux-iio@vger.kernel.org
> +Description: The value of the shunt resistor used to compute current flowing
> + between RS+ and RS- voltage sense inputs. In micro Ohms.
> +
> +These attributes describe a single physical component, exposed as two distinct
> +attributes as it is used to calculate two different values: power load and
> +current flowing between RS+ and RS- inputs.
>
^ permalink raw reply
* Re: [PATCH v4 3/4] iio: adc: Add Maxim max9611 ADC driver
From: Jonathan Cameron @ 2017-04-02 9:43 UTC (permalink / raw)
To: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490627578-15938-4-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 27/03/17 16:12, Jacopo Mondi wrote:
> Add iio driver for Maxim max9611 and max9612 current-sense amplifiers
> with 12-bits ADC interface.
>
> Datasheet publicly available at:
> https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
>
couple of bits inline. I'd failed to pick up on the shunt resistor units
before. To be consistent with the rest of the ABI, those exposed via
sysfs ought to be in ohms. It's also in ohms in the ina2xx driver.
Thanks,
Jonathan
> ---
>
> Output of iio_info on Salvator-X board for max9611 chip installed on
> VDD_0.8 lines.
> The VDD_0.8 line powers the CPU cluster and on-board RAM.
>
> iio:device0: max9611
> 5 channels found:
> voltage0: (input)
> 1 channel-specific attributes found:
> attr 0: input value: 4.085000000
> voltage1: (input)
> 3 channel-specific attributes found:
> attr 0: scale value: 14
> attr 1: offset value: 1
> attr 2: raw value: 59
> power: (input)
> 2 channel-specific attributes found:
> attr 0: shunt_resistor value: 5000
> attr 1: input value: 663.404000000
> current: (input)
> 2 channel-specific attributes found:
> attr 0: shunt_resistor value: 5000
> attr 1: input value: 817.000000000
> temp: (input)
> 2 channel-specific attributes found:
> attr 0: scale value: 480.076812289
> attr 1: raw value: 57
>
> The collected information represent:
>
> * voltage0 (current sense voltage) Vcsa
> voltage drop between RS+ and RS- input = 4,085 mV
> * voltage1: (common input voltage) Vcim
> voltage at RS+ input = (59 - 1) * 14 = 812 mV
> * current flowing on shunt resistor (Icsa)
> = Vcsa / Rshunt = 817 mA
> * power load on the sensed line (Pload)
> = Vcim * Icsa = 663 m
> * die temperature = (57 * 480.07) = 27360 milli Celsius
> ---
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max9611.c | 583 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 594 insertions(+)
> create mode 100644 drivers/iio/adc/max9611.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..82f2e7b8 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -354,6 +354,16 @@ config MAX1363
> To compile this driver as a module, choose M here: the module will be
> called max1363.
>
> +config MAX9611
> + tristate "Maxim max9611/max9612 ADC driver"
> + depends on I2C
> + help
> + Say yes here to build support for Maxim max9611/max9612 current sense
> + amplifier with 12-bits ADC interface.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called max9611.
> +
> config MCP320X
> tristate "Microchip Technology MCP3x01/02/04/08"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..149f979 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o
> obj-$(CONFIG_MAX1027) += max1027.o
> obj-$(CONFIG_MAX11100) += max11100.o
> obj-$(CONFIG_MAX1363) += max1363.o
> +obj-$(CONFIG_MAX9611) += max9611.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
> diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
> new file mode 100644
> index 0000000..e5aaf73
> --- /dev/null
> +++ b/drivers/iio/adc/max9611.c
> @@ -0,0 +1,583 @@
> +/*
> + * iio/adc/max9611.c
> + *
> + * Maxim max9611/max9612 high side current sense amplifier with
> + * 12-bit ADC interface.
> + *
> + * Copyright (C) 2017 Jacopo Mondi
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/*
> + * This driver supports input common-mode voltage, current-sense
> + * amplifier with programmable gains and die temperature reading from
> + * Maxim max9611/max9612.
> + *
> + * Op-amp, analog comparator, and watchdog functionalities are not
> + * supported by this driver.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +#define DRIVER_NAME "max9611"
> +#define MAX9611_DEV_NAME "max9611"
> +#define MAX9612_DEV_NAME "max9612"
Really trivial, but what do these defines give us other than
making the code slightly harder to review?
Just put them inline where they are used.
> +
> +/* max9611 register addresses */
> +#define MAX9611_REG_CSA_DATA 0x00
> +#define MAX9611_REG_RS_DATA 0x02
> +#define MAX9611_REG_TEMP_DATA 0x08
> +#define MAX9611_REG_CTRL1 0x0a
> +#define MAX9611_REG_CTRL2 0x0b
> +
> +/* max9611 REG1 mux configuration options */
> +#define MAX9611_MUX_MASK GENMASK(3,0)
> +#define MAX9611_MUX_SENSE_1x 0x00
> +#define MAX9611_MUX_SENSE_4x 0x01
> +#define MAX9611_MUX_SENSE_8x 0x02
> +#define MAX9611_INPUT_VOLT 0x03
> +#define MAX9611_MUX_TEMP 0x06
> +
> +/* max9611 voltage (both csa and input) helper macros */
> +#define MAX9611_VOLTAGE_SHIFT 0x04
> +#define MAX9611_VOLTAGE_RAW(_r) ((_r) >> MAX9611_VOLTAGE_SHIFT)
> +
> +/*
> + * max9611 current sense amplifier voltage output:
> + * LSB and offset values depends on selected gain (1x, 4x, 8x)
> + *
> + * GAIN LSB (nV) OFFSET (LSB steps)
> + * 1x 107500 1
> + * 4x 26880 1
> + * 8x 13440 3
> + *
> + * The complete formula to calculate current sense voltage is:
> + * (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3)
> + */
> +#define MAX9611_CSA_1X_LSB_nV 107500
> +#define MAX9611_CSA_4X_LSB_nV 26880
> +#define MAX9611_CSA_8X_LSB_nV 13440
> +
> +#define MAX9611_CSA_1X_OFFS_RAW 1
> +#define MAX9611_CSA_4X_OFFS_RAW 1
> +#define MAX9611_CSA_8X_OFFS_RAW 3
> +
> +/*
> + * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C
> + *
> + * The complete formula to calculate input common voltage is:
> + * (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000)
> + */
> +#define MAX9611_CIM_LSB_mV 14
> +#define MAX9611_CIM_OFFSET_RAW 1
> +
> +/*
> + * max9611 temperature reading: LSB is 480 milli degrees Celsius
> + *
> + * The complete formula to calculate temperature is:
> + * ((adc_read >> 7) * 1000) / (1 / 480 * 1000)
> + */
> +#define MAX9611_TEMP_MAX_POS 0x7f80
> +#define MAX9611_TEMP_MAX_NEG 0xff80
> +#define MAX9611_TEMP_MIN_NEG 0xd980
> +#define MAX9611_TEMP_MASK GENMASK(7, 15)
> +#define MAX9611_TEMP_SHIFT 0x07
> +#define MAX9611_TEMP_RAW(_r) ((_r) >> MAX9611_TEMP_SHIFT)
> +#define MAX9611_TEMP_SCALE_NUM 1000000
> +#define MAX9611_TEMP_SCALE_DIV 2083
> +
> +struct max9611_dev {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct mutex lock;
> + unsigned int shunt_resistor_uohm;
> +};
> +
> +enum max9611_conf_ids {
> + CONF_SENSE_1x,
> + CONF_SENSE_4x,
> + CONF_SENSE_8x,
> + CONF_IN_VOLT,
> + CONF_TEMP,
> +};
> +
> +/**
> + * max9611_mux_conf - associate ADC mux configuration with register address
> + * where data shall be read from
> + */
> +static const unsigned int max9611_mux_conf[][2] = {
> + /* CONF_SENSE_1x */
> + { MAX9611_MUX_SENSE_1x, MAX9611_REG_CSA_DATA },
> + /* CONF_SENSE_4x */
> + { MAX9611_MUX_SENSE_4x, MAX9611_REG_CSA_DATA },
> + /* CONF_SENSE_8x */
> + { MAX9611_MUX_SENSE_8x, MAX9611_REG_CSA_DATA },
> + /* CONF_IN_VOLT */
> + { MAX9611_INPUT_VOLT, MAX9611_REG_RS_DATA },
> + /* CONF_TEMP */
> + { MAX9611_MUX_TEMP, MAX9611_REG_TEMP_DATA },
> +};
> +
> +enum max9611_csa_gain {
> + CSA_GAIN_1x,
> + CSA_GAIN_4x,
> + CSA_GAIN_8x,
> +};
> +
> +enum max9611_csa_gain_params {
> + CSA_GAIN_LSB_nV,
> + CSA_GAIN_OFFS_RAW,
> +};
> +
> +/**
> + * max9611_csa_gain_conf - associate gain multiplier with LSB and
> + * offset values.
> + *
> + * Group together parameters associated with configurable gain
> + * on current sense amplifier path to ADC interface.
> + * Current sense read routine adjusts gain until it gets a meaningful
> + * value; use this structure to retrieve the correct LSB and offset values.
> + */
> +static const unsigned int max9611_gain_conf[][2] = {
> + { /* [0] CSA_GAIN_1x */
> + MAX9611_CSA_1X_LSB_nV,
> + MAX9611_CSA_1X_OFFS_RAW,
> + },
> + { /* [1] CSA_GAIN_4x */
> + MAX9611_CSA_4X_LSB_nV,
> + MAX9611_CSA_4X_OFFS_RAW,
> + },
> + { /* [2] CSA_GAIN_8x */
> + MAX9611_CSA_8X_LSB_nV,
> + MAX9611_CSA_8X_OFFS_RAW,
> + },
> +};
> +
> +enum max9611_chan_addrs {
> + MAX9611_CHAN_VOLTAGE_INPUT,
> + MAX9611_CHAN_VOLTAGE_SENSE,
> + MAX9611_CHAN_TEMPERATURE,
> + MAX9611_CHAN_CURRENT_LOAD,
> + MAX9611_CHAN_POWER_LOAD,
> +};
> +
> +static const struct iio_chan_spec max9611_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .address = MAX9611_CHAN_TEMPERATURE,
> + },
> + {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_VOLTAGE_SENSE,
> + .indexed = 1,
> + .channel = 0,
> + },
> + {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .address = MAX9611_CHAN_VOLTAGE_INPUT,
> + .indexed = 1,
> + .channel = 1,
> + },
> + {
> + .type = IIO_CURRENT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_CURRENT_LOAD,
> + },
> + {
> + .type = IIO_POWER,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_POWER_LOAD
> + },
> +};
> +
> +/**
> + * max9611_read_single() - read a single value from ADC interface
> + *
> + * Data registers are 16 bit long, spread between two 8 bit registers
> + * with consecutive addresses.
> + * Configure ADC mux first, then read register at address "reg_addr".
> + * The smbus_read_word routine asks for 16 bits and the ADC is kind enough
> + * to return values from "reg_addr" and "reg_addr + 1" consecutively.
> + * Data are transmitted with big-endian ordering: MSB arrives first.
> + *
> + * @max9611: max9611 device
> + * @selector: index for mux and register configuration
> + * @raw_val: the value returned from ADC
> + */
> +static int max9611_read_single(struct max9611_dev *max9611,
> + enum max9611_conf_ids selector,
> + u16 *raw_val)
> +{
> + int ret;
> +
> + u8 mux_conf = max9611_mux_conf[selector][0] & MAX9611_MUX_MASK;
> + u8 reg_addr = max9611_mux_conf[selector][1];
> +
> + /*
> + * Keep mutex lock held during read-write to avoid mux register
> + * (CTRL1) re-configuration.
> + */
> + mutex_lock(&max9611->lock);
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL1, mux_conf);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL1, mux_conf);
> + mutex_unlock(&max9611->lock);
> + return ret;
> + }
> +
> + /*
> + * need a delay here to make register configuration
> + * stabilize. 1 msec at least, from empirical testing.
> + */
> + usleep_range(1000, 2000);
> +
> + ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
> + if (ret < 0) {
> + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
> + reg_addr);
> + mutex_unlock(&max9611->lock);
> + return ret;
> + }
> +
> + *raw_val = ret;
> + mutex_unlock(&max9611->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * max9611_read_csa_voltage() - read current sense amplifier output voltage
> + *
> + * Current sense amplifier output voltage is read through a configurable
> + * 1x, 4x or 8x gain.
> + * Start with plain 1x gain, and adjust gain control properly until a
> + * meaningful value is read from ADC output.
> + *
> + * @max9611: max9611 device
> + * @adc_raw: raw value read from ADC output
> + * @csa_gain: gain configuration option selector
> + */
> +static int max9611_read_csa_voltage(struct max9611_dev *max9611,
> + u16 *adc_raw,
> + enum max9611_csa_gain *csa_gain)
> +{
> + enum max9611_conf_ids gain_selectors[] = {
> + CONF_SENSE_1x,
> + CONF_SENSE_4x,
> + CONF_SENSE_8x
> + };
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(gain_selectors); ++i) {
> + ret = max9611_read_single(max9611, gain_selectors[i], adc_raw);
> + if (ret)
> + return ret;
> +
> + if (*adc_raw > 0) {
> + *csa_gain = gain_selectors[i];
> + return 0;
> + }
> + }
> +
> + return -EIO;
> +}
> +
> +static int max9611_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max9611_dev *dev = iio_priv(indio_dev);
> + enum max9611_csa_gain gain_selector;
> + const unsigned int *csa_gain;
> + u16 adc_data;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> +
> + switch (chan->address) {
> + case MAX9611_CHAN_TEMPERATURE:
> + ret = max9611_read_single(dev, CONF_TEMP,
> + &adc_data);
> + if (ret)
> + return -EINVAL;
> +
> + *val = MAX9611_TEMP_RAW(adc_data);
> + return IIO_VAL_INT;
> +
> + case MAX9611_CHAN_VOLTAGE_INPUT:
> + ret = max9611_read_single(dev, CONF_IN_VOLT,
> + &adc_data);
> + if (ret)
> + return -EINVAL;
> +
> + *val = MAX9611_VOLTAGE_RAW(adc_data);
> + return IIO_VAL_INT;
> + }
> +
> + break;
> +
> + case IIO_CHAN_INFO_OFFSET:
> + /* MAX9611_CHAN_VOLTAGE_INPUT */
> + *val = MAX9611_CIM_OFFSET_RAW;
> +
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SCALE:
> +
> + switch (chan->address) {
> + case MAX9611_CHAN_TEMPERATURE:
> + *val = MAX9611_TEMP_SCALE_NUM;
> + *val2 = MAX9611_TEMP_SCALE_DIV;
> +
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_VOLTAGE_INPUT:
> + *val = MAX9611_CIM_LSB_mV;
> +
> + return IIO_VAL_INT;
> + }
> +
> + break;
> +
> + case IIO_CHAN_INFO_PROCESSED:
> +
> + switch (chan->address) {
> + case MAX9611_CHAN_VOLTAGE_SENSE:
> + /*
> + * processed (mV): (raw - offset) * LSB (nV) / 10^6
> + *
> + * Even if max9611 can output raw csa voltage readings,
> + * use a produced value as scale depends on gain.
> + */
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + return -EINVAL;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV];
> + *val2 = 1000000;
> +
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_CURRENT_LOAD:
> + /* processed (mA): Vcsa (nV) / Rshunt (uOhm) */
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + return -EINVAL;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV];
> + *val2 = dev->shunt_resistor_uohm;
> +
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_POWER_LOAD:
> + /*
> + * processed (mW): Vin (mV) * Vcsa (uV) /
> + * Rshunt (uOhm)
> + */
> + ret = max9611_read_single(dev, CONF_IN_VOLT,
> + &adc_data);
> + if (ret)
> + return -EINVAL;
> +
> + adc_data -= MAX9611_CIM_OFFSET_RAW;
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + MAX9611_CIM_LSB_mV;
> +
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + return -EINVAL;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + /* divide by 10^3 here to avoid 32bit overflow */
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val *= MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV] / 1000;
> + *val2 = dev->shunt_resistor_uohm;
> +
> + return IIO_VAL_FRACTIONAL;
> + }
> +
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static ssize_t max9611_shunt_resistor_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct max9611_dev *max9611 = iio_priv(dev_to_iio_dev(dev));
> +
> + return sprintf(buf, "%u\n", max9611->shunt_resistor_uohm);
Oops didn't pick up on this before. Should really be ohms with
appropriate fixed point arithmetic.
This would match with the use of ohms for DPOT devices that we already
have documented.
> +}
> +
> +static IIO_DEVICE_ATTR(in_power_shunt_resistor, 0444,
> + max9611_shunt_resistor_show, NULL, 0);
> +static IIO_DEVICE_ATTR(in_current_shunt_resistor, 0444,
> + max9611_shunt_resistor_show, NULL, 0);
> +
> +static struct attribute *max9611_attributes[] = {
> + &iio_dev_attr_in_power_shunt_resistor.dev_attr.attr,
> + &iio_dev_attr_in_current_shunt_resistor.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group max9611_attribute_group = {
> + .attrs = max9611_attributes,
> +};
> +
> +static const struct iio_info indio_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = max9611_read_raw,
> + .attrs = &max9611_attribute_group,
> +};
> +
> +static int max9611_init(struct max9611_dev *max9611)
> +{
> + struct i2c_client *client = max9611->i2c_client;
> + u16 regval;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BYTE |
> + I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> + dev_err(max9611->dev,
> + "I2c adapter does not support smbus write_byte or read_word functionalities: aborting probe.\n");
> + return -EINVAL;
> + }
> +
> + /* Make sure die temperature is in range to test communications. */
> + ret = max9611_read_single(max9611, CONF_TEMP, ®val);
> + if (ret)
> + return ret;
> +
> + regval = ret & MAX9611_TEMP_MASK;
> +
> + if ((regval > MAX9611_TEMP_MAX_POS &&
> + regval < MAX9611_TEMP_MIN_NEG) ||
> + regval > MAX9611_TEMP_MAX_NEG) {
> + dev_err(max9611->dev,
> + "Invalid value received from ADC 0x%4x: aborting\n",
> + regval);
> + return -EIO;
> + }
> +
> + /* Mux shall be zeroed back before applying other configurations */
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL1, 0);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL1, 0);
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL2, 0);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL2, 0);
> + return ret;
> + }
> + usleep_range(1000, 2000);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id max9611_of_table[] = {
> + {.compatible = "maxim,max9611", .data = MAX9611_DEV_NAME},
> + {.compatible = "maxim,max9612", .data = MAX9612_DEV_NAME},
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, max9611_of_table);
> +static int max9611_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + const char * const shunt_res_prop = "shunt-resistor-uohm";
> + const struct device_node *of_node = client->dev.of_node;
> + const struct of_device_id *of_id =
> + of_match_device(max9611_of_table, &client->dev);
> + struct max9611_dev *max9611;
> + struct iio_dev *indio_dev;
> + unsigned int of_shunt;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*max9611));
> + if (IS_ERR(indio_dev))
> + return PTR_ERR(indio_dev);
> +
> + i2c_set_clientdata(client, indio_dev);
> +
> + max9611 = iio_priv(indio_dev);
> + max9611->dev = &client->dev;
> + max9611->i2c_client = client;
> + mutex_init(&max9611->lock);
> +
> + ret = of_property_read_u32(of_node, shunt_res_prop, &of_shunt);
> + if (ret) {
> + dev_err(&client->dev,
> + "Missing %s property for %s node\n",
> + shunt_res_prop, of_node->full_name);
> + return ret;
> + }
> + max9611->shunt_resistor_uohm = of_shunt;
> +
> + ret = max9611_init(max9611);
> + if (ret)
> + return ret;
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->dev.of_node = client->dev.of_node;
> + indio_dev->name = of_id->data;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &indio_info;
> + indio_dev->channels = max9611_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max9611_channels);
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static struct i2c_driver max9611_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = max9611_of_table,
> + },
> + .probe = max9611_probe,
> +};
> +module_i2c_driver(max9611_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
> +MODULE_LICENSE("GPL v2");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] iio:adc: Driver for Linear Technology LTC2497 ADC
From: Lars-Peter Clausen @ 2017-04-02 9:52 UTC (permalink / raw)
To: Jonathan Cameron, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3d54516e-cbd1-8a61-0d83-a91d7055f44c-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 04/02/2017 11:30 AM, Jonathan Cameron wrote:
> On 27/03/17 08:23, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org wrote:
>> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>>
>> This patch adds support for the Analog Devices / Linear Technology
>> LTC2497 ADCs. The LTC2497 is a 16-channel (eight differential),
>> 16-bit, high precision, delta-sigma ADC with an automatic, differential,
>> input current cancellation front end and a 2-wire, I2C interface.
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
> Looks good to me.
>
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.
There is a v3 of this patch.
^ permalink raw reply
* Re: [PATCH v4 2/4] iio: Documentation: Add max9611 sysfs documentation
From: Jonathan Cameron @ 2017-04-02 9:53 UTC (permalink / raw)
To: Jacopo Mondi, geert, wsa+renesas, magnus.damm, laurent.pinchart,
knaack.h, lars, pmeerw, robh+dt, mark.rutland
Cc: linux-iio, linux-renesas-soc, devicetree
In-Reply-To: <6b249f47-2a79-e9a0-0599-284301319a9d@kernel.org>
On 02/04/17 10:36, Jonathan Cameron wrote:
> On 27/03/17 16:12, Jacopo Mondi wrote:
>> Add documentation for max9611 driver.
>> Document attributes describing value of shunt resistor installed between
>> RS+ and RS- voltage sense inputs.
>>
>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Good. Will pick up once the binding is sorted.
>
> Kick me if I don't!
Oops. Missed the units being micro ohms. They want to be ohms to be consistent
within the ABI.
We let various odd units go in right back at the beginning to try and remain
compatible with hwmon. Turned out to be a bad idea. Also the ina2xx uses
ohms and you should stay consistent with that.
Thanks,
Jonathan
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
>> new file mode 100644
>> index 0000000..5020d00
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
>> @@ -0,0 +1,17 @@
>> +What: /sys/bus/iio/devices/iio:deviceX/in_power_shunt_resistor
>> +Date: March 2017
>> +KernelVersion: 4.12
>> +Contact: linux-iio@vger.kernel.org
>> +Description: The value of the shunt resistor used to compute power drain on
>> + common input voltage pin (RS+). In micro Ohms.
>> +
>> +What: /sys/bus/iio/devices/iio:deviceX/in_current_shunt_resistor
>> +Date: March 2017
>> +KernelVersion: 4.12
>> +Contact: linux-iio@vger.kernel.org
>> +Description: The value of the shunt resistor used to compute current flowing
>> + between RS+ and RS- voltage sense inputs. In micro Ohms.
>> +
>> +These attributes describe a single physical component, exposed as two distinct
>> +attributes as it is used to calculate two different values: power load and
>> +current flowing between RS+ and RS- inputs.
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v2] iio:adc: Driver for Linear Technology LTC2497 ADC
From: Jonathan Cameron @ 2017-04-02 9:54 UTC (permalink / raw)
To: Lars-Peter Clausen, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <f0229cde-15b6-d45b-6d85-3c90082d0226-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
On 02/04/17 10:52, Lars-Peter Clausen wrote:
> On 04/02/2017 11:30 AM, Jonathan Cameron wrote:
>> On 27/03/17 08:23, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org wrote:
>>> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>>>
>>> This patch adds support for the Analog Devices / Linear Technology
>>> LTC2497 ADCs. The LTC2497 is a 16-channel (eight differential),
>>> 16-bit, high precision, delta-sigma ADC with an automatic, differential,
>>> input current cancellation front end and a 2-wire, I2C interface.
>>>
>>> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>> Looks good to me.
>>
>> Applied to the togreg branch of iio.git and pushed out as testing for
>> the autobuilders to play with it.
>
> There is a v3 of this patch.
>
unapplied. Thanks Lars!
^ permalink raw reply
* Re: [PATCH v3] iio:adc: Driver for Linear Technology LTC2497 ADC
From: Jonathan Cameron @ 2017-04-02 9:58 UTC (permalink / raw)
To: michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
lars-Qo5EllUWu/uELgA04lAiVw, knaack.h-Mmb7MZpHnFY,
pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490780578-2468-1-git-send-email-michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
On 29/03/17 10:42, michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org wrote:
> From: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
>
> This patch adds support for the Analog Devices / Linear Technology
> LTC2497 ADCs. The LTC2497 is a 16-channel (eight differential),
> 16-bit, high precision, delta-sigma ADC with an automatic, differential,
> input current cancellation front end and a 2-wire, I2C interface.
>
> Signed-off-by: Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
oops. Picked up the wrong version (thanks Lars). Dropped v2 and
now applied v3.
Sorry about that
Jonathan
>
> ---
>
> Changes since v1:
>
> - remove confusing kconfig help text
> - use proper defines
> - add descriptive comment
> - fix removal order
> - add poper commit mssage
>
> Changes since v2:
>
> - Sort includes
> - Use BIT() defines
> - Avoid transfer buffers on the stack
> - use msleep_interruptible()
> - Fix documentation s/Should/Must/g
> ---
> .../devicetree/bindings/iio/adc/ltc2497.txt | 13 +
> MAINTAINERS | 1 +
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ltc2497.c | 279 +++++++++++++++++++++
> 5 files changed, 304 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> create mode 100644 drivers/iio/adc/ltc2497.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ltc2497.txt b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> new file mode 100644
> index 0000000..a237ed9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ltc2497.txt
> @@ -0,0 +1,13 @@
> +* Linear Technology / Analog Devices LTC2497 ADC
> +
> +Required properties:
> + - compatible: Must be "lltc,ltc2497"
> + - reg: Must contain the ADC I2C address
> + - vref-supply: The regulator supply for ADC reference voltage
> +
> +Example:
> + ltc2497: adc@76 {
> + compatible = "lltc,ltc2497";
> + reg = <0x76>;
> + vref-supply = <<c2497_reg>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a7d6f9a..173043c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -813,6 +813,7 @@ W: http://wiki.analog.com/
> W: http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/iio/*/ad*
> +F: drivers/iio/adc/ltc2497*
> X: drivers/iio/*/adjd*
> F: drivers/staging/iio/*/ad*
> F: drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2268a6f..acc115b 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -326,6 +326,16 @@ config LTC2485
> To compile this driver as a module, choose M here: the module will be
> called ltc2485.
>
> +config LTC2497
> + tristate "Linear Technology LTC2497 ADC driver"
> + depends on I2C
> + help
> + Say yes here to build support for Linear Technology LTC2497
> + 16-Bit 8-/16-Channel Delta Sigma ADC.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ltc2497.
> +
> config MAX1027
> tristate "Maxim max1027 ADC driver"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 73dbe39..9d626b5 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
> obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
> obj-$(CONFIG_LTC2485) += ltc2485.o
> +obj-$(CONFIG_LTC2497) += ltc2497.o
> obj-$(CONFIG_MAX1027) += max1027.o
> obj-$(CONFIG_MAX11100) += max11100.o
> obj-$(CONFIG_MAX1363) += max1363.o
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> new file mode 100644
> index 0000000..2691b10
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -0,0 +1,279 @@
> +/*
> + * ltc2497.c - Driver for Analog Devices/Linear Technology LTC2497 ADC
> + *
> + * Copyright (C) 2017 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * 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/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
> +
> +struct ltc2497_st {
> + 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.
> + */
> + __be32 buf ____cacheline_aligned;
> +};
> +
> +static int ltc2497_wait_conv(struct ltc2497_st *st)
> +{
> + 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);
> + 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)
> + return ret;
> +
> + *val = ret / 1000;
> + *val2 = 17;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +#define LTC2497_CHAN(_chan, _addr) { \
> + .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), \
> +}
> +
> +#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 ltc2497_channel[] = {
> + LTC2497_CHAN(0, LTC2497_SGL),
> + LTC2497_CHAN(1, LTC2497_SGL),
> + LTC2497_CHAN(2, LTC2497_SGL),
> + LTC2497_CHAN(3, LTC2497_SGL),
> + LTC2497_CHAN(4, LTC2497_SGL),
> + LTC2497_CHAN(5, LTC2497_SGL),
> + LTC2497_CHAN(6, LTC2497_SGL),
> + LTC2497_CHAN(7, LTC2497_SGL),
> + LTC2497_CHAN(8, LTC2497_SGL),
> + LTC2497_CHAN(9, LTC2497_SGL),
> + LTC2497_CHAN(10, LTC2497_SGL),
> + LTC2497_CHAN(11, LTC2497_SGL),
> + LTC2497_CHAN(12, LTC2497_SGL),
> + LTC2497_CHAN(13, LTC2497_SGL),
> + LTC2497_CHAN(14, LTC2497_SGL),
> + LTC2497_CHAN(15, LTC2497_SGL),
> + 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,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static int ltc2497_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct iio_dev *indio_dev;
> + struct ltc2497_st *st;
> + int ret;
> +
> + 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));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + st->client = client;
> +
> + 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;
> +
> + ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT);
> + if (ret < 0)
> + goto err_regulator_disable;
> +
> + st->addr_prev = LTC2497_CONFIG_DEFAULT;
> + st->time_prev = ktime_get();
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0)
> + goto err_regulator_disable;
> +
> + return 0;
> +
> +err_regulator_disable:
> + regulator_disable(st->ref);
> +
> + return ret;
> +}
> +
> +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_device_unregister(indio_dev);
> + regulator_disable(st->ref);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ltc2497_id[] = {
> + { "ltc2497", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ltc2497_id);
> +
> +static const struct of_device_id ltc2497_of_match[] = {
> + { .compatible = "lltc,ltc2497", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ltc2497_of_match);
> +
> +static struct i2c_driver ltc2497_driver = {
> + .driver = {
> + .name = "ltc2497",
> + .of_match_table = of_match_ptr(ltc2497_of_match),
> + },
> + .probe = ltc2497_probe,
> + .remove = ltc2497_remove,
> + .id_table = ltc2497_id,
> +};
> +module_i2c_driver(ltc2497_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Linear Technology LTC2497 ADC driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* [PATCH] powerpc/sequoia: fix NAND partitions not to overlap
From: Pavel Machek @ 2017-04-02 10:05 UTC (permalink / raw)
To: sr-ynQEQJNshbs, jwboyer-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
paulus-eUNUBHrolfbYtjvyW6yDsg, mpe-Gsx/Oe8HsFggBc27wqDAHg,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 639 bytes --]
Fix overlapping NAND partitions.
Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
diff --git a/arch/powerpc/boot/dts/sequoia.dts b/arch/powerpc/boot/dts/sequoia.dts
index b1d3292..e41b88a 100644
--- a/arch/powerpc/boot/dts/sequoia.dts
+++ b/arch/powerpc/boot/dts/sequoia.dts
@@ -229,7 +229,7 @@
};
partition@84000 {
label = "user";
- reg = <0x00000000 0x01f7c000>;
+ reg = <0x00084000 0x01f7c000>;
};
};
};
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related
* Re: [PATCH v2] iio: adc: add max1117/max1118/max1119 ADC driver
From: Jonathan Cameron @ 2017-04-02 10:23 UTC (permalink / raw)
To: Akinobu Mita, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Rob Herring, Mark Rutland, Mark Brown
In-Reply-To: <1490718888-4760-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 28/03/17 17:34, Akinobu Mita wrote:
> This adds max1117/max1118/max1119 8-bit, dual-channel ADC driver.
>
> This new driver uses the zero length spi_transfers with the cs_change
> flag set and/or the non-zero delay_usecs.
>
> 1. The zero length transfer with the spi_transfer.cs_change set is
> required in order to select CH1. The chip select line must be brought
> high and low again without transfer.
>
> 2. The zero length transfer with the spi_transfer.delay_usecs > 0 is
> required for waiting the conversion to be complete. The conversion
> begins with the falling edge of the chip select. During the conversion
> process, SCLK is ignored.
>
> These two usages are unusual. But the spi controller drivers that use
> a default implementation of transfer_one_message() are likely to work.
> (I've tested this adc driver with spi-omap2-mcspi and spi-xilinx)
>
> On the other hand, some spi controller drivers that have their own
> transfer_one_message() may not work. But at least for the zero length
> transfer with delay_usecs > 0, I'm proposing a new testcase for the
> spi-loopback-test that can test whether the delay_usecs setting has
> taken effect.
Thanks for the detailed description. As you might imagine I'll
be looking for an Ack from Mark Brown on this one - or if the discussion
is proceeding elsewhere, please post a link.
Otherwise driver looks great to me.
Give me a poke in a week or two if I seem to have forgotten it.
Jonathan
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>
> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
> Cc: Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> * v2
> - move max1118_remove() after max1118_probe() for usual code ordering
> - make the probe() fail for max1118 if vref-supply isn't acquired
> - add acked-by line
>
> .../devicetree/bindings/iio/adc/max1118.txt | 21 ++
> drivers/iio/adc/Kconfig | 12 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max1118.c | 307 +++++++++++++++++++++
> 4 files changed, 341 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/max1118.txt
> create mode 100644 drivers/iio/adc/max1118.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/max1118.txt b/Documentation/devicetree/bindings/iio/adc/max1118.txt
> new file mode 100644
> index 0000000..cf33d0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max1118.txt
> @@ -0,0 +1,21 @@
> +* MAX1117/MAX1118/MAX1119 8-bit, dual-channel ADCs
> +
> +Required properties:
> + - compatible: Should be one of
> + * "maxim,max1117"
> + * "maxim,max1118"
> + * "maxim,max1119"
> + - reg: spi chip select number for the device
> + - (max1118 only) vref-supply: The regulator supply for ADC reference voltage
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> + Documentation/devicetree/bindings/spi/spi-bus.txt
> +
> +Example:
> +adc@0 {
> + compatible = "maxim,max1118";
> + reg = <0>;
> + vref-supply = <&vdd_supply>;
> + spi-max-frequency = <1000000>;
> +};
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..66262d1 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -335,6 +335,18 @@ config MAX11100
> To compile this driver as a module, choose M here: the module will be
> called max11100.
>
> +config MAX1118
> + tristate "Maxim max1117/max1118/max1119 ADCs driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to build support for Maxim max1117/max1118/max1119
> + 8-bit, dual-channel ADCs.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called max1118.
> +
> config MAX1363
> tristate "Maxim max1363 ADC driver"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..5ef3470 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
> obj-$(CONFIG_LTC2485) += ltc2485.o
> obj-$(CONFIG_MAX1027) += max1027.o
> obj-$(CONFIG_MAX11100) += max11100.o
> +obj-$(CONFIG_MAX1118) += max1118.o
> obj-$(CONFIG_MAX1363) += max1363.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c
> new file mode 100644
> index 0000000..2e9648a
> --- /dev/null
> +++ b/drivers/iio/adc/max1118.c
> @@ -0,0 +1,307 @@
> +/*
> + * MAX1117/MAX1118/MAX1119 8-bit, dual-channel ADCs driver
> + *
> + * Copyright (c) 2017 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * Datasheet: https://datasheets.maximintegrated.com/en/ds/MAX1117-MAX1119.pdf
> + *
> + * SPI interface connections
> + *
> + * SPI MAXIM
> + * Master Direction MAX1117/8/9
> + * ------ --------- -----------
> + * nCS --> CNVST
> + * SCK --> SCLK
> + * MISO <-- DOUT
> + * ------ --------- -----------
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +enum max1118_id {
> + max1117,
> + max1118,
> + max1119,
> +};
> +
> +struct max1118 {
> + struct spi_device *spi;
> + struct mutex lock;
> + struct regulator *reg;
> +
> + u8 data ____cacheline_aligned;
> +};
> +
> +#define MAX1118_CHANNEL(ch) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .channel = (ch), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_index = ch, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 8, \
> + .storagebits = 8, \
> + }, \
> + }
> +
> +static const struct iio_chan_spec max1118_channels[] = {
> + MAX1118_CHANNEL(0),
> + MAX1118_CHANNEL(1),
> + IIO_CHAN_SOFT_TIMESTAMP(2),
> +};
> +
> +static int max1118_read(struct spi_device *spi, int channel)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct max1118 *adc = iio_priv(indio_dev);
> + struct spi_transfer xfers[] = {
> + /*
> + * To select CH1 for conversion, CNVST pin must be brought high
> + * and low for a second time.
> + */
> + {
> + .len = 0,
> + .delay_usecs = 1, /* > CNVST Low Time 100 ns */
> + .cs_change = 1,
> + },
> + /*
> + * The acquisition interval begins with the falling edge of
> + * CNVST. The total acquisition and conversion process takes
> + * <7.5us.
> + */
> + {
> + .len = 0,
> + .delay_usecs = 8,
> + },
> + {
> + .rx_buf = &adc->data,
> + .len = 1,
> + },
> + };
> + int ret;
> +
> + if (channel == 0)
> + ret = spi_sync_transfer(spi, xfers + 1, 2);
> + else
> + ret = spi_sync_transfer(spi, xfers, 3);
> +
> + if (ret)
> + return ret;
> +
> + return adc->data;
> +}
> +
> +static int max1118_get_vref_mV(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct max1118 *adc = iio_priv(indio_dev);
> + const struct spi_device_id *id = spi_get_device_id(spi);
> + int vref_uV;
> +
> + switch (id->driver_data) {
> + case max1117:
> + return 2048;
> + case max1119:
> + return 4096;
> + case max1118:
> + vref_uV = regulator_get_voltage(adc->reg);
> + if (vref_uV < 0)
> + return vref_uV;
> + return vref_uV / 1000;
> + }
> +
> + return -ENODEV;
> +}
> +
> +static int max1118_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max1118 *adc = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&adc->lock);
> + *val = max1118_read(adc->spi, chan->channel);
> + mutex_unlock(&adc->lock);
> + if (*val < 0)
> + return *val;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + *val = max1118_get_vref_mV(adc->spi);
> + if (*val < 0)
> + return *val;
> + *val2 = 8;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_info max1118_info = {
> + .read_raw = max1118_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static irqreturn_t max1118_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct max1118 *adc = iio_priv(indio_dev);
> + u8 data[16] = { }; /* 2x 8-bit ADC data + padding + 8 bytes timestamp */
> + int scan_index;
> + int i = 0;
> +
> + mutex_lock(&adc->lock);
> +
> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + const struct iio_chan_spec *scan_chan =
> + &indio_dev->channels[scan_index];
> + int ret = max1118_read(adc->spi, scan_chan->channel);
> +
> + if (ret < 0) {
> + dev_warn(&adc->spi->dev,
> + "failed to get conversion data\n");
> + goto out;
> + }
> +
> + data[i] = ret;
> + i++;
> + }
> + iio_push_to_buffers_with_timestamp(indio_dev, data,
> + iio_get_time_ns(indio_dev));
> +out:
> + mutex_unlock(&adc->lock);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int max1118_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct max1118 *adc;
> + const struct spi_device_id *id = spi_get_device_id(spi);
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + adc = iio_priv(indio_dev);
> + adc->spi = spi;
> + mutex_init(&adc->lock);
> +
> + if (id->driver_data == max1118) {
> + adc->reg = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(adc->reg)) {
> + dev_err(&spi->dev, "failed to get vref regulator\n");
> + return PTR_ERR(adc->reg);
> + }
> + ret = regulator_enable(adc->reg);
> + if (ret)
> + return ret;
> + }
> +
> + spi_set_drvdata(spi, indio_dev);
> +
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->info = &max1118_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = max1118_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max1118_channels);
> +
> + /*
> + * To reinitiate a conversion on CH0, it is necessary to allow for a
> + * conversion to be complete and all of the data to be read out. Once
> + * a conversion has been completed, the MAX1117/MAX1118/MAX1119 will go
> + * into AutoShutdown mode until the next conversion is initiated.
> + */
> + max1118_read(spi, 0);
> +
> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
> + max1118_trigger_handler, NULL);
> + if (ret)
> + goto err_reg_disable;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err_buffer_cleanup;
> +
> + return 0;
> +
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
> +err_reg_disable:
> + if (id->driver_data == max1118)
> + regulator_disable(adc->reg);
> +
> + return ret;
> +}
> +
> +static int max1118_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct max1118 *adc = iio_priv(indio_dev);
> + const struct spi_device_id *id = spi_get_device_id(spi);
> +
> + iio_device_unregister(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
> + if (id->driver_data == max1118)
> + return regulator_disable(adc->reg);
> +
> + return 0;
> +}
> +
> +static const struct spi_device_id max1118_id[] = {
> + { "max1117", max1117 },
> + { "max1118", max1118 },
> + { "max1119", max1119 },
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, max1118_id);
> +
> +#ifdef CONFIG_OF
> +
> +static const struct of_device_id max1118_dt_ids[] = {
> + { .compatible = "maxim,max1117" },
> + { .compatible = "maxim,max1118" },
> + { .compatible = "maxim,max1119" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, max1118_dt_ids);
> +
> +#endif
> +
> +static struct spi_driver max1118_spi_driver = {
> + .driver = {
> + .name = "max1118",
> + .of_match_table = of_match_ptr(max1118_dt_ids),
> + },
> + .probe = max1118_probe,
> + .remove = max1118_remove,
> + .id_table = max1118_id,
> +};
> +module_spi_driver(max1118_spi_driver);
> +
> +MODULE_AUTHOR("Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("MAXIM MAX1117/MAX1118/MAX1119 ADCs driver");
> +MODULE_LICENSE("GPL v2");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/5] ARM: dts: rockchip: add ARM Mali GPU node for rk3288
From: Enric Balletbo i Serra @ 2017-04-02 10:41 UTC (permalink / raw)
To: Guillaume Tucker, Rob Herring, Mark Rutland
Cc: Sjoerd Simons, John Reitan, Wookey, devicetree, linux-rockchip,
linux-arm-kernel, linux-kernel
In-Reply-To: <46d14d3c969a35e800b1c06da0ddb652986e6827.1491118230.git.guillaume.tucker@collabora.com>
On 02/04/17 09:59, Guillaume Tucker wrote:
> Add Mali GPU device tree node for the rk3288 SoC, with devfreq
> opp table.
>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
> arch/arm/boot/dts/rk3288.dtsi | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index df8a0dbe9d91..213224ea309e 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -43,6 +43,7 @@
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/pinctrl/rockchip.h>
> #include <dt-bindings/clock/rk3288-cru.h>
> +#include <dt-bindings/power/rk3288-power.h>
> #include <dt-bindings/thermal/thermal.h>
> #include <dt-bindings/power/rk3288-power.h>
> #include <dt-bindings/soc/rockchip,boot-mode.h>
> @@ -227,6 +228,28 @@
> ports = <&vopl_out>, <&vopb_out>;
> };
>
> + gpu: mali@ffa30000 {
> + compatible = "arm,mali-midgard";
> + reg = <0xffa30000 0x10000>;
> + interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "JOB", "MMU", "GPU";
> + clocks = <&cru ACLK_GPU>;
> + clock-names = "clk_mali";
> + operating-points = <
> + /* KHz uV */
> + 100000 950000
> + 200000 950000
> + 300000 1000000
> + 400000 1100000
> + 500000 1200000
> + 600000 1250000
> + >;
> + power-domains = <&power RK3288_PD_GPU>;
> + status = "disabled";
> + };
> +
> sdmmc: dwmmc@ff0c0000 {
> compatible = "rockchip,rk3288-dw-mshc";
> max-frequency = <150000000>;
>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
^ permalink raw reply
* Re: [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
From: Jonathan Cameron @ 2017-04-02 10:51 UTC (permalink / raw)
To: Quentin Schulz, Icenowy Zheng, Lee Jones, Rob Herring,
Maxime Ripard, Chen-Yu Tsai, Zhang Rui
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <813dc1d8-89b4-9c0e-c1cc-e76efde2be1f-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 29/03/17 07:54, Quentin Schulz wrote:
> Hi,
>
> On 28/03/2017 19:30, Icenowy Zheng wrote:
>> This adds support for the Allwinner H3 thermal sensor.
>>
>> Allwinner H3 has a thermal sensor like the one in A33, but have its
>> registers nearly all re-arranged, sample clock moved to CCU and a pair
>> of bus clock and reset added. It's also the base of newer SoCs' thermal
>> sensors.
>>
>> An option is added to gpadc_data struct, to indicate whether this device
>> is a new-generation Allwinner thermal sensor.
>>
>> The thermal sensors on A64 and H5 is like the one on H3, but with of
>> course different formula factors.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>> drivers/iio/adc/sun4i-gpadc-iio.c | 130 ++++++++++++++++++++++++++++++++------
>> include/linux/mfd/sun4i-gpadc.h | 33 +++++++++-
>> 2 files changed, 141 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
>> index 74705aa37982..7512b1cae877 100644
>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>> @@ -22,6 +22,7 @@
>> * shutdown for not being used.
>> */
>>
>> +#include <linux/clk.h>
>> #include <linux/completion.h>
>> #include <linux/interrupt.h>
>> #include <linux/io.h>
>> @@ -31,6 +32,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/regmap.h>
>> +#include <linux/reset.h>
>> #include <linux/thermal.h>
>> #include <linux/delay.h>
>>
>> @@ -56,6 +58,7 @@ struct gpadc_data {
>> unsigned int tp_adc_select;
>> unsigned int (*adc_chan_select)(unsigned int chan);
>> unsigned int adc_chan_mask;
>> + bool gen2_ths;
>> };
>>
>
> Instead of a boolean, give the TEMP_DATA register address.
>
>> static const struct gpadc_data sun4i_gpadc_data = {
>> @@ -88,7 +91,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
>> static const struct gpadc_data sun8i_a33_gpadc_data = {
>> .temp_offset = -1662,
>> .temp_scale = 162,
>> - .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
>> + .tp_mode_en = SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN,
>> +};
>
> Separate patch for this?
Indeed, ideal to rework original first (really easy to review)
then add support for new part.
>
>> +
>> +static const struct gpadc_data sun8i_h3_gpadc_data = {
>> + /*
>> + * The original formula on the datasheet seems to be wrong.
>> + * These factors are calculated based on the formula in the BSP
>> + * kernel, which is originally Tem = 217 - (T / 8.253), in which Tem
>> + * is the temperature in Celsius degree and T is the raw value
>> + * from the sensor.
>> + */
>> + .temp_offset = -1791,
>> + .temp_scale = -121,
>> + .gen2_ths = true,
>> };
>>
>> struct sun4i_gpadc_iio {
>> @@ -103,6 +119,9 @@ struct sun4i_gpadc_iio {
>> atomic_t ignore_temp_data_irq;
>> const struct gpadc_data *data;
>> bool no_irq;
>> + struct clk *ths_bus_clk;
>> + struct clk *ths_clk;
>> + struct reset_control *reset;
>> /* prevents concurrent reads of temperature and ADC */
>> struct mutex mutex;
>> };
>> @@ -274,7 +293,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
>> if (info->no_irq) {
>> pm_runtime_get_sync(indio_dev->dev.parent);
>>
>> - regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
>> + if (info->data->gen2_ths)
>> + regmap_read(info->regmap, SUN8I_H3_GPADC_TEMP_DATA,
>> + val);
>> + else
>> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
>>
>
> Instead of gen2_ths, use the TEMP_DATA register address.
Agreed. Put a set of register addresses into the gpadc_data structure.
Both more flexible and easier to read.
>
>> pm_runtime_mark_last_busy(indio_dev->dev.parent);
>> pm_runtime_put_autosuspend(indio_dev->dev.parent);
>> @@ -386,10 +409,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>> {
>> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>>
>> - /* Disable the ADC on IP */
>> - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
>> - /* Disable temperature sensor on IP */
>> - regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
>> + if (info->data->gen2_ths) {
>> + /* Disable temperature sensor */
>> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2, 0);
>> + } else {
>> + /* Disable the ADC on IP */
>> + regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
>> + /* Disable temperature sensor on IP */
>> + regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
>> + }
>>
>
> Either use another register address or add a suspend function to struct
> gpadc_data which will be different for each version of the IP.
>
>> return 0;
>> }
>> @@ -398,19 +426,36 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
>> {
>> struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>>
>> - /* clkin = 6MHz */
>> - regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
>> - SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
>> - SUN4I_GPADC_CTRL0_FS_DIV(7) |
>> - SUN4I_GPADC_CTRL0_T_ACQ(63));
>> - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, info->data->tp_mode_en);
>> - regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
>> - SUN4I_GPADC_CTRL3_FILTER_EN |
>> - SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
>> - /* period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin; ~0.6s */
>> - regmap_write(info->regmap, SUN4I_GPADC_TPR,
>> - SUN4I_GPADC_TPR_TEMP_ENABLE |
>> - SUN4I_GPADC_TPR_TEMP_PERIOD(800));
>> + if (info->data->gen2_ths) {
>> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2,
>> + SUN8I_H3_GPADC_CTRL2_TEMP_SENSE_EN |
>> + SUN8I_H3_GPADC_CTRL2_T_ACQ1(31));
>> + regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
>> + SUN4I_GPADC_CTRL0_T_ACQ(31));
>> + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL3,
>> + SUN4I_GPADC_CTRL3_FILTER_EN |
>> + SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
>> + regmap_write(info->regmap, SUN8I_H3_GPADC_INTC,
>> + SUN8I_H3_GPADC_INTC_TEMP_PERIOD(800));
>> + } else {
>> + /* clkin = 6MHz */
>> + regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
>> + SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
>> + SUN4I_GPADC_CTRL0_FS_DIV(7) |
>> + SUN4I_GPADC_CTRL0_T_ACQ(63));
Looks like you'll need something like a function pointer in your
_data structure as well to cover cases where they are totally
different like here. or you could use regmap fields if they map
reasonably well... (doesn't look like they do though!)
>> + regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
>> + info->data->tp_mode_en);
>> + regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
>> + SUN4I_GPADC_CTRL3_FILTER_EN |
>> + SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
>> + /*
>> + * period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin;
>> + * ~0.6s
>> + */
>> + regmap_write(info->regmap, SUN4I_GPADC_TPR,
>> + SUN4I_GPADC_TPR_TEMP_ENABLE |
>> + SUN4I_GPADC_TPR_TEMP_PERIOD(800));
>> + }
>>
>
> Same here as suspend function?
>
>> return 0;
>> }
>> @@ -494,6 +539,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
>> .compatible = "allwinner,sun8i-a33-ths",
>> .data = &sun8i_a33_gpadc_data,
>> },
>> + {
>> + .compatible = "allwinner,sun8i-h3-ths",
>> + .data = &sun8i_h3_gpadc_data,
>> + },
>> { /* sentinel */ }
>> };
>>
>> @@ -529,6 +578,43 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>> return ret;
>> }
>>
>> + if (info->data->gen2_ths) {
>> + info->reset = devm_reset_control_get(&pdev->dev, NULL);
>> + if (IS_ERR(info->reset)) {
>> + ret = PTR_ERR(info->reset);
>> + return ret;
>> + }
>> +
>> + ret = reset_control_deassert(info->reset);
>> + if (ret)
>> + return ret;
>> +
>> + info->ths_bus_clk = devm_clk_get(&pdev->dev, "bus");
>> + if (IS_ERR(info->ths_bus_clk)) {
>> + ret = PTR_ERR(info->ths_bus_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(info->ths_bus_clk);
>> + if (ret)
>> + return ret;
>> +
>> + info->ths_clk = devm_clk_get(&pdev->dev, "ths");
>> + if (IS_ERR(info->ths_clk)) {
>> + ret = PTR_ERR(info->ths_clk);
>> + return ret;
>> + }
>> +
>> + /* Running at 6MHz */
>> + ret = clk_set_rate(info->ths_clk, 6000000);
>> + if (ret)
>> + return ret;
>> +
>> + ret = clk_prepare_enable(info->ths_clk);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> if (!IS_ENABLED(CONFIG_THERMAL_OF))
>> return 0;
>>
>> @@ -691,6 +777,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
>> if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
>> iio_map_array_unregister(indio_dev);
>>
>> + if (info->data->gen2_ths) {
>> + clk_disable_unprepare(info->ths_clk);
>> + clk_disable_unprepare(info->ths_bus_clk);
>> + reset_control_deassert(info->reset);
>> + }
>> +
>
> I'm not really fond of using this boolean as I don't see it being
> possibly reused for any other SoCs that has a GPADC or THS.
>
> Here, you could make use of a list/array of clk which then can be reused
> for other SoCs just by changing the list. Add a default rate to the
> gpadc_data structure and you're go to go.
>
>> return 0;
>> }
>>
>> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
>> index 139872c2e0fe..f794a2988a93 100644
>> --- a/include/linux/mfd/sun4i-gpadc.h
>> +++ b/include/linux/mfd/sun4i-gpadc.h
>> @@ -38,9 +38,12 @@
>> #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x))
>> #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0)
>>
>> -/* TP_CTRL1 bits for sun8i SoCs */
>> -#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
>> -#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
>> +/* TP_CTRL1 bits for sun8i A23/A33 SoCs */
>> +#define SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN BIT(8)
>> +#define SUN8I_A23_GPADC_CTRL1_GPADC_CALI_EN BIT(7)
>> +
>
> Different patch for these?
Looks to me like you want a precursor that reworks the driver to
be ready to support the new part, followed by a patch that actually
introduces the new part.
Jonathan
>
> Thanks,
> Quentin
>
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: stm32-dac: Add support for STM32 DAC
From: Jonathan Cameron @ 2017-04-02 11:16 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, amelie.delaunay-qxv4g6HH51o
In-Reply-To: <1490960707-22422-2-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 31/03/17 12:45, Fabrice Gasnier wrote:
> Document STMicroelectronics STM32 DAC (digital-to-analog converter).
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> ---
> .../devicetree/bindings/iio/dac/st,stm32-dac.txt | 56 ++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/st,stm32-dac.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.txt b/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.txt
> new file mode 100644
> index 0000000..1981983
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.txt
> @@ -0,0 +1,56 @@
> +STMicroelectronics STM32 DAC
> +
> +The STM32 DAC is a 12-bit voltage output digital-to-analog converter. The DAC
> +can be configured in 8- or 12-bit mode. In 12-bit mode, the data could be
> +left- or right-aligned.
Whilst possibly true, do we care about the alignment? That'll all get wrapped
up in the driver.
> It has two output channels, each with its own converter.
> +It has built-in noise and triangle waveform generator and supports external
> +triggers for conversions. The DAC's output buffer allows a high drive output
> +current.
Ah.. This is going to be fun :) More unusual hardware to find in an SoC.
> +
> +Contents of a stm32 dac root node:
> +-----------------------------------
> +Required properties:
> +- compatible: Must be "st,stm32h7-dac-core".
> +- reg: Offset and length of the device's register set.
> +- clocks: Must contain an entry for pclk (which feeds the peripheral bus
> + interface)
> +- clock-names: Must be "pclk".
> +- vref-supply: Phandle to the vref+ input analog reference supply.
> +
> +Optional properties:
> +- resets: Must contain the phandle to the reset controller.
> +- A pinctrl state named "default" for each DAC channel may be defined to set
> + DAC_OUTx pin in mode of operation for analog output on external pin.
> +
> +Contents of a stm32 dac child node:
> +-----------------------------------
> +DAC core node should contain at least one subnode, representing a
> +DAC instance/channel available on the machine.
> +
> +Required properties:
> +- compatible: Must be "st,stm32-dac".
> +- #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
> + Documentation/devicetree/bindings/iio/iio-bindings.txt
> +- st,dac-channel: Must be either 1 or 2, to define channel in use (e.g.
> + single channels: 1 or 2)
> +
> +Example:
> + dac: dac@40007400 {
> + compatible = "st,stm32h7-dac-core";
> + reg = <0x40007400 0x400>;
> + clocks = <&clk>;
> + clock-names = "pclk";
> + vref-supply = <®_vref>;
> +
> + dac1: dac@1 {
> + compatible = "st,stm32-dac";
> + #io-channels-cells = <1>;
> + st,dac-channel = <1>;
> + };
> +
> + dac2: dac@2 {
> + compatible = "st,stm32-dac";
> + #io-channels-cells = <1>;
> + st,dac-channel = <2>;
> + };
> + };
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH linux v9 2/5] hwmon: occ: Add sysfs interface
From: Guenter Roeck @ 2017-04-02 11:19 UTC (permalink / raw)
To: Eddie James
Cc: devicetree, linux-kernel, linux-hwmon, linux-doc, jdelvare,
corbet, mark.rutland, robh+dt, wsa, andrew, benh, joel,
Edward A. James
In-Reply-To: <1489524906-19411-3-git-send-email-eajames@linux.vnet.ibm.com>
On 03/14/2017 01:55 PM, Eddie James wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> Add a generic mechanism to expose the sensors provided by the OCC in
> sysfs.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> Documentation/hwmon/occ | 62 +++++++++++
> drivers/hwmon/occ/Makefile | 2 +-
> drivers/hwmon/occ/occ_sysfs.c | 253 ++++++++++++++++++++++++++++++++++++++++++
> drivers/hwmon/occ/occ_sysfs.h | 25 +++++
> 4 files changed, 341 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwmon/occ/occ_sysfs.c
> create mode 100644 drivers/hwmon/occ/occ_sysfs.h
>
> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
> index d1c863b..580af26 100644
> --- a/Documentation/hwmon/occ
> +++ b/Documentation/hwmon/occ
> @@ -27,6 +27,68 @@ Currently, all versions of the OCC support four types of sensor data: power,
> temperature, frequency, and "caps," which indicate limits and thresholds used
> internally on the OCC.
>
> +sysfs Entries
> +-------------
> +
> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
> +
> +The driver exports a number of sysfs files for each type of sensor. The
> +sensor-specific files vary depending on the processor type, though many of the
> +attributes are common for both the POWER8 and POWER9.
> +
> +The hwmon interface cannot define every type of sensor that may be used.
> +Therefore, the frequency sensor on the OCC uses the "input" type sensor defined
> +by the hwmon interface, rather than defining a new type of custom sensor.
> +
> +Below are detailed the names and meaning of each sensor file for both types of
> +processors. All sensors are read-only unless otherwise specified. <x> indicates
> +the hwmon index. sensor id indicates the unique internal OCC identifer. Please
> +see the POWER OCC specification for details on all these sensor values.
> +
> +frequency:
> + all processors:
> + in<x>_input - frequency value
> + in<x>_label - sensor id
> +temperature:
> + POWER8:
> + temp<x>_input - temperature value
> + temp<x>_label - sensor id
> + POWER9 (in addition to above):
> + temp<x>_type - FRU type
> +
> +power:
> + POWER8:
> + power<x>_input - power value
> + power<x>_label - sensor id
> + power<x>_average - accumulator
> + power<x>_average_interval - update tag (number of samples in
> + accumulator)
> + POWER9:
> + power<x>_input - power value
> + power<x>_label - sensor id
> + power<x>_average_min - accumulator[0]
> + power<x>_average_max - accumulator[1] (64 bits total)
> + power<x>_average_interval - update tag
> + power<x>_reset_history - (function_id | (apss_channel << 8)
> +
> +caps:
> + POWER8:
> + power<x>_cap - current powercap
> + power<x>_cap_max - max powercap
> + power<x>_cap_min - min powercap
> + power<x>_max - normal powercap
> + power<x>_alarm - user powercap, r/w
> + POWER9:
> + power<x>_cap_alarm - user powercap source
> +
> +The driver also provides two sysfs entries through hwmon to better
> +control the driver and monitor the master OCC. Though there may be multiple
> +OCCs present on the system, these two files are only present for the "master"
> +OCC.
> + name - read the name of the driver
> + update_interval - read or write the minimum interval for polling the
> + OCC.
> +
> BMC - Host Communications
> -------------------------
>
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index 3ed79a5..67b5367 100644
> --- a/drivers/hwmon/occ/Makefile
> +++ b/drivers/hwmon/occ/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_SENSORS_IBM_OCC) += occ.o
> +obj-$(CONFIG_SENSORS_IBM_OCC) += occ.o occ_sysfs.o
> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
> new file mode 100644
> index 0000000..50b20e2
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_sysfs.c
> @@ -0,0 +1,253 @@
> +/*
> + * occ_sysfs.c - OCC sysfs interface
> + *
> + * This file contains the methods and data structures for implementing the OCC
> + * hwmon sysfs entries.
> + *
> + * Copyright 2017 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include "occ.h"
> +#include "occ_sysfs.h"
> +
> +#define OCC_HWMON_NAME_LENGTH 32
> +
> +struct occ_sysfs {
> + struct device *dev;
> + struct occ *occ;
> +
> + char label_buffer[OCC_HWMON_NAME_LENGTH + 1];
> + char hwmon_name[OCC_HWMON_NAME_LENGTH + 1];
> + const u32 *sensor_hwmon_configs;
> + struct hwmon_channel_info **occ_sensors;
> + struct hwmon_chip_info occ_info;
> + u16 user_powercap;
> +};
> +
> +static int occ_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + int rc;
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> + struct occ *occ = driver->occ;
> +
> + switch (type) {
> + case hwmon_in:
> + rc = occ_get_sensor_field(occ, FREQ, channel, attr, val);
> + break;
> + case hwmon_temp:
> + rc = occ_get_sensor_field(occ, TEMP, channel, attr, val);
> + break;
> + case hwmon_power:
> + rc = occ_get_sensor_field(occ, POWER, channel, attr, val);
> + break;
> + default:
> + rc = -EOPNOTSUPP;
> + }
> +
> + return rc;
> +}
> +
> +static int occ_hwmon_read_string(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + int rc;
> + unsigned long val = 0;
> + struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> + if (!((type == hwmon_in && attr == hwmon_in_label) ||
> + (type == hwmon_temp && attr == hwmon_temp_label) ||
> + (type == hwmon_power && attr == hwmon_power_label)))
> + return -EOPNOTSUPP;
> +
> + /* will fetch the "label", the sensor_id */
> + rc = occ_hwmon_read(dev, type, attr, channel, &val);
> + if (rc < 0)
> + return rc;
> +
> + /* just use one label buffer for all sensors. works with current hwmon
> + * implementation. only alternative is to store a buffer for each
> + * sensor, which gets expensive quickly.
> + */
Sorry for the late reply.
No, this doesn't work and is racy. Reading all labels from multiple threads
in parallel will result in random data.
The label is supposed to be constant for each sensor. If it isn't, it is not
a label. Either create it as constant and use the generated string, or drop
the attribute.
Guenter
^ permalink raw reply
* Re: [PATCH 2/4] iio: dac: add support for stm32 DAC
From: Jonathan Cameron @ 2017-04-02 11:32 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, amelie.delaunay-qxv4g6HH51o
In-Reply-To: <1490960707-22422-3-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 31/03/17 12:45, Fabrice Gasnier wrote:
> Add support for STMicroelectronics STM32 DAC. It's a 12-bit, voltage
> output digital-to-analog converter. It has two output channels, each
> with its own converter.
> It supports 8 bits or 12bits left/right aligned data format. Only
> 12bits right-aligned is used here. It has built-in noise or
> triangle waveform generator, and supports external triggers for
> conversions.
> Each channel can be used independently, with separate trigger, then
> separate IIO devices are used to handle this. Core driver is intended
> to share common resources such as clock, reset, reference voltage and
> registers.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Annoyingly my laptop just crashed mid way through reviewing this..
Ah well, hopefully I'll remember everything (there wasn't much).
For DACs the 'enable' attribute is not normally used. Rather we
use the powerdown one. The reasoning being that we care about what
the state is when it is powered down. Even if that isn't controllable
I would expect to see it exported as powerdown_mode with a fixed value.
Other than that - looks pretty good to me.
Jonathan
> ---
> drivers/iio/dac/Kconfig | 15 ++
> drivers/iio/dac/Makefile | 2 +
> drivers/iio/dac/stm32-dac-core.c | 180 ++++++++++++++++++++++++
> drivers/iio/dac/stm32-dac-core.h | 51 +++++++
> drivers/iio/dac/stm32-dac.c | 296 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 544 insertions(+)
> create mode 100644 drivers/iio/dac/stm32-dac-core.c
> create mode 100644 drivers/iio/dac/stm32-dac-core.h
> create mode 100644 drivers/iio/dac/stm32-dac.c
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index d3084028..7198648 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -274,6 +274,21 @@ config MCP4922
> To compile this driver as a module, choose M here: the module
> will be called mcp4922.
>
> +config STM32_DAC
> + tristate "STMicroelectronics STM32 DAC"
> + depends on (ARCH_STM32 && OF) || COMPILE_TEST
> + depends on REGULATOR
> + select STM32_DAC_CORE
> + help
> + Say yes here to build support for STMicroelectronics STM32 Digital
> + to Analog Converter (DAC).
> +
> + This driver can also be built as a module. If so, the module
> + will be called stm32-dac.
> +
> +config STM32_DAC_CORE
> + tristate
> +
> config VF610_DAC
> tristate "Vybrid vf610 DAC driver"
> depends on OF
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index f01bf4a..afe8ae7 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -29,4 +29,6 @@ obj-$(CONFIG_MAX517) += max517.o
> obj-$(CONFIG_MAX5821) += max5821.o
> obj-$(CONFIG_MCP4725) += mcp4725.o
> obj-$(CONFIG_MCP4922) += mcp4922.o
> +obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
> +obj-$(CONFIG_STM32_DAC) += stm32-dac.o
> obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/stm32-dac-core.c b/drivers/iio/dac/stm32-dac-core.c
> new file mode 100644
> index 0000000..75e4878
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac-core.c
> @@ -0,0 +1,180 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +
> +#include "stm32-dac-core.h"
> +
> +/**
> + * struct stm32_dac_priv - stm32 DAC core private data
> + * @pclk: peripheral clock common for all DACs
> + * @rst: peripheral reset control
> + * @vref: regulator reference
> + * @common: Common data for all DAC instances
> + */
> +struct stm32_dac_priv {
> + struct clk *pclk;
> + struct reset_control *rst;
> + struct regulator *vref;
> + struct stm32_dac_common common;
> +};
> +
> +static struct stm32_dac_priv *to_stm32_dac_priv(struct stm32_dac_common *com)
> +{
> + return container_of(com, struct stm32_dac_priv, common);
> +}
> +
> +static const struct regmap_config stm32_dac_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 0x3fc,
> +};
> +
> +static int stm32_dac_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct stm32_dac_priv *priv;
> + struct regmap *regmap;
> + struct resource *res;
> + void __iomem *mmio;
> + int ret;
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mmio = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mmio))
> + return PTR_ERR(mmio);
> +
> + regmap = devm_regmap_init_mmio(dev, mmio, &stm32_dac_regmap_cfg);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> + priv->common.regmap = regmap;
> +
> + priv->vref = devm_regulator_get(dev, "vref");
> + if (IS_ERR(priv->vref)) {
> + ret = PTR_ERR(priv->vref);
> + dev_err(dev, "vref get failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(priv->vref);
> + if (ret < 0) {
> + dev_err(dev, "vref enable failed\n");
> + return ret;
> + }
> +
> + ret = regulator_get_voltage(priv->vref);
> + if (ret < 0) {
> + dev_err(dev, "vref get voltage failed, %d\n", ret);
> + goto err_vref;
> + }
> + priv->common.vref_mv = ret / 1000;
> + dev_dbg(dev, "vref+=%dmV\n", priv->common.vref_mv);
> +
> + priv->pclk = devm_clk_get(dev, "pclk");
> + if (IS_ERR(priv->pclk)) {
> + ret = PTR_ERR(priv->pclk);
> + dev_err(dev, "pclk get failed\n");
> + goto err_vref;
> + }
> +
> + ret = clk_prepare_enable(priv->pclk);
> + if (ret < 0) {
> + dev_err(dev, "pclk enable failed\n");
> + goto err_vref;
> + }
> +
> + priv->rst = devm_reset_control_get(dev, NULL);
> + if (!IS_ERR(priv->rst)) {
> + reset_control_assert(priv->rst);
> + udelay(2);
> + reset_control_deassert(priv->rst);
> + }
> +
> + /* When clock speed is higher than 80MHz, set HFSEL */
> + priv->common.hfsel = (clk_get_rate(priv->pclk) > 80000000UL);
> + ret = regmap_update_bits(regmap, STM32_DAC_CR, STM32H7_DAC_CR_HFSEL,
> + priv->common.hfsel ? STM32H7_DAC_CR_HFSEL : 0);
> + if (ret)
> + goto err_pclk;
> +
> + platform_set_drvdata(pdev, &priv->common);
> +
> + ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, dev);
> + if (ret < 0) {
> + dev_err(dev, "failed to populate DT children\n");
> + goto err_pclk;
> + }
> +
> + return 0;
> +
> +err_pclk:
> + clk_disable_unprepare(priv->pclk);
> +err_vref:
> + regulator_disable(priv->vref);
> +
> + return ret;
> +}
> +
> +static int stm32_dac_remove(struct platform_device *pdev)
> +{
> + struct stm32_dac_common *common = platform_get_drvdata(pdev);
> + struct stm32_dac_priv *priv = to_stm32_dac_priv(common);
> +
> + of_platform_depopulate(&pdev->dev);
> + clk_disable_unprepare(priv->pclk);
> + regulator_disable(priv->vref);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_dac_of_match[] = {
> + { .compatible = "st,stm32h7-dac-core", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dac_of_match);
> +
> +static struct platform_driver stm32_dac_driver = {
> + .probe = stm32_dac_probe,
> + .remove = stm32_dac_remove,
> + .driver = {
> + .name = "stm32-dac-core",
> + .of_match_table = stm32_dac_of_match,
> + },
> +};
> +module_platform_driver(stm32_dac_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 DAC core driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:stm32-dac-core");
> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
> new file mode 100644
> index 0000000..d3099f7
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -0,0 +1,51 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __STM32_DAC_CORE_H
> +#define __STM32_DAC_CORE_H
> +
> +#include <linux/regmap.h>
> +
> +/* STM32 DAC registers */
> +#define STM32_DAC_CR 0x00
> +#define STM32_DAC_DHR12R1 0x08
> +#define STM32_DAC_DHR12R2 0x14
> +#define STM32_DAC_DOR1 0x2C
> +#define STM32_DAC_DOR2 0x30
> +
> +/* STM32_DAC_CR bit fields */
> +#define STM32_DAC_CR_EN1 BIT(0)
> +#define STM32H7_DAC_CR_HFSEL BIT(15)
> +#define STM32_DAC_CR_EN2 BIT(16)
> +
> +/**
> + * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
> + * @regmap: DAC registers shared via regmap
> + * @vref_mv: reference voltage (mv)
> + * @hfsel: high speed bus clock
> + */
> +struct stm32_dac_common {
> + struct regmap *regmap;
> + int vref_mv;
> + bool hfsel;
> +};
> +
> +#endif
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> new file mode 100644
> index 0000000..ee9711d
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -0,0 +1,296 @@
> +/*
> + * This file is part of STM32 DAC driver
> + *
> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
> + * Authors: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> + * Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> + *
> + * License type: GPLv2
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "stm32-dac-core.h"
> +
> +#define STM32_DAC_CHANNEL_1 1
> +#define STM32_DAC_CHANNEL_2 2
> +
> +/**
> + * struct stm32_dac - private data of DAC driver
> + * @common: reference to DAC common data
> + */
> +struct stm32_dac {
> + struct stm32_dac_common *common;
> +};
> +
> +static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
> +{
> + u32 en, val;
> + int ret;
> +
> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
> + if (ret < 0)
> + return ret;
> + if (channel == STM32_DAC_CHANNEL_1)
> + en = FIELD_GET(STM32_DAC_CR_EN1, val);
> + else
> + en = FIELD_GET(STM32_DAC_CR_EN2, val);
> +
> + return !!en;
> +}
> +
> +static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 en = (channel == STM32_DAC_CHANNEL_1) ?
> + STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
> + int ret;
> +
> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, en);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "Enable failed\n");
> + return ret;
> + }
> +
> + /*
> + * When HFSEL is set, it is not allowed to write the DHRx register
> + * during 8 clock cycles after the ENx bit is set. It is not allowed
> + * to make software/hardware trigger during this period neither.
> + */
> + if (dac->common->hfsel)
> + udelay(1);
> +
> + return 0;
> +}
> +
> +static int stm32_dac_disable(struct iio_dev *indio_dev, int channel)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 en = (channel == STM32_DAC_CHANNEL_1) ?
> + STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
> + int ret;
> +
> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, 0);
> + if (ret)
> + dev_err(&indio_dev->dev, "Disable failed\n");
> +
> + return ret;
> +}
> +
> +static int stm32_dac_get_value(struct stm32_dac *dac, int channel, int *val)
> +{
> + int ret;
> +
> + if (channel == STM32_DAC_CHANNEL_1)
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DOR1, val);
> + else
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DOR2, val);
> +
> + return ret ? ret : IIO_VAL_INT;
> +}
> +
> +static int stm32_dac_set_value(struct stm32_dac *dac, int channel, int val)
> +{
> + int ret;
> +
> + if (channel == STM32_DAC_CHANNEL_1)
> + ret = regmap_write(dac->common->regmap, STM32_DAC_DHR12R1, val);
> + else
> + ret = regmap_write(dac->common->regmap, STM32_DAC_DHR12R2, val);
> +
> + return ret;
> +}
> +
> +static int stm32_dac_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return stm32_dac_get_value(dac, chan->channel, val);
> + case IIO_CHAN_INFO_SCALE:
> + *val = dac->common->vref_mv;
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_ENABLE:
> + ret = stm32_dac_is_enabled(dac, chan->channel);
> + if (ret < 0)
> + return ret;
> + *val = ret;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int stm32_dac_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return stm32_dac_set_value(dac, chan->channel, val);
> + case IIO_CHAN_INFO_ENABLE:
> + if (!!val)
> + return stm32_dac_enable(indio_dev, chan->channel);
> + else
> + return stm32_dac_disable(indio_dev, chan->channel);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned reg, unsigned writeval,
> + unsigned *readval)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + if (!readval)
> + return regmap_write(dac->common->regmap, reg, writeval);
> + else
> + return regmap_read(dac->common->regmap, reg, readval);
> +}
> +
> +static const struct iio_info stm32_dac_iio_info = {
> + .read_raw = &stm32_dac_read_raw,
> + .write_raw = &stm32_dac_write_raw,
> + .debugfs_reg_access = &stm32_dac_debugfs_reg_access,
> + .driver_module = THIS_MODULE,
> +};
> +
> +#define STM32_DAC_CHANNEL(chan, name) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .output = 1, \
> + .channel = chan, \
> + .info_mask_separate = \
> + BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_ENABLE) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .datasheet_name = name, \
> +}
> +
> +static const struct iio_chan_spec stm32_dac_channels[] = {
> + STM32_DAC_CHANNEL(STM32_DAC_CHANNEL_1, "out1"),
> + STM32_DAC_CHANNEL(STM32_DAC_CHANNEL_2, "out2"),
> +};
> +
> +static int stm32_dac_chan_of_init(struct iio_dev *indio_dev)
> +{
> + struct device_node *np = indio_dev->dev.of_node;
> + unsigned int i;
> + u32 channel;
> + int ret;
> +
> + ret = of_property_read_u32(np, "st,dac-channel", &channel);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Failed to read st,dac-channel\n");
> + return ret;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(stm32_dac_channels); i++) {
> + if (stm32_dac_channels[i].channel == channel)
> + break;
> + }
> + if (i >= ARRAY_SIZE(stm32_dac_channels)) {
> + dev_err(&indio_dev->dev, "Invalid st,dac-channel\n");
> + return -EINVAL;
> + }
> +
> + indio_dev->channels = &stm32_dac_channels[i];
> + indio_dev->num_channels = 1;
> +
> + return 0;
> +};
> +
> +static int stm32_dac_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev;
> + struct stm32_dac *dac;
> + int ret;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*dac));
> + if (!indio_dev)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, indio_dev);
> +
> + dac = iio_priv(indio_dev);
> + dac->common = dev_get_drvdata(pdev->dev.parent);
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &stm32_dac_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = stm32_dac_chan_of_init(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int stm32_dac_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> + iio_device_unregister(indio_dev);
use devm_iio_device_register and drop the remove entirely
(I guess this may make no sense once I've looked at later
patches however!)
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_dac_of_match[] = {
> + { .compatible = "st,stm32-dac", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dac_of_match);
> +
> +static struct platform_driver stm32_dac_driver = {
> + .probe = stm32_dac_probe,
> + .remove = stm32_dac_remove,
> + .driver = {
> + .name = "stm32-dac",
> + .of_match_table = stm32_dac_of_match,
> + },
> +};
> +module_platform_driver(stm32_dac_driver);
> +
> +MODULE_ALIAS("platform:stm32-dac");
> +MODULE_AUTHOR("Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 DAC driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH v6] hwmon: ads7828 optional parameters from the device tree
From: Guenter Roeck @ 2017-04-02 11:35 UTC (permalink / raw)
To: Sam Povilus, wsa, jdelvare, linux-hwmon, linux-i2c, devicetree,
robh+dt, mark.rutland
In-Reply-To: <20170330034637.29708-1-kernel.development@povil.us>
On 03/29/2017 08:46 PM, Sam Povilus wrote:
> Adding the ability for the ads7828 and ads7830 to use the device tree to
> get their optional parameters, instead of using platform devices. This
> allows people using custom boards to also use the ads7828 in a non-default
> manner.
>
> v2: conforming to coding style
> v3: changing from "_" to "-" for device tree entries
> v4: using regulator subsystem for voltage source, renaming differential
> input property
> v5: checking for invalid vref values sooner and returning error if they are
> bad, not warning user.
> v6: optimization per Guenter Roeck
>
> Signed-off-by: Sam Povilus <kernel.development@povil.us>
Applied, after fixing up some whitespace errors in ads7828.txt.
Guenter
> ---
> .../devicetree/bindings/hwmon/ads7828.txt | 26 ++++++++++++++++++++++
> drivers/hwmon/ads7828.c | 15 +++++++++++++
> 2 files changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/ads7828.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ads7828.txt b/Documentation/devicetree/bindings/hwmon/ads7828.txt
> new file mode 100644
> index 000000000000..a1251b19079d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ads7828.txt
> @@ -0,0 +1,26 @@
> +ads7828 properties
> +
> +Required properties:
> +- compatible: Should be one of
> + ti,ads7828
> + ti,ads7830
> +- reg: I2C address
> +
> +Optional properties:
> +
> +- ti,differential-input
> + Set to use the device in differential mode.
> +- vref
changed to vref-supply
> + The external reference on the device is set to this regulators output. If it
> + does not exists the internal reference will be used and output by the ads78xx
> + on the "external vref" pin.
> +
> + Example ADS7828 node:
> +
> + ads7828: ads@48 {
> + comatible = "ti,ads7828";
> + reg = <0x48>;
> + vref-supply = <&vref>;
> + ti,differential-input;
> + };
> +
> diff --git a/drivers/hwmon/ads7828.c b/drivers/hwmon/ads7828.c
> index ee396ff167d9..88330a233b1d 100644
> --- a/drivers/hwmon/ads7828.c
> +++ b/drivers/hwmon/ads7828.c
> @@ -34,6 +34,7 @@
> #include <linux/platform_data/ads7828.h>
> #include <linux/regmap.h>
> #include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
>
> /* The ADS7828 registers */
> #define ADS7828_CMD_SD_SE 0x80 /* Single ended inputs */
> @@ -118,9 +119,11 @@ static int ads7828_probe(struct i2c_client *client,
> struct ads7828_data *data;
> struct device *hwmon_dev;
> unsigned int vref_mv = ADS7828_INT_VREF_MV;
> + unsigned int vref_uv;
> bool diff_input = false;
> bool ext_vref = false;
> unsigned int regval;
> + struct regulator *reg;
>
> data = devm_kzalloc(dev, sizeof(struct ads7828_data), GFP_KERNEL);
> if (!data)
> @@ -131,6 +134,18 @@ static int ads7828_probe(struct i2c_client *client,
> ext_vref = pdata->ext_vref;
> if (ext_vref && pdata->vref_mv)
> vref_mv = pdata->vref_mv;
> + } else if (dev->of_node) {
> + diff_input = of_property_read_bool(dev->of_node,
> + "ti,differential-input");
> + reg = devm_regulator_get_optional(dev, "vref");
> + if (!IS_ERR(reg)) {
> + vref_uv = regulator_get_voltage(reg);
> + vref_mv = DIV_ROUND_CLOSEST(vref_uv, 1000);
> + if (vref_mv < ADS7828_EXT_VREF_MV_MIN ||
> + vref_mv > ADS7828_EXT_VREF_MV_MAX)
> + return -EINVAL;
> + ext_vref = true;
> + }
> }
>
> /* Bound Vref with min/max values */
>
^ permalink raw reply
* Re: [PATCH 3/4] iio: dac: stm32: add support for trigger events
From: Jonathan Cameron @ 2017-04-02 11:45 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, amelie.delaunay-qxv4g6HH51o
In-Reply-To: <1490960707-22422-4-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 31/03/17 12:45, Fabrice Gasnier wrote:
> STM32 DAC supports triggers to synchronize conversions. When trigger
> occurs, data is transferred from DHR (data holding register) to DOR
> (data output register) so output voltage is updated.
> Both hardware and software triggers are supported.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Hmm. This is a somewhat different use of triggered event from normal...
What you have here is rather closer to the output buffers stuff that Analog
have in their tree which hasn't made it upstream yet.
To that end I'll want Lars to have a look at this... I've completely
lost track of where they are with this.
Perhaps Lars can give us a quick update?
If that was in place (or what I have in my head was true anyway),
it would look like the reverse of the triggered buffer input devices.
You'd be able to write to a software buffer and it would clock them
out as the trigger fires (here I think it would have to keep updating
the DHR whenever the trigger occurs).
Even if it's not there, we aren't necessarily looking at terribly big job
to implement it in the core and that would make this handling a lot more
'standard' and consistent.
Jonathan
> ---
> drivers/iio/dac/Kconfig | 3 +
> drivers/iio/dac/stm32-dac-core.h | 12 ++++
> drivers/iio/dac/stm32-dac.c | 124 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 136 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 7198648..786c38b 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -278,6 +278,9 @@ config STM32_DAC
> tristate "STMicroelectronics STM32 DAC"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
> depends on REGULATOR
> + select IIO_TRIGGERED_EVENT
> + select IIO_STM32_TIMER_TRIGGER
> + select MFD_STM32_TIMERS
> select STM32_DAC_CORE
> help
> Say yes here to build support for STMicroelectronics STM32 Digital
> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
> index d3099f7..3bf211c 100644
> --- a/drivers/iio/dac/stm32-dac-core.h
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -26,6 +26,7 @@
>
> /* STM32 DAC registers */
> #define STM32_DAC_CR 0x00
> +#define STM32_DAC_SWTRIGR 0x04
> #define STM32_DAC_DHR12R1 0x08
> #define STM32_DAC_DHR12R2 0x14
> #define STM32_DAC_DOR1 0x2C
> @@ -33,8 +34,19 @@
>
> /* STM32_DAC_CR bit fields */
> #define STM32_DAC_CR_EN1 BIT(0)
> +#define STM32H7_DAC_CR_TEN1 BIT(1)
> +#define STM32H7_DAC_CR_TSEL1_SHIFT 2
> +#define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
> +#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
> +#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
> #define STM32H7_DAC_CR_HFSEL BIT(15)
> #define STM32_DAC_CR_EN2 BIT(16)
> +#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
> +#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
> +
> +/* STM32_DAC_SWTRIGR bit fields */
> +#define STM32_DAC_SWTRIGR_SWTRIG1 BIT(0)
> +#define STM32_DAC_SWTRIGR_SWTRIG2 BIT(1)
>
> /**
> * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> index ee9711d..62e43e9 100644
> --- a/drivers/iio/dac/stm32-dac.c
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -23,6 +23,10 @@
> #include <linux/bitfield.h>
> #include <linux/delay.h>
> #include <linux/iio/iio.h>
> +#include <linux/iio/timer/stm32-timer-trigger.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_event.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> @@ -31,15 +35,112 @@
>
> #define STM32_DAC_CHANNEL_1 1
> #define STM32_DAC_CHANNEL_2 2
> +/* channel2 shift */
> +#define STM32_DAC_CHAN2_SHIFT 16
>
> /**
> * struct stm32_dac - private data of DAC driver
> * @common: reference to DAC common data
> + * @swtrig: Using software trigger
> */
> struct stm32_dac {
> struct stm32_dac_common *common;
> + bool swtrig;
> };
>
> +/**
> + * struct stm32_dac_trig_info - DAC trigger info
> + * @name: name of the trigger, corresponding to its source
> + * @tsel: trigger selection, value to be configured in DAC_CR.TSELx
> + */
> +struct stm32_dac_trig_info {
> + const char *name;
> + u32 tsel;
> +};
> +
> +static const struct stm32_dac_trig_info stm32h7_dac_trinfo[] = {
> + { "swtrig", 0 },
> + { TIM1_TRGO, 1 },
> + { TIM2_TRGO, 2 },
> + { TIM4_TRGO, 3 },
> + { TIM5_TRGO, 4 },
> + { TIM6_TRGO, 5 },
> + { TIM7_TRGO, 6 },
> + { TIM8_TRGO, 7 },
> + {},
> +};
> +
> +static irqreturn_t stm32_dac_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + int channel = indio_dev->channels[0].channel;
> +
> + /* Using software trigger? Then, trigger it now */
> + if (dac->swtrig) {
> + u32 swtrig;
> +
> + if (channel == STM32_DAC_CHANNEL_1)
> + swtrig = STM32_DAC_SWTRIGR_SWTRIG1;
> + else
> + swtrig = STM32_DAC_SWTRIGR_SWTRIG2;
> + regmap_update_bits(dac->common->regmap, STM32_DAC_SWTRIGR,
> + swtrig, swtrig);
> + }
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static unsigned int stm32_dac_get_trig_tsel(struct stm32_dac *dac,
> + struct iio_trigger *trig)
> +{
> + unsigned int i;
> +
> + /* skip 1st trigger that should be swtrig */
> + for (i = 1; stm32h7_dac_trinfo[i].name; i++) {
> + /*
> + * Checking both stm32 timer trigger type and trig name
> + * should be safe against arbitrary trigger names.
> + */
> + if (is_stm32_timer_trigger(trig) &&
> + !strcmp(stm32h7_dac_trinfo[i].name, trig->name)) {
> + return stm32h7_dac_trinfo[i].tsel;
> + }
> + }
> +
> + /* When no trigger has been found, default to software trigger */
> + dac->swtrig = true;
> +
> + return stm32h7_dac_trinfo[0].tsel;
> +}
> +
> +static int stm32_dac_set_trig(struct stm32_dac *dac, struct iio_trigger *trig,
> + int channel)
> +{
> + struct iio_dev *indio_dev = iio_priv_to_dev(dac);
> + u32 shift = channel == STM32_DAC_CHANNEL_1 ? 0 : STM32_DAC_CHAN2_SHIFT;
> + u32 val = 0, tsel;
> + u32 msk = (STM32H7_DAC_CR_TEN1 | STM32H7_DAC_CR_TSEL1) << shift;
> +
> + dac->swtrig = false;
> + if (trig) {
> + /* select & enable trigger (tsel / ten) */
> + tsel = stm32_dac_get_trig_tsel(dac, trig);
> + val = tsel << STM32H7_DAC_CR_TSEL1_SHIFT;
> + val = (val | STM32H7_DAC_CR_TEN1) << shift;
> + }
> +
> + if (trig)
> + dev_dbg(&indio_dev->dev, "enable trigger: %s\n", trig->name);
> + else
> + dev_dbg(&indio_dev->dev, "disable trigger\n");
> +
> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, msk, val);
> +}
> +
> static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
> {
> u32 en, val;
> @@ -63,9 +164,16 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
> STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
> int ret;
>
> + ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "Trigger setup failed\n");
> + return ret;
> + }
> +
> ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, en);
> if (ret < 0) {
> dev_err(&indio_dev->dev, "Enable failed\n");
> + stm32_dac_set_trig(dac, NULL, channel);
> return ret;
> }
>
> @@ -88,10 +196,12 @@ static int stm32_dac_disable(struct iio_dev *indio_dev, int channel)
> int ret;
>
> ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, 0);
> - if (ret)
> + if (ret) {
> dev_err(&indio_dev->dev, "Disable failed\n");
> + return ret;
> + }
>
> - return ret;
> + return stm32_dac_set_trig(dac, NULL, channel);
> }
>
> static int stm32_dac_get_value(struct stm32_dac *dac, int channel, int *val)
> @@ -258,10 +368,17 @@ static int stm32_dac_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - ret = iio_device_register(indio_dev);
> + ret = iio_triggered_event_setup(indio_dev, NULL,
> + stm32_dac_trigger_handler);
> if (ret)
> return ret;
>
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + iio_triggered_event_cleanup(indio_dev);
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -269,6 +386,7 @@ static int stm32_dac_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>
> + iio_triggered_event_cleanup(indio_dev);
> iio_device_unregister(indio_dev);
>
> return 0;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/5] drivers: watchdog: Add STM32 IWDG driver
From: Guenter Roeck @ 2017-04-02 11:48 UTC (permalink / raw)
To: Yannick Fertre, Wim Van Sebroeck, Rob Herring, Alexandre TORGUE,
Benjamin Gaignard, Maxime Coquelin
Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Philippe Cornu,
Gabriel FERNANDEZ, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490886926-20585-3-git-send-email-yannick.fertre-qxv4g6HH51o@public.gmane.org>
On 03/30/2017 08:15 AM, Yannick Fertre wrote:
> This patch adds IWDG (Independent WatchDoG) support for STM32 platform.
>
> Signed-off-by: Yannick FERTRE <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> ---
> drivers/watchdog/Kconfig | 12 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/stm32_iwdg.c | 260 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 273 insertions(+)
> create mode 100644 drivers/watchdog/stm32_iwdg.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 52a70ee..d014deb 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -744,6 +744,18 @@ config ZX2967_WATCHDOG
> To compile this driver as a module, choose M here: the
> module will be called zx2967_wdt.
>
> +config STM32_WATCHDOG
> + tristate "STM32 Independent WatchDoG (IWDG) support"
> + depends on ARCH_STM32
> + select WATCHDOG_CORE
> + default y
> + help
> + Say Y here to include support for the watchdog timer
> + in stm32 SoCs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called stm32_iwdg.
> +
> # AVR32 Architecture
>
> config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a2126e2..a35e423 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
> obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
> obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
> obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> +obj-$(CONFIG_STM32_WATCHDOG) += stm32_iwdg.o
>
> # AVR32 Architecture
> obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> new file mode 100644
> index 0000000..629c305
> --- /dev/null
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -0,0 +1,260 @@
> +/*
> + * Driver for STM32 Independent Watchdog
> + *
> + * Copyright (C) Yannick Fertre 2017
> + * Author: Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>
> + *
> + * This driver is based on tegra_wdt.c
> + *
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +/* watchdog trigger timeout, in seconds */
> +#define WDT_MIN_TIMEOUT 1
> +#define WDT_MAX_TIMEOUT 32
> +#define WDT_DEFAULT_TIMEOUT 30
> +
> +/* IWDG registers */
> +#define IWDG_KR 0x00 /* Key register */
> +#define IWDG_PR 0x04 /* Prescaler Register */
> +#define IWDG_RLR 0x08 /* ReLoad Register */
> +#define IWDG_SR 0x0C /* Status Register */
> +#define IWDG_WINR 0x10 /* Windows Register */
> +
> +/* IWDG_KR register bit mask */
> +#define KR_KEY_RELOAD 0xAAAA /* reload counter enable */
> +#define KR_KEY_ENABLE 0xCCCC /* peripheral enable */
> +#define KR_KEY_EWA 0x5555 /* write access enable */
> +#define KR_KEY_DWA 0x0000 /* write access disable */
> +
> +/* IWDG_PR register bit values */
> +#define PR_4 0x00 /* prescaler set to 4 */
> +#define PR_8 0x01 /* prescaler set to 8 */
> +#define PR_16 0x02 /* prescaler set to 16 */
> +#define PR_32 0x03 /* prescaler set to 32 */
> +#define PR_64 0x04 /* prescaler set to 64 */
> +#define PR_128 0x05 /* prescaler set to 128 */
> +#define PR_256 0x06 /* prescaler set to 256 */
> +
> +/* IWDG_RLR register values */
> +#define RLR_MAX 0xFFF /* max value supported by reload register */
> +
> +/* IWDG_SR register bit mask */
> +#define FLAG_PVU BIT(0) /* Watchdog prescaler value update */
> +#define FLAG_RVU BIT(1) /* Watchdog counter reload value update */
> +
> +/* set timeout to 100000 us */
> +#define TIMEOUT_US 100000
> +#define SLEEP_US 1000
> +
> +struct stm32_iwdg {
> + struct watchdog_device wdd;
> + void __iomem *regs;
> + struct clk *clk;
> + unsigned int rate;
> +};
> +
> +static inline u32 reg_read(void __iomem *base, u32 reg)
> +{
> + return readl_relaxed(base + reg);
> +}
> +
> +static inline void reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> + writel_relaxed(val, base + reg);
> +}
> +
> +static int stm32_iwdg_start(struct watchdog_device *wdd)
> +{
> + struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> + u32 val = FLAG_PVU | FLAG_RVU;
> + u32 reload;
> + int ret;
> +
> + dev_dbg(wdd->parent, "%s\n", __func__);
> +
> + /* enable watchdog */
> + reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA);
> + reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE);
> +
> + /* prescaler fixed to 256 */
> + reload = (wdd->timeout * wdt->rate) / 256;
> + if (reload > RLR_MAX + 1)
> + reload = RLR_MAX + 1;
> +
> + /* set prescaler & reload registers */
> + reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */
> + reg_write(wdt->regs, IWDG_RLR, reload - 1);
> +
> + /* wait for the registers to be updated (max 100ms) */
> + ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val,
> + !(val & (FLAG_PVU | FLAG_RVU)),
> + SLEEP_US, TIMEOUT_US);
> + if (ret) {
> + dev_err(wdd->parent,
> + "Fail to set prescaler or reload registers\n");
> + return ret;
> + }
> +
> + /* reload watchdog */
> + reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> + return 0;
> +}
> +
> +static int stm32_iwdg_ping(struct watchdog_device *wdd)
> +{
> + struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> +
> + dev_dbg(wdd->parent, "%s\n", __func__);
> +
> + /* reload watchdog */
> + reg_write(wdt->regs, IWDG_KR, KR_KEY_RELOAD);
> +
> + return 0;
> +}
> +
> +static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout);
> +
> + wdd->timeout = timeout;
> +
> + if (watchdog_active(wdd))
> + return stm32_iwdg_start(wdd);
> +
> + return 0;
> +}
> +
> +static const struct watchdog_info stm32_iwdg_info = {
> + .options = WDIOF_SETTIMEOUT |
> + WDIOF_MAGICCLOSE |
> + WDIOF_KEEPALIVEPING,
> + .identity = "STM32 Independent Watchdog",
> +};
> +
> +static struct watchdog_ops stm32_iwdg_ops = {
> + .owner = THIS_MODULE,
> + .start = stm32_iwdg_start,
> + .ping = stm32_iwdg_ping,
> + .set_timeout = stm32_iwdg_set_timeout,
> +};
> +
> +static int stm32_iwdg_probe(struct platform_device *pdev)
> +{
> + struct watchdog_device *wdd;
> + struct stm32_iwdg *wdt;
> + struct resource *res;
> + void __iomem *regs;
> + struct clk *clk;
> + int ret;
> +
> + /* This is the timer base. */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(regs)) {
> + dev_err(&pdev->dev, "Could not get resource\n");
> + return PTR_ERR(regs);
> + }
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Unable to get clock\n");
> + return PTR_ERR(clk);
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
> + return ret;
> + }
> +
> + /*
> + * Allocate our watchdog driver data, which has the
> + * struct watchdog_device nested within it.
> + */
> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> + if (!wdt) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + /* Initialize struct stm32_iwdg. */
> + wdt->regs = regs;
> + wdt->clk = clk;
> + wdt->rate = clk_get_rate(clk);
> +
> + /* Initialize struct watchdog_device. */
> + wdd = &wdt->wdd;
> + wdd->info = &stm32_iwdg_info;
> + wdd->ops = &stm32_iwdg_ops;
> + wdd->min_timeout = WDT_MIN_TIMEOUT;
> + wdd->max_timeout = WDT_MAX_TIMEOUT;
> + wdd->max_hw_heartbeat_ms = WDT_MAX_TIMEOUT;
This would have to be in milli-seconds. Also only provide max_timeout
or max_hw_heartbeat_ms, but not both.
> + wdd->parent = &pdev->dev;
> +
> + watchdog_set_drvdata(wdd, wdt);
> + watchdog_set_nowayout(wdd, true);
> +
> + ret = watchdog_init_timeout(wdd, WDT_DEFAULT_TIMEOUT, &pdev->dev);
Timeout parameter should be 0 to give the core a chance to pick up a timeout
from fdt if specified. Otherwise the call is quite useless.
> + if (ret)
> + dev_warn(&pdev->dev,
> + "unable to set timeout value, using default\n");
> +
> + ret = watchdog_register_device(wdd);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "failed to register watchdog device\n");
> + goto err;
> + }
> +
> + platform_set_drvdata(pdev, wdt);
> +
> + return 0;
> +err:
> + clk_disable_unprepare(clk);
> +
> + return ret;
> +}
> +
> +static int stm32_iwdg_remove(struct platform_device *pdev)
> +{
> + struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
> +
> + watchdog_unregister_device(&wdt->wdd);
> + clk_disable_unprepare(wdt->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_iwdg_of_match[] = {
> + { .compatible = "st,stm32-iwdg" },
> + { /* end node */ }
> +};
> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
> +
> +static struct platform_driver stm32_iwdg_driver = {
> + .probe = stm32_iwdg_probe,
> + .remove = stm32_iwdg_remove,
> + .driver = {
> + .name = "iwdg",
> + .of_match_table = stm32_iwdg_of_match,
> + },
> +};
> +module_platform_driver(stm32_iwdg_driver);
> +
> +MODULE_AUTHOR("Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 Independent Watchdog Driver");
> +MODULE_LICENSE("GPL v2");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/4] iio: dac: stm32: add support for waveform generator
From: Jonathan Cameron @ 2017-04-02 12:19 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, amelie.delaunay-qxv4g6HH51o,
Hennerich, Michael
In-Reply-To: <1490960707-22422-5-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 31/03/17 12:45, Fabrice Gasnier wrote:
> STM32 DAC has built-in noise or triangle waveform generator.
> Waveform generator requires trigger to be configured.
> - "wave" extended attribute selects noise or triangle.
> - "mamp" extended attribute selects either LFSR (linear feedback
> shift register) mask for noise waveform, OR triangle amplitude.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Looks like AN3126 is the relevant doc.
(a quick note from this relevant to earlier patches- doc says
1-3 channels - perhaps build that from the start with that
possibility in mind).
As you probably know, this wanders into a large chunk of 'poorly'
defined ABI within IIO as it stands.
Note there are a number of waveform generators still in staging.
Not a lot of movement on getting them out of staging unfortunately
(so far!)
However, let us keep those drivers in mind as we work on ABI and
I definitely want some input from someone at Analog.
Lars, who is best for this? I see at least some of these were
originally Michael's work.
They do have partial docs under
drivers/staging/iio/Documentation/sysfs-bus-iio-dds
I'll highlight thoughts from there as I look through this...
> ---
> Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 | 32 ++++++
> drivers/iio/dac/stm32-dac.c | 124 ++++++++++++++++++++++
> 2 files changed, 156 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> new file mode 100644
> index 0000000..c2432e1
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
> @@ -0,0 +1,32 @@
> +What: /sys/bus/iio/devices/iio:deviceX/wave
> +What: /sys/bus/iio/devices/iio:deviceX/wave_available
Needs to be channel associated. Whilst in your case you have basically
a pair of single channel devices, in more general case, it's not usual
to have multiple parallel waveform generators clocked together.
Old ABI is:
What: /sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype etc
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
> +Description:
> + List and/or select waveform generation provided by STM32 DAC:
> + - "none": (default) means normal DAC operations
none kind of hints at nothing coming out. Perhaps 'flat' would be closer?
i.e. only changes when someone tells it to.
> + - "noise": select noise waveform
> + - "triangle": select triangle waveform
> + Note: when waveform generator is used, writing _raw sysfs entry
> + adds a DC offset to generated waveform. Reading it reports
> + current output value.
Interesting. This gets fiddly but one option would be to describe the whole
device as a dds.
Then we have flat type above, combined with an _offset.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/mamp
> +What: /sys/bus/iio/devices/iio:deviceX/mamp_available
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
> +Description:
> + List and select mask/amplitude used for noise/triangle waveform
> + generator, which are:
> + - "0": unmask bit 0 of LFSR / triangle amplitude equal to 1
> + - "1": unmask bit [1:0] of LFSR / triangle amplitude equal to 3
> + - "2": unmask bit [2:0] of LFSR / triangle amplitude equal to 7
> + - "3": unmask bit [3:0] of LFSR / triangle amplitude equal to 15
> + - "4": unmask bit [4:0] of LFSR / triangle amplitude equal to 31
> + - "5": unmask bit [5:0] of LFSR / triangle amplitude equal to 63
> + - "6": unmask bit [6:0] of LFSR / triangle amplitude equal to 127
> + - "7": unmask bit [7:0] of LFSR / triangle amplitude equal to 255
> + - "8": unmask bit [8:0] of LFSR / triangle amplitude equal to 511
> + - "9": unmask bit [9:0] of LFSR / triangle amplitude equal to 1023
> + - "10": unmask bit [10:0] of LFSR / triangle amplitude equal to 2047
> + - "11": unmask bit [11:0] of LFSR / triangle amplitude equal to 4095
I don't fully understand what is going on here - so I'm guessing somewhat.
Let us try describing these generically. If we define standard 'forms' of each
waveform type. Say a 0 to 1 V peak to peak, then we could use _scale to control
this nicely.
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> index 62e43e9..d7dda78 100644
> --- a/drivers/iio/dac/stm32-dac.c
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -41,10 +41,14 @@
> /**
> * struct stm32_dac - private data of DAC driver
> * @common: reference to DAC common data
> + * @wave: waveform generator
> + * @mamp: waveform mask/amplitude
> * @swtrig: Using software trigger
> */
> struct stm32_dac {
> struct stm32_dac_common *common;
> + u32 wave;
> + u32 mamp;
> bool swtrig;
> };
>
> @@ -157,6 +161,24 @@ static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
> return !!en;
> }
>
> +static int stm32_dac_wavegen(struct stm32_dac *dac, int channel)
> +{
> + struct regmap *regmap = dac->common->regmap;
> + u32 mask, val;
> +
> + if (channel == STM32_DAC_CHANNEL_1) {
> + val = FIELD_PREP(STM32_DAC_CR_WAVE1, dac->wave) |
> + FIELD_PREP(STM32_DAC_CR_MAMP1, dac->mamp);
> + mask = STM32_DAC_CR_WAVE1 | STM32_DAC_CR_MAMP1;
> + } else {
> + val = FIELD_PREP(STM32_DAC_CR_WAVE2, dac->wave) |
> + FIELD_PREP(STM32_DAC_CR_MAMP2, dac->mamp);
> + mask = STM32_DAC_CR_WAVE2 | STM32_DAC_CR_MAMP2;
> + }
> +
> + return regmap_update_bits(regmap, STM32_DAC_CR, mask, val);
> +}
> +
> static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
> {
> struct stm32_dac *dac = iio_priv(indio_dev);
> @@ -164,6 +186,17 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
> STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
> int ret;
>
> + if (dac->wave && !indio_dev->trig) {
> + dev_err(&indio_dev->dev, "Wavegen requires a trigger\n");
> + return -EINVAL;
> + }
> +
> + ret = stm32_dac_wavegen(dac, channel);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "Wavegen setup failed\n");
> + return ret;
> + }
> +
> ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
> if (ret < 0) {
> dev_err(&indio_dev->dev, "Trigger setup failed\n");
> @@ -291,6 +324,96 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
> .driver_module = THIS_MODULE,
> };
>
> +/* waveform generator wave selection */
> +static const char * const stm32_dac_wave_desc[] = {
> + "none",
> + "noise",
> + "triangle",
> +};
> +
> +static int stm32_dac_set_wave(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int type)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + if (stm32_dac_is_enabled(dac, chan->channel))
> + return -EBUSY;
> + dac->wave = type;
> +
> + return 0;
> +}
> +
> +static int stm32_dac_get_wave(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + return dac->wave;
> +}
> +
> +static const struct iio_enum stm32_dac_wave_enum = {
> + .items = stm32_dac_wave_desc,
> + .num_items = ARRAY_SIZE(stm32_dac_wave_desc),
> + .get = stm32_dac_get_wave,
> + .set = stm32_dac_set_wave,
> +};
> +
> +/*
> + * waveform generator mask/amplitude selection:
> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, [1:0]...)
> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
> + */
> +static const char * const stm32_dac_mamp_desc[] = {
> + "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11",
> +};
> +
> +static int stm32_dac_set_mamp(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int type)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + if (stm32_dac_is_enabled(dac, chan->channel))
> + return -EBUSY;
> + dac->mamp = type;
> +
> + return 0;
> +}
> +
> +static int stm32_dac_get_mamp(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> +
> + return dac->mamp;
> +}
> +
> +static const struct iio_enum stm32_dac_mamp_enum = {
> + .items = stm32_dac_mamp_desc,
> + .num_items = ARRAY_SIZE(stm32_dac_mamp_desc),
> + .get = stm32_dac_get_mamp,
> + .set = stm32_dac_set_mamp,
> +};
> +
> +static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
> + IIO_ENUM("wave", IIO_SHARED_BY_ALL, &stm32_dac_wave_enum),
> + {
> + .name = "wave_available",
> + .shared = IIO_SHARED_BY_ALL,
> + .read = iio_enum_available_read,
> + .private = (uintptr_t)&stm32_dac_wave_enum,
> + },
> + IIO_ENUM("mamp", IIO_SHARED_BY_ALL, &stm32_dac_mamp_enum),
> + {
> + .name = "mamp_available",
> + .shared = IIO_SHARED_BY_ALL,
> + .read = iio_enum_available_read,
> + .private = (uintptr_t)&stm32_dac_mamp_enum,
> + },
> + {},
> +};
> +
> #define STM32_DAC_CHANNEL(chan, name) { \
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> @@ -306,6 +429,7 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
> .storagebits = 16, \
> }, \
> .datasheet_name = name, \
> + .ext_info = stm32_dac_ext_info \
> }
>
> static const struct iio_chan_spec stm32_dac_channels[] = {
>
^ permalink raw reply
* Re: [PATCH 3/4] iio: dac: stm32: add support for trigger events
From: Jonathan Cameron @ 2017-04-02 12:21 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, amelie.delaunay-qxv4g6HH51o
In-Reply-To: <eb4d6292-5ae6-c52d-eeb7-2f14c3a4abef-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 02/04/17 12:45, Jonathan Cameron wrote:
> On 31/03/17 12:45, Fabrice Gasnier wrote:
>> STM32 DAC supports triggers to synchronize conversions. When trigger
>> occurs, data is transferred from DHR (data holding register) to DOR
>> (data output register) so output voltage is updated.
>> Both hardware and software triggers are supported.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
> Hmm. This is a somewhat different use of triggered event from normal...
>
> What you have here is rather closer to the output buffers stuff that Analog
> have in their tree which hasn't made it upstream yet.
> To that end I'll want Lars to have a look at this... I've completely
> lost track of where they are with this.
> Perhaps Lars can give us a quick update?
>
> If that was in place (or what I have in my head was true anyway),
> it would look like the reverse of the triggered buffer input devices.
> You'd be able to write to a software buffer and it would clock them
> out as the trigger fires (here I think it would have to keep updating
> the DHR whenever the trigger occurs).
>
> Even if it's not there, we aren't necessarily looking at terribly big job
> to implement it in the core and that would make this handling a lot more
> 'standard' and consistent.
Having tracked down some limited docs (AN3126 - Audio and waveform
generation using the DAC in STM32 microcontrollers) the fact this
can also be driven by DMA definitely argues in favour of working with
Analog on getting the output buffers support upstream.
*crosses fingers people have the time!*
>
> Jonathan
>
>> ---
>> drivers/iio/dac/Kconfig | 3 +
>> drivers/iio/dac/stm32-dac-core.h | 12 ++++
>> drivers/iio/dac/stm32-dac.c | 124 ++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 136 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>> index 7198648..786c38b 100644
>> --- a/drivers/iio/dac/Kconfig
>> +++ b/drivers/iio/dac/Kconfig
>> @@ -278,6 +278,9 @@ config STM32_DAC
>> tristate "STMicroelectronics STM32 DAC"
>> depends on (ARCH_STM32 && OF) || COMPILE_TEST
>> depends on REGULATOR
>> + select IIO_TRIGGERED_EVENT
>> + select IIO_STM32_TIMER_TRIGGER
>> + select MFD_STM32_TIMERS
>> select STM32_DAC_CORE
>> help
>> Say yes here to build support for STMicroelectronics STM32 Digital
>> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
>> index d3099f7..3bf211c 100644
>> --- a/drivers/iio/dac/stm32-dac-core.h
>> +++ b/drivers/iio/dac/stm32-dac-core.h
>> @@ -26,6 +26,7 @@
>>
>> /* STM32 DAC registers */
>> #define STM32_DAC_CR 0x00
>> +#define STM32_DAC_SWTRIGR 0x04
>> #define STM32_DAC_DHR12R1 0x08
>> #define STM32_DAC_DHR12R2 0x14
>> #define STM32_DAC_DOR1 0x2C
>> @@ -33,8 +34,19 @@
>>
>> /* STM32_DAC_CR bit fields */
>> #define STM32_DAC_CR_EN1 BIT(0)
>> +#define STM32H7_DAC_CR_TEN1 BIT(1)
>> +#define STM32H7_DAC_CR_TSEL1_SHIFT 2
>> +#define STM32H7_DAC_CR_TSEL1 GENMASK(5, 2)
>> +#define STM32_DAC_CR_WAVE1 GENMASK(7, 6)
>> +#define STM32_DAC_CR_MAMP1 GENMASK(11, 8)
>> #define STM32H7_DAC_CR_HFSEL BIT(15)
>> #define STM32_DAC_CR_EN2 BIT(16)
>> +#define STM32_DAC_CR_WAVE2 GENMASK(23, 22)
>> +#define STM32_DAC_CR_MAMP2 GENMASK(27, 24)
>> +
>> +/* STM32_DAC_SWTRIGR bit fields */
>> +#define STM32_DAC_SWTRIGR_SWTRIG1 BIT(0)
>> +#define STM32_DAC_SWTRIGR_SWTRIG2 BIT(1)
>>
>> /**
>> * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>> index ee9711d..62e43e9 100644
>> --- a/drivers/iio/dac/stm32-dac.c
>> +++ b/drivers/iio/dac/stm32-dac.c
>> @@ -23,6 +23,10 @@
>> #include <linux/bitfield.h>
>> #include <linux/delay.h>
>> #include <linux/iio/iio.h>
>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_event.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> @@ -31,15 +35,112 @@
>>
>> #define STM32_DAC_CHANNEL_1 1
>> #define STM32_DAC_CHANNEL_2 2
>> +/* channel2 shift */
>> +#define STM32_DAC_CHAN2_SHIFT 16
>>
>> /**
>> * struct stm32_dac - private data of DAC driver
>> * @common: reference to DAC common data
>> + * @swtrig: Using software trigger
>> */
>> struct stm32_dac {
>> struct stm32_dac_common *common;
>> + bool swtrig;
>> };
>>
>> +/**
>> + * struct stm32_dac_trig_info - DAC trigger info
>> + * @name: name of the trigger, corresponding to its source
>> + * @tsel: trigger selection, value to be configured in DAC_CR.TSELx
>> + */
>> +struct stm32_dac_trig_info {
>> + const char *name;
>> + u32 tsel;
>> +};
>> +
>> +static const struct stm32_dac_trig_info stm32h7_dac_trinfo[] = {
>> + { "swtrig", 0 },
>> + { TIM1_TRGO, 1 },
>> + { TIM2_TRGO, 2 },
>> + { TIM4_TRGO, 3 },
>> + { TIM5_TRGO, 4 },
>> + { TIM6_TRGO, 5 },
>> + { TIM7_TRGO, 6 },
>> + { TIM8_TRGO, 7 },
>> + {},
>> +};
>> +
>> +static irqreturn_t stm32_dac_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct stm32_dac *dac = iio_priv(indio_dev);
>> + int channel = indio_dev->channels[0].channel;
>> +
>> + /* Using software trigger? Then, trigger it now */
>> + if (dac->swtrig) {
>> + u32 swtrig;
>> +
>> + if (channel == STM32_DAC_CHANNEL_1)
>> + swtrig = STM32_DAC_SWTRIGR_SWTRIG1;
>> + else
>> + swtrig = STM32_DAC_SWTRIGR_SWTRIG2;
>> + regmap_update_bits(dac->common->regmap, STM32_DAC_SWTRIGR,
>> + swtrig, swtrig);
>> + }
>> +
>> + iio_trigger_notify_done(indio_dev->trig);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static unsigned int stm32_dac_get_trig_tsel(struct stm32_dac *dac,
>> + struct iio_trigger *trig)
>> +{
>> + unsigned int i;
>> +
>> + /* skip 1st trigger that should be swtrig */
>> + for (i = 1; stm32h7_dac_trinfo[i].name; i++) {
>> + /*
>> + * Checking both stm32 timer trigger type and trig name
>> + * should be safe against arbitrary trigger names.
>> + */
>> + if (is_stm32_timer_trigger(trig) &&
>> + !strcmp(stm32h7_dac_trinfo[i].name, trig->name)) {
>> + return stm32h7_dac_trinfo[i].tsel;
>> + }
>> + }
>> +
>> + /* When no trigger has been found, default to software trigger */
>> + dac->swtrig = true;
>> +
>> + return stm32h7_dac_trinfo[0].tsel;
>> +}
>> +
>> +static int stm32_dac_set_trig(struct stm32_dac *dac, struct iio_trigger *trig,
>> + int channel)
>> +{
>> + struct iio_dev *indio_dev = iio_priv_to_dev(dac);
>> + u32 shift = channel == STM32_DAC_CHANNEL_1 ? 0 : STM32_DAC_CHAN2_SHIFT;
>> + u32 val = 0, tsel;
>> + u32 msk = (STM32H7_DAC_CR_TEN1 | STM32H7_DAC_CR_TSEL1) << shift;
>> +
>> + dac->swtrig = false;
>> + if (trig) {
>> + /* select & enable trigger (tsel / ten) */
>> + tsel = stm32_dac_get_trig_tsel(dac, trig);
>> + val = tsel << STM32H7_DAC_CR_TSEL1_SHIFT;
>> + val = (val | STM32H7_DAC_CR_TEN1) << shift;
>> + }
>> +
>> + if (trig)
>> + dev_dbg(&indio_dev->dev, "enable trigger: %s\n", trig->name);
>> + else
>> + dev_dbg(&indio_dev->dev, "disable trigger\n");
>> +
>> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, msk, val);
>> +}
>> +
>> static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
>> {
>> u32 en, val;
>> @@ -63,9 +164,16 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
>> STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
>> int ret;
>>
>> + ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
>> + if (ret < 0) {
>> + dev_err(&indio_dev->dev, "Trigger setup failed\n");
>> + return ret;
>> + }
>> +
>> ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, en);
>> if (ret < 0) {
>> dev_err(&indio_dev->dev, "Enable failed\n");
>> + stm32_dac_set_trig(dac, NULL, channel);
>> return ret;
>> }
>>
>> @@ -88,10 +196,12 @@ static int stm32_dac_disable(struct iio_dev *indio_dev, int channel)
>> int ret;
>>
>> ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, 0);
>> - if (ret)
>> + if (ret) {
>> dev_err(&indio_dev->dev, "Disable failed\n");
>> + return ret;
>> + }
>>
>> - return ret;
>> + return stm32_dac_set_trig(dac, NULL, channel);
>> }
>>
>> static int stm32_dac_get_value(struct stm32_dac *dac, int channel, int *val)
>> @@ -258,10 +368,17 @@ static int stm32_dac_probe(struct platform_device *pdev)
>> if (ret < 0)
>> return ret;
>>
>> - ret = iio_device_register(indio_dev);
>> + ret = iio_triggered_event_setup(indio_dev, NULL,
>> + stm32_dac_trigger_handler);
>> if (ret)
>> return ret;
>>
>> + ret = iio_device_register(indio_dev);
>> + if (ret) {
>> + iio_triggered_event_cleanup(indio_dev);
>> + return ret;
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -269,6 +386,7 @@ static int stm32_dac_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>
>> + iio_triggered_event_cleanup(indio_dev);
>> iio_device_unregister(indio_dev);
>>
>> return 0;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 1/2] leds: Add driver for Qualcomm LPG
From: Jacek Anaszewski @ 2017-04-02 12:54 UTC (permalink / raw)
To: Pavel Machek, Bjorn Andersson
Cc: Rob Herring, Richard Purdie, linux-kernel, linux-leds,
linux-arm-msm, Mark Rutland, devicetree
In-Reply-To: <bf999f34-4509-6f0b-602c-f82d50a18e97@gmail.com>
On 03/31/2017 11:28 AM, Jacek Anaszewski wrote:
> Hi Bjorn and Pavel,
>
> On 03/30/2017 09:43 AM, Pavel Machek wrote:
>> Hi!
>>
>>>>> There is a binding for ti,lp55xx, but there's nothing I can reuse from
>>>>> that binding...because it's completely different hardware.
>>>>
>>>> Agreed, if you drop the pattern stuff from the binding, at least for now.
>>>
>>> I do not have a strong preference to expose these knobs in devicetree
>>> and I do fear that finding some common "pattern" bindings that suits
>>> everyone will be very difficult.
>>>
>>> So I'll drop them from the binding for now.
>>
>> Ok.
>>
>>>> If you want driver merged quickly, I believe the best way would be to
>>>> leave out pattern support for now. We can merge the basic driver
>>>> easily to 4.12.
>>>>
>>>
>>> I'm not that much in a hurry and would rather see that we resolve any
>>> outstanding issues with the implementation of the pattern handling.
>>
>> Ok, good.
>>
>>> But regardless of this we still have the problem that the typical
>>> Qualcomm PMIC has 8 LPG-blocks and any triple could be driving a
>>> RGB-LED. So we would have to create some sort of in-driver-wrapper
>>> around any three instances exposing them as a single LED to the user.
>>
>> Yes, I believe we should do the wrapping. In N900 case,
>>
>>> I rather expose the individual channels and make sure that when we
>>> trigger a blink operation or enable a pattern (i.e. the two operations
>>> that do require synchronization) we will perform that synchronization
>>> under the hood.
>>
>> First, we need a way to tell userspace which LEDs are synchronized,
>> because otherwise it will be confusing.
>
> There is one year old discussion [0] about the possible approaches
> to RGB sub-LEDs synchronization problem and patterns in general.
> My last message with API design proposal has been left unanswered.
>
> Probably we continue that discussion here.
>
> Generally Bjorn's drivers touch two yet to be addressed issues:
> - RGB LED support
> - Generic support for patterns
>
> It is likely that both issues can be solved by utilizing trigger
> mechanism. The possible solution to the problem Bjorn tried to
> address with /sys/class/leds/<led>/pattern comma separated list
> could be a trigger with adjustable number of pattern intervals.
>
> The trigger once activated would create a directory with the
> number of files corresponding to the number of requested intervals,
> and then user could write an interval value by writing it to the
> corresponding file. Somehow related approach has been implemented
> for USB port LED trigger:
>
> 0f247626cbbf ('usb: core: Introduce a USB port LED trigger")
>
> In both RGB and pattern approaches we should assess
> if it is acceptable to provide a pattern for trigger name,
> e.g. blink-pattern-{num_intervals}.
Actually we could achieve the goal by listing all available pattern
configurations for given LED class device, so in case of Qualcomm LPG
driver we could have transition-pattern-1 to transition-pattern-15
listed after executing "cat trigger".
In case of RGB trigger we would have qcom-rgb among other triggers
listed in a result of executing "cat trigger". Then qcom-rgb-1 LED class
device could appear in /sys/class/leds, which would expose files
red-led-name, green-led-name and blue-led-name. Once all files are
initialized with appropriate LED class device names the RGB brightness
could be updated synchronously in all involved LEDs by executing
e.g. "echo 1 > update_color". Of course all LEDs that set qcom-rgb
trigger would have to avoid changing device state on write to
their brightness file.
I wonder if I'm not missing some vital constraints here that could
make this design unfeasible.
Best regards,
Jacek Anaszewski
> If so, then "echo transition-pattern-15" would create a directory
> e.g. transition_intervals with files interval_0 to interval_14,
> that could be adjusted by userspace.
>
>> Second, there are more issues than just patterns with the RGB
>> LED. Most important is ability to set particular colors. You want to
>> set the RGB LED to "white", but that does not mean you can set
>> red=green=blue=1.0. You want color to look the same on LCD and on the
>> LED, which means coefficients for white and some kind of function for
>> brightness-to-PWM conversion.
>
> Shouldn't we leave that entirely to the userspace? Can we come up
> with coefficients that will guarantee the same result on all existing
> LCD devices?
>
>>>> Incremental patches sound like a good idea, yes.
>>>>
>>>> I'd say that testing with actual RGB LED is not a requirement... as
>>>> long as we design reasonable interface where the synchronizaction will
>>>> be easy.
>>>>
>>>
>>> As this relates to the board layout (which LPG-channels are hooked to a
>>> RGB) I think it makes sense to expose a mechanism in devicetree to
>>> indicate which channels should have their pattern/blink synchronized.
>>>
>>> We should be able to extend the LUT (the hardware that actually
>>> implements the pattern-walker logic) with a DT-property like:
>>>
>>> qcom,synchronize-group-0 = <1, 2, 3>;
>>> qcom,synchronize-group-1 = <5, 6, 7>;
>>>
>>> And whenever we configure a pattern involving one of the affected LEDs
>>> from a group we start all of them.
>>
>> Yes we need some kind of grouping.
>>
>> Additional complexity in the N900 case... groups can actually be
>> configured at run time. Original Maemo used that ability to group 6
>> keyboard backlight leds, and then run pattern on them. OTOH... I don't
>> think we _need_ to support that functionality.
>>
>> Best regards,
>> Pavel
>>
>
> [0] https://lkml.org/lkml/2016/4/18/179
>
^ permalink raw reply
* Re: [PATCH 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Andy Shevchenko @ 2017-04-02 13:07 UTC (permalink / raw)
To: Joel Stanley
Cc: Greg Kroah-Hartman, Jiri Slaby, Mark Rutland, Rob Herring,
Jeremy Kerr, linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree,
Benjamin Herrenschmidt
In-Reply-To: <20170328054458.29341-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
On Tue, Mar 28, 2017 at 8:44 AM, Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org> wrote:
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
> Documentation/devicetree/bindings/serial/8250.txt | 2 +
> drivers/tty/serial/Kconfig | 10 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/aspeed-vuart.c | 335 ++++++++++++++++++++++
And why it's not under 8250 folder?
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250/8250.h"
> +
> +#define AST_VUART_GCRA 0x20
> +#define AST_VUART_GCRA_VUART_EN 0x01
> +#define AST_VUART_GCRA_HOST_TX_DISCARD 0x20
BIT(x) ?
> +#define AST_VUART_GCRB 0x24
> +#define AST_VUART_GCRB_HOST_SIRQ_MASK 0xf0
GENMASK() ?
> +#define AST_VUART_GCRB_HOST_SIRQ_SHIFT 4
> +#define AST_VUART_ADDRL 0x28
> +#define AST_VUART_ADDRH 0x2c
> +
> +struct ast_vuart {
> + struct platform_device *pdev;
Ususally there is no need to keep pointer to sturct platform_device,
rahter we need struct device *dev.
> + void __iomem *regs;
> + struct clk *clk;
> + int line;
> +};
> +
> +static ssize_t ast_vuart_show_addr(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + u16 addr;
> +
> + addr = (readb(vuart->regs + AST_VUART_ADDRH) << 8) |
> + (readb(vuart->regs + AST_VUART_ADDRL));
It looks like you have register shift 2 bits and byte accessors. We
have some helpers for that (serial_in() / serial_out() or alike).
> +
> + return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t ast_vuart_set_addr(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + writeb((val >> 8) & 0xff, vuart->regs + AST_VUART_ADDRH);
> + writeb((val >> 0) & 0xff, vuart->regs + AST_VUART_ADDRL);
Useless & 0xff.
> +
> + return count;
> +}
> +}
> +
> +
Extra empty line.
> +/**
If you are going to use kernel doc do it in accordance with howto.
> + * The device tree parsing code here is heavily based on that of the of_serial
> + * driver, but we have a few core differences, as we need to use our own
> + * ioremapping for extra register support
> + */
> +static int ast_vuart_probe(struct platform_device *pdev)
> +{
> + struct uart_8250_port port;
> + struct resource resource;
> + struct ast_vuart *vuart;
> + struct device_node *np;
> + u32 clk, prop;
> + int rc;
> +
> + np = pdev->dev.of_node;
And if np == NULL?
> +
> + vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> + if (!vuart)
> + return -ENOMEM;
> +
> + vuart->pdev = pdev;
> + rc = of_address_to_resource(np, 0, &resource);
> + if (rc) {
> + dev_warn(&pdev->dev, "invalid address\n");
> + return rc;
> + }
> +
> + /* create our own mapping for VUART-specific registers */
> + vuart->regs = devm_ioremap_resource(&pdev->dev, &resource);
> + if (IS_ERR(vuart->regs)) {
> + dev_warn(&pdev->dev, "failed to map registers\n");
> + return PTR_ERR(vuart->regs);
> + }
Can you use platform_get-resource() + devm_ioremap_resource() ?
If no, why?
> +
> + memset(&port, 0, sizeof(port));
> + port.port.private_data = vuart;
> + port.port.membase = vuart->regs;
> + port.port.mapbase = resource.start;
> + port.port.mapsize = resource_size(&resource);
> + port.port.startup = ast_vuart_startup;
> + port.port.shutdown = ast_vuart_shutdown;
> +
> + if (of_property_read_u32(np, "clock-frequency", &clk)) {
> + vuart->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(vuart->clk)) {
> + dev_warn(&pdev->dev,
> + "clk or clock-frequency not defined\n");
> + return PTR_ERR(vuart->clk);
> + }
> +
> + rc = clk_prepare_enable(vuart->clk);
> + if (rc < 0)
> + return rc;
> +
> + clk = clk_get_rate(vuart->clk);
> + }
> +
> + /* If current-speed was set, then try not to change it. */
> + if (of_property_read_u32(np, "current-speed", &prop) == 0)
> + port.port.custom_divisor = clk / (16 * prop);
> +
> + /* Check for shifted address mapping */
> + if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> + port.port.mapbase += prop;
> +
> + /* Check for registers offset within the devices address range */
> + if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> + port.port.regshift = prop;
> +
> + /* Check for fifo size */
> + if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + port.port.fifosize = prop;
Perhaps you need other way around, check for error and supply a
default in such case.
> +
> + /* Check for a fixed line number */
> + rc = of_alias_get_id(np, "serial");
> + if (rc >= 0)
> + port.port.line = rc;
> +
> + port.port.irq = irq_of_parse_and_map(np, 0);
> + port.port.irqflags = IRQF_SHARED;
This is set by core. You already supplied correct flag for that below.
> + port.port.iotype = UPIO_MEM;
> + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
You hide an error code from of_property_read_u32() here. Why?
And if there is an error you are continuing with what? 0?
> + switch (prop) {
> + case 1:
> + port.port.iotype = UPIO_MEM;
> + break;
> + case 4:
> + port.port.iotype = of_device_is_big_endian(np) ?
> + UPIO_MEM32BE : UPIO_MEM32;
> + break;
> + default:
> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> + prop);
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
> + }
> +
> + port.port.type = PORT_16550A;
> + port.port.uartclk = clk;
> + port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> + | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> + if (of_find_property(np, "no-loopback-test", NULL))
> + port.port.flags |= UPF_SKIP_TEST;
You perhaps meant _read_bool() for sake of consistency.
> +
> + port.port.dev = &pdev->dev;
> +
> + if (port.port.fifosize)
> + port.capabilities = UART_CAP_FIFO;
> +
> + if (of_property_read_bool(pdev->dev.of_node,
> + "auto-flow-control"))
One line?
> + port.capabilities |= UART_CAP_AFE;
> +
> + rc = serial8250_register_8250_port(&port);
> + if (rc < 0)
> + goto err_clk_disable;
> +
> +
> + vuart->line = rc;
> + ast_vuart_set_enabled(vuart, true);
> + ast_vuart_set_host_tx_discard(vuart, true);
> + platform_set_drvdata(pdev, vuart);
> +
> + /* extra sysfs control */
> + rc = device_create_file(&pdev->dev, &dev_attr_lpc_address);
> + if (rc)
> + dev_warn(&pdev->dev, "can't create lpc_address file\n");
> + rc = device_create_file(&pdev->dev, &dev_attr_sirq);
> + if (rc)
> + dev_warn(&pdev->dev, "can't create sirq file\n");
> +
> + return 0;
> +
> +err_clk_disable:
> + if (vuart->clk)
> + clk_disable_unprepare(vuart->clk);
> +
> + irq_dispose_mapping(port.port.irq);
> + return rc;
> +}
> +
> +static int ast_vuart_remove(struct platform_device *pdev)
> +{
> + struct ast_vuart *vuart = platform_get_drvdata(pdev);
> +
> + ast_vuart_set_enabled(vuart, false);
> +
> + if (vuart->clk)
> + clk_disable_unprepare(vuart->clk);
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenk
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC PATCH v2 0/4] IIO-based thermal sensor driver for Allwinner H3 SoC
From: Icenowy Zheng @ 2017-04-02 13:33 UTC (permalink / raw)
To: Lee Jones, Maxime Ripard, Chen-Yu Tsai, Jonathan Cameron,
Quentin Schulz, Zhang Rui
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
Allwiner H3 SoC has a thermal sensor, which is a large refactored version of
the old Allwinner "GPADC" (although it have already only thermal part left
in A33).
This patch tried to add support for the sensor in H3 based on the A33
thermal sensor patchset by Quentin Schulz at [1].
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/495505.html
Icenowy Zheng (4):
dt-bindings: update the Allwinner GPADC device tree binding for H3
iio: adc: sun4i-gpadc-iio: rename A23/A33-specified registers to
contain A23
iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
ARM: sun8i: h3: add support for the thermal sensor in H3
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 23 ++-
arch/arm/boot/dts/sun8i-h3.dtsi | 26 +++
drivers/iio/adc/sun4i-gpadc-iio.c | 215 +++++++++++++++++----
include/linux/mfd/sun4i-gpadc.h | 36 +++-
4 files changed, 254 insertions(+), 46 deletions(-)
--
2.12.2
^ permalink raw reply
* [RFC PATCH v2 1/4] dt-bindings: update the Allwinner GPADC device tree binding for H3
From: Icenowy Zheng @ 2017-04-02 13:33 UTC (permalink / raw)
To: Lee Jones, Maxime Ripard, Chen-Yu Tsai, Jonathan Cameron,
Quentin Schulz, Zhang Rui
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170402133304.56824-1-icenowy-h8G6r0blFSE@public.gmane.org>
Allwinner H3 features a thermal sensor like the one in A33, but has its
register re-arranged, the clock divider moved to CCU (originally the
clock divider is in ADC) and added a pair of bus clock and reset.
Update the binding document to cover H3.
Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
.../devicetree/bindings/mfd/sun4i-gpadc.txt | 23 ++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index badff3611a98..7753133ca0ff 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -4,12 +4,20 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
and sometimes as a touchscreen controller.
Required properties:
- - compatible: "allwinner,sun8i-a33-ths",
+ - compatible: must contain one of the following compatibles:
+ - "allwinner,sun8i-a33-ths"
+ - "allwinner,sun8i-h3-ths"
- reg: mmio address range of the chip,
- #thermal-sensor-cells: shall be 0,
- #io-channel-cells: shall be 0,
-Example:
+Required properties for the following compatibles:
+ - "allwinner,sun8i-h3-ths"
+ - clocks: the bus clock and the input clock of the ADC,
+ - clock-names: should be "bus" and "ths",
+ - resets: the bus reset of the ADC,
+
+Example for A33:
ths: ths@01c25000 {
compatible = "allwinner,sun8i-a33-ths";
reg = <0x01c25000 0x100>;
@@ -17,6 +25,17 @@ Example:
#io-channel-cells = <0>;
};
+Example for H3:
+ ths: ths@01c25000 {
+ compatible = "allwinner,sun8i-h3-ths";
+ reg = <0x01c25000 0x100>;
+ clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+ clock-names = "bus", "ths";
+ resets = <&ccu RST_BUS_THS>;
+ #thermal-sensor-cells = <0>;
+ #io-channel-cells = <0>;
+ };
+
sun4i, sun5i and sun6i SoCs are also supported via the older binding:
sun4i resistive touchscreen controller
--
2.12.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox