* Re: [PATCH v5 2/4] iio: Documentation: Add max9611 sysfs documentation
From: Jonathan Cameron @ 2017-04-08 16:00 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: <1491488454-22468-3-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 06/04/17 15:20, 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-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Looks good to me. There is the small matter that when we generalize this
we will probably loose some detail, but such is life.
Applied to the togreg branch of iio.git and pushed out as testing.
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..6d2d2b0
> --- /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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: The value of the shunt resistor used to compute power drain on
> + common input voltage pin (RS+). In Ohms.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/in_current_shunt_resistor
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description: The value of the shunt resistor used to compute current flowing
> + between RS+ and RS- voltage sense inputs. In 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 v5 3/4] iio: adc: Add Maxim max9611 ADC driver
From: Jonathan Cameron @ 2017-04-08 16:01 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: <1491488454-22468-4-git-send-email-jacopo+renesas@jmondi.org>
On 06/04/17 15:20, 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@jmondi.org>
Applied to the togreg branch of iio.git and and pushed out as testing
to see if the various tests can find anything we have all missed!
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: 5.000
> attr 1: input value: 663.404000000
> current: (input)
> 2 channel-specific attributes found:
> attr 0: shunt_resistor value: 5.000
> attr 1: input value: 817.000000000
> temp: (input)
> 2 channel-specific attributes found:
> attr 0: scale value: 480.076812289
> attr 1: raw value: 59
>
> 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 | 585 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 596 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..f0b47fa
> --- /dev/null
> +++ b/drivers/iio/adc/max9611.c
> @@ -0,0 +1,585 @@
> +/*
> + * 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"
> +
> +/* 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));
> + unsigned int i, r;
> +
> + i = max9611->shunt_resistor_uohm / 1000;
> + r = max9611->shunt_resistor_uohm % 1000;
> +
> + return sprintf(buf, "%u.%03u\n", i, r);
> +}
> +
> +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"},
> + {.compatible = "maxim,max9612", .data = "max9612"},
> + { },
> +};
> +
> +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-micro-ohms";
> + 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@jmondi.org>");
> +MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v5 4/4] arm64: dts: salvator-x: Add current sense amplifiers
From: Jonathan Cameron @ 2017-04-08 16:02 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: <1491488454-22468-5-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 06/04/17 15:20, Jacopo Mondi wrote:
> Add device nodes for two Maxim max961x current sense amplifiers
> sensing VDD_08 and DVFS_08 lines.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Driver now applied and should make the coming merge window easily,
so I'll assume this will head upstream via an appropriate tree.
Thanks,
Jonathan
> ---
> arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> index c7f40f8..c4860a1 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -257,6 +257,24 @@
> status = "okay";
> };
>
> +&i2c4 {
> + status = "okay";
> +
> + csa_vdd: adc@7c {
> + compatible = "maxim,max9611";
> + reg = <0x7c>;
> +
> + shunt-resistor-micro-ohms = <5000>;
> + };
> +
> + csa_dvfs: adc@7f {
> + compatible = "maxim,max9611";
> + reg = <0x7f>;
> +
> + shunt-resistor-micro-ohms = <5000>;
> + };
> +};
> +
> &wdt0 {
> timeout-sec = <60>;
> status = "okay";
>
--
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 v3 5/9] arm, arm64: factorize common cpu capacity default code
From: Greg KH @ 2017-04-08 16:25 UTC (permalink / raw)
To: Juri Lelli
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-lFZ/pmaqli7XmaaqVzeoHQ, sudeep.holla-5wv7dgnIgG8,
lorenzo.pieralisi-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, morten.rasmussen-5wv7dgnIgG8,
dietmar.eggemann-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
Russell King
In-Reply-To: <20170327131825.32134-6-juri.lelli-5wv7dgnIgG8@public.gmane.org>
On Mon, Mar 27, 2017 at 02:18:21PM +0100, Juri Lelli wrote:
> arm and arm64 share lot of code relative to parsing CPU capacity
> information from DT, using that information for appropriate scaling and
> exposing a sysfs interface for chaging such values at runtime.
>
> Factorize such code in a common place (driver/base/arch_topology.c) in
> preparation for further additions.
>
> Suggested-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Suggested-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Suggested-by: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
I need some acks here from the arm maintainers before I can take this
series...
thanks,
greg k-h
--
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 4/8] iio: adc: sun4i-gpadc-iio: add support for A33 thermal sensor
From: Jonathan Cameron @ 2017-04-08 16:36 UTC (permalink / raw)
To: Quentin Schulz, dmitry.torokhov, robh+dt, mark.rutland,
maxime.ripard, wens, lee.jones, linux, knaack.h, lars, pmeerw
Cc: thomas.petazzoni, devicetree, linux-iio, linux-kernel,
linux-sunxi, icenowy, linux-input, linux-arm-kernel
In-Reply-To: <20170405090634.4649-5-quentin.schulz@free-electrons.com>
On 05/04/17 10:06, Quentin Schulz wrote:
> This adds support for the Allwinner A33 thermal sensor.
>
> Unlike the A10, A13 and A31, the Allwinner A33 only has one channel
> which is dedicated to the thermal sensor. Moreover, its thermal sensor
> does not generate interruptions, thus we only need to directly read the
> register storing the temperature value.
>
> The MFD used by the A10, A13 and A31, was created to avoid breaking the
> DT binding, but since the nodes for the ADC weren't there for the A33,
> it is not needed.
>
> Though the A33 does not have an internal ADC, it has a thermal sensor
> which shares the same registers with GPADC of the already supported SoCs
> and almost the same bits, for the same purpose (thermal sensor).
>
> The thermal sensor behaves exactly the same (except the presence of
> interrupts or not) on the different SoCs.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
A small amount of fuzz turned up with a fix from Arnd for thermal
dependencies.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
>
> v3:
> - switched compatible from allwinner,sun8i-a33-gpadc-iio to
> allwinner,sun8i-a33-ths to better reflect the datasheet's name,
> - fixed the non-working if (!IS_ENABLED(THERMAL_OF)) by prefixing it with
> CONFIG,
>
> v2:
> - removed added comments in Kconfig,
> - simplified Kconfig depends on condition,
> - removed THERMAL_OF requirement for sun8i,
> - renamed sun8i_gpadc_channels to sun8i_a33_gpadc_channels,
> - renamed use_dt boolean in no_irq as it reflects better why we need it,
> - removed spurious/unneeded modifications done in v1,
>
> drivers/iio/adc/Kconfig | 2 +-
> drivers/iio/adc/sun4i-gpadc-iio.c | 100 ++++++++++++++++++++++++++++++++++++--
> include/linux/mfd/sun4i-gpadc.h | 4 ++
> 3 files changed, 102 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index d0af51d..d9b6101 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -561,7 +561,7 @@ config STX104
> config SUN4I_GPADC
> tristate "Support for the Allwinner SoCs GPADC"
> depends on IIO
> - depends on MFD_SUN4I_GPADC
> + depends on MFD_SUN4I_GPADC || MACH_SUN8I
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> GPADC. This ADC provides 4 channels which can be used as an ADC or as
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 7cb997a..74705aa 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -85,6 +85,12 @@ static const struct gpadc_data sun6i_gpadc_data = {
> .adc_chan_mask = SUN6I_GPADC_CTRL1_ADC_CHAN_MASK,
> };
>
> +static const struct gpadc_data sun8i_a33_gpadc_data = {
> + .temp_offset = -1662,
> + .temp_scale = 162,
> + .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> +};
> +
> struct sun4i_gpadc_iio {
> struct iio_dev *indio_dev;
> struct completion completion;
> @@ -96,6 +102,7 @@ struct sun4i_gpadc_iio {
> unsigned int temp_data_irq;
> atomic_t ignore_temp_data_irq;
> const struct gpadc_data *data;
> + bool no_irq;
> /* prevents concurrent reads of temperature and ADC */
> struct mutex mutex;
> };
> @@ -138,6 +145,23 @@ static const struct iio_chan_spec sun4i_gpadc_channels_no_temp[] = {
> SUN4I_GPADC_ADC_CHANNEL(3, "adc_chan3"),
> };
>
> +static const struct iio_chan_spec sun8i_a33_gpadc_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .datasheet_name = "temp_adc",
> + },
> +};
> +
> +static const struct regmap_config sun4i_gpadc_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
> static int sun4i_prepare_for_irq(struct iio_dev *indio_dev, int channel,
> unsigned int irq)
> {
> @@ -247,6 +271,17 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
> {
> struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> + if (info->no_irq) {
> + pm_runtime_get_sync(indio_dev->dev.parent);
> +
> + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> +
> + pm_runtime_mark_last_busy(indio_dev->dev.parent);
> + pm_runtime_put_autosuspend(indio_dev->dev.parent);
> +
> + return 0;
> + }
> +
> return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq);
> }
>
> @@ -454,6 +489,58 @@ static int sun4i_irq_init(struct platform_device *pdev, const char *name,
> return 0;
> }
>
> +static const struct of_device_id sun4i_gpadc_of_id[] = {
> + {
> + .compatible = "allwinner,sun8i-a33-ths",
> + .data = &sun8i_a33_gpadc_data,
> + },
> + { /* sentinel */ }
> +};
> +
> +static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
> + struct iio_dev *indio_dev)
> +{
> + struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
> + const struct of_device_id *of_dev;
> + struct thermal_zone_device *tzd;
> + struct resource *mem;
> + void __iomem *base;
> + int ret;
> +
> + of_dev = of_match_device(sun4i_gpadc_of_id, &pdev->dev);
> + if (!of_dev)
> + return -ENODEV;
> +
> + info->no_irq = true;
> + info->data = (struct gpadc_data *)of_dev->data;
> + indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels);
> + indio_dev->channels = sun8i_a33_gpadc_channels;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun4i_gpadc_regmap_config);
> + if (IS_ERR(info->regmap)) {
> + ret = PTR_ERR(info->regmap);
> + dev_err(&pdev->dev, "failed to init regmap: %d\n", ret);
> + return ret;
> + }
> +
> + if (!IS_ENABLED(CONFIG_THERMAL_OF))
> + return 0;
> +
> + tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, info,
> + &sun4i_ts_tz_ops);
> + if (IS_ERR(tzd))
> + dev_err(&pdev->dev, "could not register thermal sensor: %ld\n",
> + PTR_ERR(tzd));
> +
> + return PTR_ERR_OR_ZERO(tzd);
> +}
> +
> static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
> struct iio_dev *indio_dev)
> {
> @@ -462,6 +549,7 @@ static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
> dev_get_drvdata(pdev->dev.parent);
> int ret;
>
> + info->no_irq = false;
> info->regmap = sun4i_gpadc_dev->regmap;
>
> indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
> @@ -561,7 +649,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> indio_dev->info = &sun4i_gpadc_iio_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> - ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
> + if (pdev->dev.of_node)
> + ret = sun4i_gpadc_probe_dt(pdev, indio_dev);
> + else
> + ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
> +
> if (ret)
> return ret;
>
> @@ -580,7 +672,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> return 0;
>
> err_map:
> - if (IS_ENABLED(CONFIG_THERMAL_OF))
> + if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> pm_runtime_put(&pdev->dev);
> @@ -592,10 +684,11 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> static int sun4i_gpadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
>
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> - if (IS_ENABLED(CONFIG_THERMAL_OF))
> + if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> return 0;
> @@ -611,6 +704,7 @@ static const struct platform_device_id sun4i_gpadc_id[] = {
> static struct platform_driver sun4i_gpadc_driver = {
> .driver = {
> .name = "sun4i-gpadc-iio",
> + .of_match_table = sun4i_gpadc_of_id,
> .pm = &sun4i_gpadc_pm_ops,
> },
> .id_table = sun4i_gpadc_id,
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 509e736..139872c 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -38,6 +38,10 @@
> #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)
> +
> #define SUN4I_GPADC_CTRL2 0x08
>
> #define SUN4I_GPADC_CTRL2_TP_SENSITIVE_ADJUST(x) ((GENMASK(3, 0) & (x)) << 28)
>
^ permalink raw reply
* Re: [PATCH 1/3 v5] iio: xoadc: augment DT bindings a bit
From: Jonathan Cameron @ 2017-04-08 16:40 UTC (permalink / raw)
To: Linus Walleij, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170404120819.6921-1-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 04/04/17 13:08, Linus Walleij wrote:
> In order to accommodate in a logical manner for the premuxed channels
> in PM8921 and the similarly addressed channels in later PMICs, we
> need a twocell arrangement with premux and analog mux setting as
> a tuple to uniquely identify a hardware channel.
>
> These bindings are not yet in use, so it should be fine to augment
> them before we actually start using it in drivers and device trees.
>
> This scheme came out of lengthy discussions and reverse-engineering
> and reading of the few information sources we have.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Acked-by: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Suggested-by: Björn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> ChangeLog v3->v5:
> - Some nitpicky spelling fixes. Added Rob's ACK.
> ChangeLog v2->v3:
> - Drop leading zeroes on unit addresses.
> ChangeLog v1->v2:
> - Name nodes with <01 02> in a foo@0102 pattern.
> - Minor spelling nits.
> - Delete flimsy leftover docs from an interrim development path.
> ---
> .../bindings/iio/adc/qcom,pm8xxx-xoadc.txt | 76 ++++++++++++----------
> 1 file changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> index 53cd146d8096..3ae06127789e 100644
> --- a/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> @@ -19,32 +19,42 @@ Required properties:
> with PMIC variant but is typically something like 2.2 or 1.8V.
>
> The following required properties are standard for IO channels, see
> -iio-bindings.txt for more details:
> +iio-bindings.txt for more details, but notice that this particular
> +ADC has a special addressing scheme that require two cells for
> +identifying each ADC channel:
>
> -- #address-cells: should be set to <1>
> +- #address-cells: should be set to <2>, the first cell is the
> + prescaler (on PM8058) or premux (on PM8921) with two valid bits
> + so legal values are 0x00, 0x01 or 0x02. The second cell
> + is the main analog mux setting (0x00..0x0f). The combination
> + of prescaler/premux and analog mux uniquely addresses a hardware
> + channel on all systems.
>
> - #size-cells: should be set to <0>
>
> -- #io-channel-cells: should be set to <1>
> +- #io-channel-cells: should be set to <2>, again the cells are
> + precaler or premux followed by the analog muxing line.
>
> - interrupts: should refer to the parent PMIC interrupt controller
> and reference the proper ADC interrupt.
>
> Required subnodes:
>
> -The ADC channels are configured as subnodes of the ADC. Since some of
> -them are used for calibrating the ADC, these nodes are compulsory:
> +The ADC channels are configured as subnodes of the ADC.
> +
> +Since some of them are used for calibrating the ADC, these nodes are
> +compulsory:
>
> adc-channel@c {
> - reg = <0x0c>;
> + reg = <0x00 0x0c>;
> };
>
> adc-channel@d {
> - reg = <0x0d>;
> + reg = <0x00 0x0d>;
> };
>
> adc-channel@f {
> - reg = <0x0f>;
> + reg = <0x00 0x0f>;
> };
>
> These three nodes are used for absolute and ratiometric calibration
> @@ -52,13 +62,13 @@ and only need to have these reg values: they are by hardware definition
> 1:1 ratio converters that sample 625, 1250 and 0 milliV and create
> an interpolation calibration for all other ADCs.
>
> -Optional subnodes: any channels other than channel 0x0c, 0x0d and
> -0x0f are optional.
> +Optional subnodes: any channels other than channels [0x00 0x0c],
> +[0x00 0x0d] and [0x00 0x0f] are optional.
>
> Required channel node properties:
>
> - reg: should contain the hardware channel number in the range
> - 0 .. 0x0f (4 bits). The hardware only supports 16 channels.
> + 0 .. 0xff (8 bits).
>
> Optional channel node properties:
>
> @@ -94,56 +104,54 @@ Example:
> xoadc: xoadc@197 {
> compatible = "qcom,pm8058-adc";
> reg = <0x197>;
> - interrupt-parent = <&pm8058>;
> - interrupts = <76 1>;
> - #address-cells = <1>;
> + interrupts-extended = <&pm8058 76 IRQ_TYPE_EDGE_RISING>;
> + #address-cells = <2>;
> #size-cells = <0>;
> - #io-channel-cells = <1>;
> + #io-channel-cells = <2>;
>
> vcoin: adc-channel@0 {
> - reg = <0x00>;
> + reg = <0x00 0x00>;
> };
> vbat: adc-channel@1 {
> - reg = <0x01>;
> + reg = <0x00 0x01>;
> };
> dcin: adc-channel@2 {
> - reg = <0x02>;
> + reg = <0x00 0x02>;
> };
> ichg: adc-channel@3 {
> - reg = <0x03>;
> + reg = <0x00 0x03>;
> };
> vph_pwr: adc-channel@4 {
> - reg = <0x04>;
> + reg = <0x00 0x04>;
> };
> usb_vbus: adc-channel@a {
> - reg = <0x0a>;
> + reg = <0x00 0x0a>;
> };
> die_temp: adc-channel@b {
> - reg = <0x0b>;
> + reg = <0x00 0x0b>;
> };
> ref_625mv: adc-channel@c {
> - reg = <0x0c>;
> + reg = <0x00 0x0c>;
> };
> ref_1250mv: adc-channel@d {
> - reg = <0x0d>;
> + reg = <0x00 0x0d>;
> };
> ref_325mv: adc-channel@e {
> - reg = <0x0e>;
> + reg = <0x00 0x0e>;
> };
> ref_muxoff: adc-channel@f {
> - reg = <0x0f>;
> + reg = <0x00 0x0f>;
> };
> };
>
> -
> /* IIO client node */
> iio-hwmon {
> compatible = "iio-hwmon";
> - io-channels = <&xoadc 0x01>, /* Battery */
> - <&xoadc 0x02>, /* DC in (charger) */
> - <&xoadc 0x04>, /* VPH the main system voltage */
> - <&xoadc 0x0b>, /* Die temperature */
> - <&xoadc 0x0c>, /* Reference voltage 1.25V */
> - <&xoadc 0x0d>, /* Reference voltage 0.625V */
> - <&xoadc 0x0e>; /* Reference voltage 0.325V */
> + io-channels = <&xoadc 0x00 0x01>, /* Battery */
> + <&xoadc 0x00 0x02>, /* DC in (charger) */
> + <&xoadc 0x00 0x04>, /* VPH the main system voltage */
> + <&xoadc 0x00 0x0b>, /* Die temperature */
> + <&xoadc 0x00 0x0c>, /* Reference voltage 1.25V */
> + <&xoadc 0x00 0x0d>, /* Reference voltage 0.625V */
> + <&xoadc 0x00 0x0e>; /* Reference voltage 0.325V */
> };
>
^ permalink raw reply
* Re: [PATCH v5 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
From: jmondi @ 2017-04-08 16:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Geert Uytterhoeven, Jacopo Mondi, Wolfram Sang, Magnus Damm,
Laurent Pinchart, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, Rob Herring, Mark Rutland,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <e7a53314-2e57-ae3a-af53-0d22ab445f5a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Hi Jonathan,
On Sat, Apr 08, 2017 at 04:59:39PM +0100, Jonathan Cameron wrote:
> On 06/04/17 15:43, Geert Uytterhoeven wrote:
>
> > On Thu, Apr 6, 2017 at 4:20 PM, Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt
> >> @@ -0,0 +1,27 @@
> >> +* 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-micro-homs: Value, in micro Ohms, of the current sense shunt
> >
> > s/homs/ohms/
> I'll fix the title as Rob requested and this.
>
Uh! I missed that title thing! Sorry about this, and thank you
Jonathan for doing that!
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>
> Thanks for your hard work on this one!
>
Thank you all for your effort in reviewing this
Thanks
j
> Jonathan
> >
> >> + resistor
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
> >
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> > -- Linus Torvalds
> > --
> > 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
> >
>
--
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: [PATCHv3 02/10] serdev: add serdev_device_wait_until_sent
From: Greg Kroah-Hartman @ 2017-04-08 16:57 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, Rob Herring,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, linux-bluetooth, linux-serial, devicetree,
linux-kernel
In-Reply-To: <20170328155939.31566-3-sre@kernel.org>
On Tue, Mar 28, 2017 at 05:59:31PM +0200, Sebastian Reichel wrote:
> Add method, which waits until the transmission buffer has been sent.
> Note, that the change in ttyport_write_wakeup is related, since
> tty_wait_until_sent will hang without that change.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> ---
> Changes since PATCHv2:
> * Avoid goto in ttyport_write_wakeup
> ---
> drivers/tty/serdev/core.c | 11 +++++++++++
> drivers/tty/serdev/serdev-ttyport.c | 18 ++++++++++++++----
> include/linux/serdev.h | 3 +++
> 3 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f4c6c90add78..a63b74031e22 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -173,6 +173,17 @@ void serdev_device_set_flow_control(struct serdev_device *serdev, bool enable)
> }
> EXPORT_SYMBOL_GPL(serdev_device_set_flow_control);
>
> +void serdev_device_wait_until_sent(struct serdev_device *serdev, long timeout)
> +{
> + struct serdev_controller *ctrl = serdev->ctrl;
> +
> + if (!ctrl || !ctrl->ops->wait_until_sent)
> + return;
> +
> + ctrl->ops->wait_until_sent(ctrl, timeout);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_wait_until_sent);
Is this still needed now that we have serdev_device_write() with an
unlimited timeout available?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/4] iio: dac: add support for stm32 DAC
From: Jonathan Cameron @ 2017-04-08 17:13 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: <523a6842-7ffe-2236-dda8-6278c8637fa6-qxv4g6HH51o@public.gmane.org>
On 05/04/17 16:48, Fabrice Gasnier wrote:
> On 04/02/2017 01:32 PM, Jonathan Cameron wrote:
>> 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..
>>
> Hi Jonathan,
> I hope I have nothing to do with 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.
>>
> Ok, I'll try to use powerdown_mode in V2 as other DACs do. For now,
> basically, I'll remap same functionality as 'enable' of this patch.
>
> What do you mean by 'fixed value' ?
Read only with only one value.
>
> But, this also raise me one question:
> Current patch use 'enable' to set EN bits in control register. Then, DAC
> output goes from Hi-Z to buffered output.
> There is also other power modes available. One of them is 'unbuffered':
> output buffer can be disabled/bypassed.
> This typically can save power, but it only makes sense to use it
> depending on output load impedance (This is explained in AN3126 as you
> pointed out in later patch).
> Current patch uses buffered output (which suits all needs regarding
> output load impedance). And the question is...
>
> Should I expose this power modes to userland by using 'powerdown_mode' ?
>
> OR... I'd rather rely on a dt property like st,dac-output-mode to manage
> this, because buffered/unbuffered output modes depends on HW output load
> impedance. Do you agree with this approach to use:
> - powerdown_mode as Hi-Z / enable switch, with dedicated dt property to
> set output power mode ?
Yes. If it's hardware dependant like this it belongs in dt to my mind.
>
> Please let me know your opinion.
>
> But, I think this can be part of another patchset...
Absolutely. No need to support all the bells and whistles
from the start!
>
>> 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!)
> Good guess ;-)
> But okay, I'll use devm_ anyway, regardless of other patches.
>
> Thanks & Regards,
> Fabrice
>
>>> +
>>> + 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");
>>>
>>
> --
> 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
>
--
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 3/4] iio: dac: stm32: add support for trigger events
From: Jonathan Cameron @ 2017-04-08 17: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: <ef0c3a6f-1d67-a7d7-b7ae-8b06b5e92414-qxv4g6HH51o@public.gmane.org>
On 05/04/17 17:44, Fabrice Gasnier wrote:
> On 04/02/2017 02:21 PM, Jonathan Cameron wrote:
>> 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...
>>>
> Waveform generator in STM32 DAC requires a trigger to increment /
> decrement internal counter in case of triangle generator. Noise
> generator is a bit different, but same trigger usage applies. I agree
> this is unusual.
> Is it acceptable to use event trigger for this use ?
Not sure. It's kind of a like an output trigger but with the buffer
tied to a hardware source (a bit like the hardware consumers we have
in the other direction).
I think it's closer to an output trigger than an event trigger certainly.
Question is whether it should be something else entirely...
>
>>> 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).
>
> Hmm.. for waveform generator mode, there is no need for data buffer. DAC
> generate samples itself, using trigger. But, i agree it would be nice
> for playing data samples (write DHR registers, or dma), yes.
Definitely in the nice to have category - except wrt to having a
hardware_buffer_provider or similar to provide the data stream.
This is kind of similar to when we have a data pipeline on incoming data
where the data is never actually visible to some IIO device because it's
pushed into some processing engine directly.
It's a bit of an oddity that we need to think about when looking at output
triggering. The previous DDS devices I have seen have all be directly
clocked...
>
>>>
>>> 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!*
>
> Hopefully this can happen.
>
> For the time being, I'll propose a similar patch in V2. I found out this
> patch is missing a clear path to (re-)assign trigger, once set by
> userland. Also, driver never gets informed in case trigger gets changed
> or removed, without re-enabling it:
> e.g. like echo "" > trigger/current_trigger
> I'll propose a small change. Hope you agree with this approach.
>
Cool. Definitely looking for some analog devices input on this.
They have a quite a few similar out of tree drivers beyond those currently
in staging...
Michael / Lars?
I briefly discussed output buffers with Lars earlier in the week and the
main outstanding issues were around userspace ABI. Last time we talked
about this in detail was quite a few years ago IIRC so time for a revisit.
+ hopefully some progress.
Jonathan
> Thanks,
> Fabrice
>
>>>
>>> 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
>>>
>>
> --
> 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 4/4] iio: dac: stm32: add support for waveform generator
From: Jonathan Cameron @ 2017-04-08 17:21 UTC (permalink / raw)
To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
linux-kernel
Cc: linux-iio, mark.rutland, mcoquelin.stm32, alexandre.torgue, lars,
knaack.h, pmeerw, benjamin.gaignard, benjamin.gaignard,
linus.walleij, amelie.delaunay, Hennerich, Michael
In-Reply-To: <ca8304bf-9107-f248-baff-8106fd68341c@st.com>
On 05/04/17 17:46, Fabrice Gasnier wrote:
> JonathanOn 04/02/2017 02:19 PM, Jonathan Cameron wrote:
>> 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@st.com>
>>
>> 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).
> Hi Jonathan,
>
> Just to clarify this, some products like STM32F334xx have 3 channels,
> yes. Several STM32 DAC IPs (& so registers) are instantiated: DAC1 have
> two outputs (dac1_out1 & dac1_out2), DAC2 have one output (e.g.
> dac2_out1). Driver can be instantiated several times. Is it ok ?
Sure - I'd missed the complex structure when reading the doc.
>
>>
>> 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...
>
> Thanks for pointing this out.
>
>>
>>
>>> ---
>>> 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
>>
> I'll rework this in V2.
>>
>>> +KernelVersion: 4.12
>>> +Contact: fabrice.gasnier@st.com
>>> +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.
>
> I'll update from 'none' to 'flat' in V2, and use _offset.
It's a bit ugly and doesn't generalize well to driving this via DMA for example...
J
>>
>>> +
>>> +What: /sys/bus/iio/devices/iio:deviceX/mamp
>>> +What: /sys/bus/iio/devices/iio:deviceX/mamp_available
>>> +KernelVersion: 4.12
>>> +Contact: fabrice.gasnier@st.com
>>> +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.
> Sorry for this, this is basically amplitude.
>
> I think best is to rename above to something like 'amplitude' and
> 'amplitude_available'.
>
> I'll rework this in V2.
>>
>>
>> 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[] = {
>>>
>>
> --
> 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 v4 3/8] iio: adc: sun4i-gpadc-iio: move code used in MFD probing to new function
From: Jonathan Cameron @ 2017-04-08 17:26 UTC (permalink / raw)
To: Quentin Schulz, dmitry.torokhov, robh+dt, mark.rutland,
maxime.ripard, wens, lee.jones, linux, knaack.h, lars, pmeerw
Cc: thomas.petazzoni, devicetree, linux-iio, linux-kernel,
linux-sunxi, icenowy, linux-input, linux-arm-kernel
In-Reply-To: <20170405090634.4649-4-quentin.schulz@free-electrons.com>
On 05/04/17 10:06, Quentin Schulz wrote:
> This moves code used in MFD probing to a new sun4i_gpadc_probe_mfd
> function.
>
> This driver was initially written for A10, A13 and A31 SoCs which
> already had a DT binding for this IP, thus we needed to use an MFD to
> probe the different drivers without changing the DT binding of these
> SoCs.
>
> For SoCs that will require to create a DT binding for this IP, we can
> avoid using an MFD, thus we need two separate functions: one for probing
> via MFD and one for probing without MFD.
>
> This split the code specific to MFD probing in a function separated from
> the driver probe function.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
Clearly the series is getting split up and going via different trees
which is fine. I'll drop my Ack and apply it to the togreg branch of iio.git
with a signed-off.
Thanks,
Jonathan
> ---
>
> v3:
> - updated commit log,
>
> added in v2
>
> drivers/iio/adc/sun4i-gpadc-iio.c | 78 ++++++++++++++++++++++-----------------
> 1 file changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index a8e134f..7cb997a 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -454,31 +454,16 @@ static int sun4i_irq_init(struct platform_device *pdev, const char *name,
> return 0;
> }
>
> -static int sun4i_gpadc_probe(struct platform_device *pdev)
> +static int sun4i_gpadc_probe_mfd(struct platform_device *pdev,
> + struct iio_dev *indio_dev)
> {
> - struct sun4i_gpadc_iio *info;
> - struct iio_dev *indio_dev;
> + struct sun4i_gpadc_iio *info = iio_priv(indio_dev);
> + struct sun4i_gpadc_dev *sun4i_gpadc_dev =
> + dev_get_drvdata(pdev->dev.parent);
> int ret;
> - struct sun4i_gpadc_dev *sun4i_gpadc_dev;
> -
> - sun4i_gpadc_dev = dev_get_drvdata(pdev->dev.parent);
> -
> - indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> - if (!indio_dev)
> - return -ENOMEM;
>
> - info = iio_priv(indio_dev);
> - platform_set_drvdata(pdev, indio_dev);
> -
> - mutex_init(&info->mutex);
> info->regmap = sun4i_gpadc_dev->regmap;
> - info->indio_dev = indio_dev;
> - init_completion(&info->completion);
> - 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 = &sun4i_gpadc_iio_info;
> - indio_dev->modes = INDIO_DIRECT_MODE;
> +
> indio_dev->num_channels = ARRAY_SIZE(sun4i_gpadc_channels);
> indio_dev->channels = sun4i_gpadc_channels;
>
> @@ -519,8 +504,7 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> dev_err(&pdev->dev,
> "could not register thermal sensor: %ld\n",
> PTR_ERR(tzd));
> - ret = PTR_ERR(tzd);
> - goto err;
> + return PTR_ERR(tzd);
> }
> } else {
> indio_dev->num_channels =
> @@ -528,36 +512,65 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> indio_dev->channels = sun4i_gpadc_channels_no_temp;
> }
>
> - pm_runtime_set_autosuspend_delay(&pdev->dev,
> - SUN4I_GPADC_AUTOSUSPEND_DELAY);
> - pm_runtime_use_autosuspend(&pdev->dev);
> - pm_runtime_set_suspended(&pdev->dev);
> - pm_runtime_enable(&pdev->dev);
> -
> if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> ret = sun4i_irq_init(pdev, "TEMP_DATA_PENDING",
> sun4i_gpadc_temp_data_irq_handler,
> "temp_data", &info->temp_data_irq,
> &info->ignore_temp_data_irq);
> if (ret < 0)
> - goto err;
> + return ret;
> }
>
> ret = sun4i_irq_init(pdev, "FIFO_DATA_PENDING",
> sun4i_gpadc_fifo_data_irq_handler, "fifo_data",
> &info->fifo_data_irq, &info->ignore_fifo_data_irq);
> if (ret < 0)
> - goto err;
> + return ret;
>
> if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> ret = iio_map_array_register(indio_dev, sun4i_gpadc_hwmon_maps);
> if (ret < 0) {
> dev_err(&pdev->dev,
> "failed to register iio map array\n");
> - goto err;
> + return ret;
> }
> }
>
> + return 0;
> +}
> +
> +static int sun4i_gpadc_probe(struct platform_device *pdev)
> +{
> + struct sun4i_gpadc_iio *info;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + info = iio_priv(indio_dev);
> + platform_set_drvdata(pdev, indio_dev);
> +
> + mutex_init(&info->mutex);
> + info->indio_dev = indio_dev;
> + init_completion(&info->completion);
> + 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 = &sun4i_gpadc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = sun4i_gpadc_probe_mfd(pdev, indio_dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_set_autosuspend_delay(&pdev->dev,
> + SUN4I_GPADC_AUTOSUSPEND_DELAY);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> ret = devm_iio_device_register(&pdev->dev, indio_dev);
> if (ret < 0) {
> dev_err(&pdev->dev, "could not register the device\n");
> @@ -570,7 +583,6 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
> if (IS_ENABLED(CONFIG_THERMAL_OF))
> iio_map_array_unregister(indio_dev);
>
> -err:
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
>
^ permalink raw reply
* [PATCH V2] clk: hi6220: Add the hi655x's pmic clock
From: Daniel Lezcano @ 2017-04-08 20:30 UTC (permalink / raw)
To: sboyd-sgV2jX0FEOL9JmXXK+q4OQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
lee.jones-QSEj5FYQhm4dnm+yROfE0A
Cc: xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-clk-u79uwXL29TY76Z2rM5mHXA
The hi655x multi function device is a PMIC providing regulators.
The PMIC also provides a clock for the WiFi and the Bluetooth, let's implement
this clock in order to add it in the hi655x MFD and allow proper wireless
initialization.
Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changelog:
V2:
- Added COMPILE_TEST option, compiled on x86
- Removed useless parenthesis
- Used of_clk_hw_simple_get() instead of deref dance
- Do bailout if the clock-names is not specified
- Rollback on error
- Folded mfd line change and binding
- Added #clock-cells binding documentation
- Added #clock-cells in the DT
V1: initial post
---
---
.../devicetree/bindings/mfd/hisilicon,hi655x.txt | 6 +
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 +
drivers/clk/Kconfig | 8 ++
drivers/clk/Makefile | 1 +
drivers/clk/clk-hi655x.c | 140 +++++++++++++++++++++
drivers/mfd/hi655x-pmic.c | 3 +-
6 files changed, 158 insertions(+), 1 deletion(-)
create mode 100644 drivers/clk/clk-hi655x.c
diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
index 0548569..194e2a9fd 100644
--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
@@ -16,6 +16,11 @@ Required properties:
- reg: Base address of PMIC on Hi6220 SoC.
- interrupt-controller: Hi655x has internal IRQs (has own IRQ domain).
- pmic-gpios: The GPIO used by PMIC IRQ.
+- #clock-cells: From common clock binding; shall be set to 0
+
+Optional properties:
+- clock-output-names: From common clock binding to override the
+ default output clock name
Example:
pmic: pmic@f8000000 {
@@ -24,4 +29,5 @@ Example:
interrupt-controller;
#interrupt-cells = <2>;
pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+ clock-cells = <0>;
}
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index dba3c13..bb9afb1 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -328,6 +328,7 @@
interrupt-controller;
#interrupt-cells = <2>;
pmic-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+ #clock-cells = <0>;
regulators {
ldo2: LDO2 {
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 9356ab4..36cfea3 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -47,6 +47,14 @@ config COMMON_CLK_RK808
clocked at 32KHz each. Clkout1 is always on, Clkout2 can off
by control register.
+config COMMON_CLK_HI655X
+ tristate "Clock driver for Hi655x"
+ depends on MFD_HI655X_PMIC || COMPILE_TEST
+ ---help---
+ This driver supports the hi655x PMIC clock. This
+ multi-function device has one fixed-rate oscillator, clocked
+ at 32KHz.
+
config COMMON_CLK_SCPI
tristate "Clock driver controlled via SCPI interface"
depends on ARM_SCPI_PROTOCOL || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 92c12b8..c19983a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o
obj-$(CONFIG_COMMON_CLK_PWM) += clk-pwm.o
obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o
obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o
+obj-$(CONFIG_COMMON_CLK_HI655X) += clk-hi655x.o
obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o
obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o
obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o
diff --git a/drivers/clk/clk-hi655x.c b/drivers/clk/clk-hi655x.c
new file mode 100644
index 0000000..b2bea32
--- /dev/null
+++ b/drivers/clk/clk-hi655x.c
@@ -0,0 +1,140 @@
+/*
+ * Clock driver for Hi655x
+ *
+ * Copyright (c) 2016, Linaro Ltd.
+ *
+ * Author: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ */
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/hi655x-pmic.h>
+
+#define HI655X_CLK_BASE HI655X_BUS_ADDR(0x1c)
+#define HI655X_CLK_SET BIT(6)
+
+struct hi655x_clk {
+ struct hi655x_pmic *hi655x;
+ struct clk_hw clk_hw;
+};
+
+static unsigned long hi655x_clk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return 32768;
+}
+
+static int hi655x_clk_enable(struct clk_hw *hw, bool enable)
+{
+ struct hi655x_clk *hi655x_clk =
+ container_of(hw, struct hi655x_clk, clk_hw);
+
+ struct hi655x_pmic *hi655x = hi655x_clk->hi655x;
+
+ return regmap_update_bits(hi655x->regmap, HI655X_CLK_BASE,
+ HI655X_CLK_SET, enable ? HI655X_CLK_SET : 0);
+}
+
+static int hi655x_clk_prepare(struct clk_hw *hw)
+{
+ return hi655x_clk_enable(hw, true);
+}
+
+static void hi655x_clk_unprepare(struct clk_hw *hw)
+{
+ hi655x_clk_enable(hw, false);
+}
+
+static int hi655x_clk_is_prepared(struct clk_hw *hw)
+{
+ struct hi655x_clk *hi655x_clk =
+ container_of(hw, struct hi655x_clk, clk_hw);
+ struct hi655x_pmic *hi655x = hi655x_clk->hi655x;
+ int ret;
+ uint32_t val;
+
+ ret = regmap_read(hi655x->regmap, HI655X_CLK_BASE, &val);
+ if (ret < 0)
+ return ret;
+
+ return val & HI655X_CLK_BASE;
+}
+
+static const struct clk_ops hi655x_clk_ops = {
+ .prepare = hi655x_clk_prepare,
+ .unprepare = hi655x_clk_unprepare,
+ .is_prepared = hi655x_clk_is_prepared,
+ .recalc_rate = hi655x_clk_recalc_rate,
+};
+
+static int hi655x_clk_probe(struct platform_device *pdev)
+{
+ struct device *parent = pdev->dev.parent;
+ struct hi655x_pmic *hi655x = dev_get_drvdata(parent);
+ struct clk_init_data *hi655x_clk_init;
+ struct hi655x_clk *hi655x_clk;
+ const char *clk_name = "hi655x-clk";
+ int ret;
+
+ hi655x_clk = devm_kzalloc(&pdev->dev, sizeof(*hi655x_clk), GFP_KERNEL);
+ if (!hi655x_clk)
+ return -ENOMEM;
+
+ hi655x_clk_init = devm_kzalloc(&pdev->dev, sizeof(*hi655x_clk_init),
+ GFP_KERNEL);
+ if (!hi655x_clk_init)
+ return -ENOMEM;
+
+ of_property_read_string_index(parent->of_node, "clock-output-names",
+ 0, &clk_name);
+
+ hi655x_clk_init->name = clk_name;
+ hi655x_clk_init->ops = &hi655x_clk_ops;
+
+ hi655x_clk->clk_hw.init = hi655x_clk_init;
+ hi655x_clk->hi655x = hi655x;
+
+ platform_set_drvdata(pdev, hi655x_clk);
+
+ ret = devm_clk_hw_register(&pdev->dev, &hi655x_clk->clk_hw);
+ if (ret)
+ return ret;
+
+ ret = of_clk_add_hw_provider(parent->of_node, of_clk_hw_simple_get,
+ &hi655x_clk->clk_hw);
+ if (ret)
+ return ret;
+
+ ret = clk_hw_register_clkdev(&hi655x_clk->clk_hw, clk_name, NULL);
+ if (ret)
+ of_clk_del_provider(parent->of_node);
+
+ return ret;
+}
+
+static struct platform_driver hi655x_clk_driver = {
+ .probe = hi655x_clk_probe,
+ .driver = {
+ .name = "hi655x-clk",
+ },
+};
+
+module_platform_driver(hi655x_clk_driver);
+
+MODULE_DESCRIPTION("Clk driver for the hi655x series PMICs");
+MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi655x-clk");
diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
index ba706ad..c37ccbf 100644
--- a/drivers/mfd/hi655x-pmic.c
+++ b/drivers/mfd/hi655x-pmic.c
@@ -77,7 +77,8 @@
.num_resources = ARRAY_SIZE(pwrkey_resources),
.resources = &pwrkey_resources[0],
},
- { .name = "hi655x-regulator", },
+ { .name = "hi655x-regulator", },
+ { .name = "hi655x-clk", },
};
static void hi655x_local_irq_clear(struct regmap *map)
--
1.9.1
--
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 related
* Re: [PATCH] drm/panel: simple: Add support for Seiko 43WVF1G
From: Fabio Estevam @ 2017-04-08 20:45 UTC (permalink / raw)
To: Rob Herring, Thierry Reding
Cc: Breno Lima, Shawn Guo, David Airlie, Mark Rutland,
DRI mailing list,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Breno Matheus Lima, Fabio Estevam
In-Reply-To: <CAOMZO5C+7v=ppVpSYwKEiprCSVSM1z0wVVOwSmbuEPaO3618Wg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Thierry/Rob,
On Tue, Feb 7, 2017 at 10:48 PM, Fabio Estevam <festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Feb 7, 2017 at 9:36 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> Except I have no way of knowing whether: a) you omitted a supply
>> because you don't (yet) care, b) the panel has a single supply and you
>> are using power-supply or c) the panel has multiple supplies and your
>> binding is wrong.
>>
>> I can only eliminate A if you list the supplies. Just need something
>> like "power-supply : see simple-panel.txt". I've still got to go read
>> the panel spec if I really want to check the binding.
>
> Just checked the panel datasheet at
> http://www.glyn.de/data/glyn/media/doc/43wvf1g-0.pdf and it lists two
> supplies:
> DVDD (Digital power supply) and AVDD (Analog power supply).
>
> Our dts was just providing a single 'power-supply' which referred to a
> GPIO enabled regulator that drives DVDD.
>
> So it seems we missed to pass AVDD (not software controlled in our
> case, but we need to describe it in dts anyway).
>
> Does this mean we cannot use simple-panel for this particular panel
> and we should add a separate driver for it?
Please confirm if we need to create a separate driver for this panel, thanks.
--
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] arm64: tegra: Add CPU and PSCI nodes for NVIDIA Tegra210 platforms
From: Vagrant Cascadian @ 2017-04-09 2:22 UTC (permalink / raw)
To: Thierry Reding, Stephen Warren
Cc: linux-tegra, devicetree, Martin Michlmayr, linux-arm-kernel,
Jon Hunter
In-Reply-To: <1490701718-16571-1-git-send-email-jonathanh@nvidia.com>
[-- Attachment #1.1: Type: text/plain, Size: 2289 bytes --]
On 2017-03-28, Jon Hunter wrote:
> Add the CPU and PSCI nodes for the NVIDIA Tegra210 platforms so that
> all CPUs can be enabled on boot. This assumes that the PSCI firmware
> has been loaded during the initial bootstrap on the device before the
> kernel starts (which is typically the case for these platforms). The
> PSCI firmware version is set to v0.2 which aligns with the current
> shipping version for Tegra.
>
> Reported-by: Martin Michlmayr <tbm@cyrius.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Tested on a jetson-tx1; all CPUs were recognized.
Tested-By: Vagrant Cascadian <vagrant@debian.org>
> ---
> arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 23 +++++++++++++++++++++++
> arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi | 24 ++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> index 906fb836d241..de1696c28140 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi
> @@ -296,6 +296,29 @@
> };
> };
>
> + cpus {
> + cpu@0 {
> + enable-method = "psci";
> + };
> +
> + cpu@1 {
> + enable-method = "psci";
> + };
> +
> + cpu@2 {
> + enable-method = "psci";
> + };
> +
> + cpu@3 {
> + enable-method = "psci";
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> regulators {
> vdd_gpu: regulator@100 {
> compatible = "pwm-regulator";
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi b/arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi
> index 0ec92578cacb..67cb039965fd 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2530.dtsi
> @@ -51,4 +51,28 @@
> clock-frequency = <32768>;
> };
> };
> +
> + cpus {
> + cpu@0 {
> + enable-method = "psci";
> + };
> +
> + cpu@1 {
> + enable-method = "psci";
> + };
> +
> + cpu@2 {
> + enable-method = "psci";
> + };
> +
> + cpu@3 {
> + enable-method = "psci";
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> };
> --
> 2.7.4
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/5] iio: dac: add support for stm32 DAC
From: Jonathan Cameron @ 2017-04-09 8:39 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
In-Reply-To: <1491495116-7209-3-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 06/04/17 17:11, 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>
A few little bits and pieces from the change to powerdown modes...
I have also been thinking about whether we can take this prior to sorting
out the DDS side, but worry a little that we may end up wanting to change
the way we define the channels - so that might need sorting to some degree
before we can get the basics in place.
Jonathan
> ---
> Changes in v2:
> - Define 'Hi-Z'/'enable' powerdown modes instead of using 'enable'
> attribute normally not used for DACs.
> - use 'reg' instead of 'st,dac-channel' property
> - Use macro to differentiate channels
> - Fix typos, remove leading '&' for functions
> - Add comments on single channel per device
> - Use devm_iio_device_register variant, removes need for .remove
> ---
> 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 | 350 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 598 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..daf0993
> --- /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 selected
> + */
> +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..c0d993a
> --- /dev/null
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -0,0 +1,350 @@
> +/*
> + * 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
> +#define STM32_DAC_IS_CHAN_1(ch) ((ch) & STM32_DAC_CHANNEL_1)
> +
> +/**
> + * 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 iio_dev *indio_dev, int channel)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 en, val;
> + int ret;
> +
> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
> + if (ret < 0)
> + return ret;
> + if (STM32_DAC_IS_CHAN_1(channel))
> + 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 = STM32_DAC_IS_CHAN_1(channel) ?
> + 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 either.
> + */
> + 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 = STM32_DAC_IS_CHAN_1(channel) ?
> + 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 (STM32_DAC_IS_CHAN_1(channel))
> + 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 (STM32_DAC_IS_CHAN_1(channel))
> + 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);
> +
> + 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;
> + 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);
> + 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,
> +};
> +
> +static const char * const stm32_dac_powerdown_modes[] = {
> + "Hi-Z",
> + "enable",
I wouldn't expect to see enable in here, that is handled by the power down
attribute.
Hi-Z is the currently defined threestate I think?
Documenation/ABI/testing/sysfs-bus-iio
Fine to propose additions to this but they need documenting in that general
file.
> +};
> +
> +static int stm32_dac_get_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + return stm32_dac_is_enabled(indio_dev, chan->channel);
> +}
> +
> +static int stm32_dac_set_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int type)
> +{
> + if (type)
> + return stm32_dac_enable(indio_dev, chan->channel);
> + else
> + return stm32_dac_disable(indio_dev, chan->channel);
> +}
> +
> +static ssize_t stm32_dac_read_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + int ret = stm32_dac_is_enabled(indio_dev, chan->channel);
> +
> + if (ret < 0)
> + return ret;
> +
> + return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + unsigned int en;
> + int ret;
> +
> + ret = kstrtouint(buf, 0, &en);
strtobool perhaps?
> + if (ret)
> + return ret;
> +
> + ret = stm32_dac_set_powerdown_mode(indio_dev, chan, en);
> + if (ret < 0)
> + return ret;
> +
> + return len;
> +}
> +
> +static const struct iio_enum stm32_dac_powerdown_mode_en = {
> + .items = stm32_dac_powerdown_modes,
> + .num_items = ARRAY_SIZE(stm32_dac_powerdown_modes),
> + .get = stm32_dac_get_powerdown_mode,
> + .set = stm32_dac_set_powerdown_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
> + {
> + .name = "powerdown",
> + .read = stm32_dac_read_powerdown,
> + .write = stm32_dac_write_powerdown,
> + .shared = IIO_SEPARATE,
> + },
> + IIO_ENUM("powerdown_mode", IIO_SEPARATE, &stm32_dac_powerdown_mode_en),
> + IIO_ENUM_AVAILABLE("powerdown_mode", &stm32_dac_powerdown_mode_en),
> + {},
> +};
> +
> +#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_SCALE), \
> + /* scan_index is always 0 as num_channels is 1 */ \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + }, \
> + .datasheet_name = name, \
> + .ext_info = stm32_dac_ext_info \
> +}
> +
> +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, "reg", &channel);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Failed to read reg property\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];
> + /*
> + * Expose only one channel here, as they can be used independently,
> + * with separate trigger. Then separate IIO devices are instantiated
> + * to manage this.
> + */
> + 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;
> +
> + return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +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,
> + .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 v2 3/5] iio: trigger: add set_trigger callback to notify device
From: Jonathan Cameron @ 2017-04-09 8: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
In-Reply-To: <1491495116-7209-4-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 06/04/17 17:11, Fabrice Gasnier wrote:
> Add 'set_trigger' callback to iio info structure. This allows device
> to be notified when a trigger (or no trigger) has been assigned. This
> maybe useful for instance in non buffered mode (e.g. event triggered).
> This is called, after trigger and device side validate callbacks have
> been called.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Seems sensible to me...
J
> ---
> Changes in v2:
> - Added set_trigger callback used in remaining patches
> ---
> drivers/iio/industrialio-trigger.c | 6 ++++++
> include/linux/iio/iio.h | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index 978e1592..010bdf2 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -434,6 +434,12 @@ static ssize_t iio_trigger_write_current(struct device *dev,
> goto out_trigger_put;
> }
>
> + if (indio_dev->info->set_trigger) {
> + ret = indio_dev->info->set_trigger(indio_dev, trig);
> + if (ret)
> + goto out_trigger_put;
> + }
> +
> indio_dev->trig = trig;
>
> if (oldtrig) {
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 3f5ea2e..9f51065 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -399,6 +399,7 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> * @write_event_value: write a configuration value for the event.
> * @validate_trigger: function to validate the trigger when the
> * current trigger gets changed.
> + * @set_trigger: function to notify current trigger gets changed.
> * @update_scan_mode: function to configure device and scan buffer when
> * channels have changed
> * @debugfs_reg_access: function to read or write register value of device
> @@ -478,6 +479,7 @@ struct iio_info {
>
> int (*validate_trigger)(struct iio_dev *indio_dev,
> struct iio_trigger *trig);
> + int (*set_trigger)(struct iio_dev *indio_dev, struct iio_trigger *trig);
> int (*update_scan_mode)(struct iio_dev *indio_dev,
> const unsigned long *scan_mask);
> int (*debugfs_reg_access)(struct iio_dev *indio_dev,
>
^ permalink raw reply
* Re: [PATCH v2 4/5] iio: dac: stm32: add support for trigger events
From: Jonathan Cameron @ 2017-04-09 9:04 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
In-Reply-To: <1491495116-7209-5-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 06/04/17 17:11, 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. I'm really not sure on how to handle this... The problem to my mind
is knowing when it has triggered so we can know that it makes sense to
put the next reading in.... Anyhow bear with me...
I think the same arguement holds for this as equivalent input devices.
There we have always argued that if you need multichannel synchronized
capture (which is 'kind' of what we are looking at here) then you have
to use the buffered interface.
I think we need buffered output for this to fit nicely in the subsystem.
It definitely isn't a correct use of the event triggers.
That means we really need to figure out the ABI for buffered output and
get that sorted. To my mind it shouldn't be too tricky and should look
much like buffered input, just with us writing to a fifo from userspace
rather than reading from it.
The DMA side of that can come later, in a similar fashion to how it is
added for the ADC side of things. We can also have 'providers'
(equivalent of 'consumers' on the ADC side), perhaps giving a neat way
of describing DDS devices (I'm not so sure on this yet).
So to my mind, if you are not in buffered mode and do a sysfs write it
should be 'instant'. If in buffered mode, then it will wait on the
trigger.
So the complex side of things is what we 'know' about the data flow.
1) Case you have here. We want to do direct write through to the device,
but have no way of knowing (or do we?) that it has triggered and written
the data to the output. So we have no way of knowing we can push the next
value in from a fifo yet... In this case I guess the solution might be to
have a fifo length of 0. That is data flows straight to hardware.
2) Simple stream case - always enough data in the fifo and we get an interrupt
to signify that the previous trigger happened.
3) Case where we are only just keeping up. So we won't have data in the fifo
until sometime after the previous trigger. In this case we need the fifo to
push straight through if there isn't data ready to go.
4) Case where we are not pushing data fast enough. Just don't update?
That last case 4 is nasty as the reason we typically want to do triggered
DAC updates is to ensure we always have valid states in some control loop,
but we might get a race here where one DAC has a value ready to go on a trigger
and another one isn't quite ready. In this case we might want to hold off
until all are ready... So there might need to be a sanity check that everyone
on a given trigger is ready to go - an extra callback.
So a bit fiddly and I'm not sure I like the representation of through flow as
a fifo of 0 length... (can't think of a neater way though atm)
Anyhow, time to sort output buffers out once and for all I think if we can
get a reasonable group of people together who have the time.
Sorry Fabrice that this has hit your driver! Perhaps we can figure
enough out to be able to at least get the basics (i.e. patches 1,2) in as
asap.
Jonathan
> ---
> Changes in v2:
> - Fix issue with trigger, by using set_trigger callback
> - trigger can now be assigned, no matters powerdown state
> ---
> drivers/iio/dac/Kconfig | 3 +
> drivers/iio/dac/stm32-dac-core.h | 8 +++
> drivers/iio/dac/stm32-dac.c | 127 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 137 insertions(+), 1 deletion(-)
>
> 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 daf0993..e51a468 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,9 +34,16 @@
>
> /* 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 STM32H7_DAC_CR_HFSEL BIT(15)
> #define STM32_DAC_CR_EN2 BIT(16)
>
> +/* 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)
> * @regmap: DAC registers shared via regmap
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> index c0d993a..a7a078e 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>
> @@ -32,15 +36,113 @@
> #define STM32_DAC_CHANNEL_1 1
> #define STM32_DAC_CHANNEL_2 2
> #define STM32_DAC_IS_CHAN_1(ch) ((ch) & STM32_DAC_CHANNEL_1)
> +/* 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 */
Can we get here otherwise?
If not I'd prefer to either see an error on the other case
(perhaps simply return IRQ_NONE)
> + if (dac->swtrig) {
> + u32 swtrig;
> +
> + if (STM32_DAC_IS_CHAN_1(channel))
> + 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_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + int channel = indio_dev->channels[0].channel;
> + u32 shift = STM32_DAC_IS_CHAN_1(channel) ? 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 iio_dev *indio_dev, int channel)
> {
> struct stm32_dac *dac = iio_priv(indio_dev);
> @@ -167,6 +269,7 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
> static const struct iio_info stm32_dac_iio_info = {
> .read_raw = stm32_dac_read_raw,
> .write_raw = stm32_dac_write_raw,
> + .set_trigger = stm32_dac_set_trigger,
> .debugfs_reg_access = stm32_dac_debugfs_reg_access,
> .driver_module = THIS_MODULE,
> };
> @@ -326,7 +429,28 @@ static int stm32_dac_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - return devm_iio_device_register(&pdev->dev, 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;
> +}
> +
> +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;
> }
>
> static const struct of_device_id stm32_dac_of_match[] = {
> @@ -337,6 +461,7 @@ static int stm32_dac_probe(struct platform_device *pdev)
>
> 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,
>
^ permalink raw reply
* Re: [PATCH v2 5/5] iio: dac: stm32: add support for waveform generator
From: Jonathan Cameron @ 2017-04-09 9:34 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
In-Reply-To: <1491495116-7209-6-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 06/04/17 17:11, Fabrice Gasnier wrote:
> STM32 DAC has built-in noise or triangle waveform generator.
> - "wavetype" extended attribute selects noise or triangle.
> - "amplitude" extended attribute selects amplitude for waveform generator
>
> A DC offset can be added to waveform generator output. This can be done
> using out_voltage[1/2]_offset
>
> Waveform generator requires a trigger to be configured, to increment /
> decrement internal counter in case of triangle generator. Noise
> generator is a bit different, but also requires a trigger to generate
> samples.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Various bits inline. Mostly I think the blockers on this will be
making sure the ABI defined is generic enough to handle the more crazy
DDS chips out there... (basically the ones doing frequency modulation).
Jonathan
> ---
> Changes in v2:
> - use _offset parameter to add DC offset to waveform generator
Conceptually this offset is just the normal DAC output value (particularly
in the flat case) I guess we can paper over this by having the _raw
and this always have th same value, but it's a little inelegant.
Still people are going to expect _raw to control it when in DAC mode but
that makes limited sense in DDS mode.
Mind you nothing stops us defining all DDS channels as the sum of whatever
the DDS is doing and whatever is _raw is set to. Perhaps we tidy this up
purely through documentation. Think of the DDS as a modulation on top
of the DAC...
> - Rework ABI to better fit existing DDS ABI: use out_voltageY_wavetype,
> out_voltage_wavetype_available, out_voltageY_amplitude,
> out_voltage_amplitude_available
Hmm. I'm thinking those amplitude values aren't nice and don't fit well
with the more general ABI.
I suggested (but didn't really expand upon) having standard defined types
for each waveform then using scale to control the amplitude.
Is that something that might work here?
So say we have our triangle standard form having an amplitude of 1V Peak to
Peak. Then we can use scale to make it whatever we actually have in this
case? The docs for wave type will need to describe those standard forms
though.
Hmm. Whether this is worth doing is unclear however as we'll still have
to describe the 'frequency' in terms of the clock ticks (here the triggers)
So maybe amplitude is worth having. Again, looking for input from ADI lot
on this... There are some really fiddly cases to describe were we are doing
symbol encoding so have multiple waveforms that we are switching between based
on some external signal. Any ABI needs to encompass that sort of usage.
Parts like the AD9833 for example...
> - Better explain trigger usage in case of waveform generator.
> ---
> Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 | 16 +++
> drivers/iio/dac/stm32-dac-core.h | 4 +
> drivers/iio/dac/stm32-dac.c | 158 +++++++++++++++++++++-
> 3 files changed, 177 insertions(+), 1 deletion(-)
> 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..8f1fa009
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
Fair enough to initially introduced these for this part only, but I'd very
much like to see us agree on these sufficiently to get them into the main
docs asap so we have something to work with for getting the DDS chips out
of staging...
> @@ -0,0 +1,16 @@
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_wavetype
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_wavetype_available
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
> +Description:
> + List and/or select waveform generation provided by STM32 DAC:
> + - "flat": waveform generator disabled (default)
> + - "noise": select noise waveform
> + - "triangle": select triangle waveform
> +
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_amplitude
> +What: /sys/bus/iio/devices/iio:deviceX/out_voltage_amplitude_available
> +KernelVersion: 4.12
> +Contact: fabrice.gasnier-qxv4g6HH51o@public.gmane.org
> +Description:
> + List and/or select amplitude used for waveform generator
> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
> index e51a468..0f02975 100644
> --- a/drivers/iio/dac/stm32-dac-core.h
> +++ b/drivers/iio/dac/stm32-dac-core.h
> @@ -37,8 +37,12 @@
> #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)
> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
> index a7a078e..2ed75db 100644
> --- a/drivers/iio/dac/stm32-dac.c
> +++ b/drivers/iio/dac/stm32-dac.c
> @@ -42,10 +42,12 @@
> /**
> * struct stm32_dac - private data of DAC driver
> * @common: reference to DAC common data
> + * @wavetype: waveform generator
> * @swtrig: Using software trigger
> */
> struct stm32_dac {
> struct stm32_dac_common *common;
> + u32 wavetype;
> bool swtrig;
> };
>
> @@ -222,6 +224,29 @@ static int stm32_dac_set_value(struct stm32_dac *dac, int channel, int val)
> return ret;
> }
>
> +static int stm32_dac_get_offset(struct stm32_dac *dac, int channel, int *val)
> +{
> + int ret;
> +
> + /* Offset is only relevant in waveform generation mode. */
> + if (!dac->wavetype) {
> + *val = 0;
> + return IIO_VAL_INT;
> + }
> +
> + /*
> + * In waveform generation mode, DC offset in DHR is added to waveform
> + * generator output, then stored to DOR (data output register).
> + * Read offset from DHR.
> + */
Just thinking what fun we could have if we do the fifo based output to push
this that I was suggesting for the previous patch ;) triangles on top
of fun general waveforms..
> + if (STM32_DAC_IS_CHAN_1(channel))
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R1, val);
> + else
> + ret = regmap_read(dac->common->regmap, STM32_DAC_DHR12R2, val);
> +
> + return ret ? ret : IIO_VAL_INT;
> +}
> +
> static int stm32_dac_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -231,6 +256,8 @@ static int stm32_dac_read_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> return stm32_dac_get_value(dac, chan->channel, val);
> + case IIO_CHAN_INFO_OFFSET:
> + return stm32_dac_get_offset(dac, chan->channel, val);
> case IIO_CHAN_INFO_SCALE:
> *val = dac->common->vref_mv;
> *val2 = chan->scan_type.realbits;
> @@ -247,8 +274,16 @@ static int stm32_dac_write_raw(struct iio_dev *indio_dev,
> struct stm32_dac *dac = iio_priv(indio_dev);
>
> switch (mask) {
> + case IIO_CHAN_INFO_OFFSET:
> + /* Offset only makes sense in waveform generation mode */
> + if (dac->wavetype)
> + return stm32_dac_set_value(dac, chan->channel, val);
> + return -EBUSY;
Yeah, I think I sent you down a blind alley here. If people agree, lets
just define DDS signals as always being the sum of _raw + the dds element.
Then it's easy.
> case IIO_CHAN_INFO_RAW:
> - return stm32_dac_set_value(dac, chan->channel, val);
> + if (!dac->wavetype)
> + return stm32_dac_set_value(dac, chan->channel, val);
> + /* raw value is read only in waveform generation mode */
> + return -EBUSY;
> default:
> return -EINVAL;
> }
> @@ -334,6 +369,122 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
> .set = stm32_dac_set_powerdown_mode,
> };
>
> +/* waveform generator wave selection */
> +static const char * const stm32_dac_wavetype_desc[] = {
> + "flat",
> + "noise",
> + "triangle",
> +};
> +
> +static int stm32_dac_set_wavetype(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int wavetype)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 mask, val;
> + int ret;
> +
> + /*
> + * Waveform generator requires a trigger to be configured, to increment
> + * or decrement internal counter in case of triangle generator. Noise
> + * generator is a bit different, but also requires a trigger to
> + * generate samples.
> + */
> + if (wavetype && !indio_dev->trig)
> + dev_dbg(&indio_dev->dev, "Wavegen requires a trigger\n");
> +
> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
> + val = FIELD_PREP(STM32_DAC_CR_WAVE1, wavetype);
> + mask = STM32_DAC_CR_WAVE1;
> + } else {
> + val = FIELD_PREP(STM32_DAC_CR_WAVE2, wavetype);
> + mask = STM32_DAC_CR_WAVE2;
> + }
> +
> + ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
> + if (ret)
> + return ret;
> + dac->wavetype = wavetype;
> +
> + return 0;
> +}
> +
> +static int stm32_dac_get_wavetype(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
> + if (ret < 0)
> + return ret;
> +
> + if (STM32_DAC_IS_CHAN_1(chan->channel))
> + return FIELD_GET(STM32_DAC_CR_WAVE1, val);
> + else
> + return FIELD_GET(STM32_DAC_CR_WAVE2, val);
> +}
> +
> +static const struct iio_enum stm32_dac_wavetype_enum = {
> + .items = stm32_dac_wavetype_desc,
> + .num_items = ARRAY_SIZE(stm32_dac_wavetype_desc),
> + .get = stm32_dac_get_wavetype,
> + .set = stm32_dac_set_wavetype,
> +};
> +
> +/*
> + * waveform generator mamp selection: mask/amplitude
> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, [1:0]...)
This needs breaking out into two attributes - for noise it isn't amplitude in
any conventional sense... Keep the result device specific for now. I'm not
even sure what type of pseudo random source that actually is...
If anyone wants to figure it out it would be great to document it in a general
form!
> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
> + */
> +static const char * const stm32_dac_amplitude_desc[] = {
> + "1", "3", "7", "15", "31", "63", "127", "255", "511", "1023", "2047",
> + "4095",
> +};
> +
> +static int stm32_dac_set_amplitude(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int amplitude)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 mask, val;
> +
> + if (STM32_DAC_IS_CHAN_1(chan->channel)) {
> + val = FIELD_PREP(STM32_DAC_CR_MAMP1, amplitude);
> + mask = STM32_DAC_CR_MAMP1;
> + } else {
> + val = FIELD_PREP(STM32_DAC_CR_MAMP2, amplitude);
> + mask = STM32_DAC_CR_MAMP2;
> + }
> +
> + return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, mask, val);
> +}
> +
> +static int stm32_dac_get_amplitude(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct stm32_dac *dac = iio_priv(indio_dev);
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(dac->common->regmap, STM32_DAC_CR, &val);
> + if (ret < 0)
> + return ret;
> +
> + if (STM32_DAC_IS_CHAN_1(chan->channel))
> + return FIELD_GET(STM32_DAC_CR_MAMP1, val);
> + else
> + return FIELD_GET(STM32_DAC_CR_MAMP2, val);
> +}
> +
> +static const struct iio_enum stm32_dac_amplitude_enum = {
> + .items = stm32_dac_amplitude_desc,
> + .num_items = ARRAY_SIZE(stm32_dac_amplitude_desc),
> + .get = stm32_dac_get_amplitude,
> + .set = stm32_dac_set_amplitude,
> +};
> +
> static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
> {
> .name = "powerdown",
> @@ -343,6 +494,10 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
> },
> IIO_ENUM("powerdown_mode", IIO_SEPARATE, &stm32_dac_powerdown_mode_en),
> IIO_ENUM_AVAILABLE("powerdown_mode", &stm32_dac_powerdown_mode_en),
> + IIO_ENUM("wavetype", IIO_SEPARATE, &stm32_dac_wavetype_enum),
> + IIO_ENUM_AVAILABLE("wavetype", &stm32_dac_wavetype_enum),
> + IIO_ENUM("amplitude", IIO_SEPARATE, &stm32_dac_amplitude_enum),
> + IIO_ENUM_AVAILABLE("amplitude", &stm32_dac_amplitude_enum),
> {},
> };
>
> @@ -352,6 +507,7 @@ static ssize_t stm32_dac_write_powerdown(struct iio_dev *indio_dev,
> .output = 1, \
> .channel = chan, \
> .info_mask_separate = \
> + BIT(IIO_CHAN_INFO_OFFSET) | \
> BIT(IIO_CHAN_INFO_RAW) | \
> BIT(IIO_CHAN_INFO_SCALE), \
> /* scan_index is always 0 as num_channels is 1 */ \
>
^ permalink raw reply
* Re: [PATCH v2] iio: adc: add max1117/max1118/max1119 ADC driver
From: Jonathan Cameron @ 2017-04-09 10:06 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: <3e81cb76-90b9-841f-438b-b90752ad1901-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 02/04/17 11:23, Jonathan Cameron wrote:
> 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.
I did a bit of digging and found that Mark has applied the tests
for a zero length transfer - so presumably he is happy with them!
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Will be interesting to see if we get reports of controllers that
don't handle this correctly.
Thanks,
Jonathan
>
> 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 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 V3 1/3] dt-bindings: mtd: document linux,part-probe property
From: Boris Brezillon @ 2017-04-09 10:40 UTC (permalink / raw)
To: Rafał Miłecki
Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, Rob Herring, Mark Rutland, Frank Rowand,
Linus Walleij, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <20170331114016.26858-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Rafal,
On Fri, 31 Mar 2017 13:40:14 +0200
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> Support for this property has been introduced in 2010 with commit
> 9d5da3a9b849 ("mtd: extend physmap_of to let the device tree specify the
> parition probe") but it was never documented. Fix this by adding a
> proper description and example.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mtd/common.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
> index fc068b923d7a..1ada70e718b2 100644
> --- a/Documentation/devicetree/bindings/mtd/common.txt
> +++ b/Documentation/devicetree/bindings/mtd/common.txt
> @@ -6,10 +6,17 @@ Optional properties:
> controller based name) in order to ease flash device identification
> and/or describe what they are used for.
>
> +- linux,part-probe: if present, this property should contain a list of strings
> + with partition probes to be used for the flash device. A role of partition
> + probe (parser) is to read/construct partition table and register found
> + partitions. Getting partition table may be platform or device specific so
> + various devices may use various Linux drivers for this purpose.
> +
> Example:
>
> flash@0 {
> label = "System-firmware";
> + linux,part-probe = "cmdlinepart", "ofpart";
>
> /* flash type specific properties */
> };
--
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 V3 2/3] mtd: add core code reading DT specified part probes
From: Boris Brezillon @ 2017-04-09 11:04 UTC (permalink / raw)
To: Rafał Miłecki
Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, Rob Herring, Mark Rutland, Frank Rowand,
Linus Walleij, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <20170331114016.26858-2-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Rafal,
On Fri, 31 Mar 2017 13:40:15 +0200
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> Handling (creating) partitions for flash devices requires using a proper
> driver that will read partition table (out of somewhere). We can't
> simply try all existing drivers one by one:
> 1) It would increase boot time
> 2) The order could be a problem
> 3) In some corner cases parsers could misinterpret some data as a table
> Due to this MTD subsystem allows drivers to specify a list of applicable
> part probes.
>
> So far physmap_of was the only driver with support for linux,part-probe
> DT property. Other ones had list or probes hardcoded which wasn't making
> them really flexible. It prevented using common flash drivers on
> platforms that required some specific partition table access.
>
> This commit adds support for mentioned DT property directly to the MTD
> core. It's a rewritten implementation of physmap_of's code and it makes
> original code obsolete. Thanks to calling it on device parse
> registration (as suggested by Boris) all drivers gain support for it for
> free.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
> drivers/mtd/mtdcore.c | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 66a9dedd1062..917def28c756 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -664,6 +664,32 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
> }
> }
>
> +static const char * const *mtd_of_get_probes(struct device_node *np)
To be consistent with NAND DT helpers, can you put the of_get_ prefix
at the beginning: of_get_mtd_probes().
And get_probes() is a bit too generic IMHO, how about
of_get_mtd_part_probes() (or any other name clearly showing that this
is about partition parsers/probes).
> +{
> + const char **res;
> + int count;
> +
> + count = of_property_count_strings(np, "linux,part-probe");
> + if (count < 0)
> + return NULL;
> +
> + res = kzalloc((count + 1) * sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return NULL;
> +
> + count = of_property_read_string_array(np, "linux,part-probe", res,
> + count);
> + if (count < 0)
> + return NULL;
> +
> + return res;
> +}
> +
> +static inline void mtd_of_free_probes(const char * const *probes)
Drop the inline, the compiler is smart enough to decide by itself.
> +{
> + kfree(probes);
> +}
This is not really DT specific, and we might want to extract the list
of probes by other means in the future (cmdline, ACPI?).
How about declaring these 2 functions:
static char **mtd_alloc_part_type_table(int nentries)
{
return kzalloc((nentries + 1) * sizeof(*res), GFP_KERNEL);
}
static void mtd_free_part_type_table(const char * const *table)
{
kfree(table);
}
You can then use mtd_alloc_part_type_table() in
of_get_mtd_part_probes() to allocate your partition type table.
> +
> /**
> * mtd_device_parse_register - parse partitions and register an MTD device.
> *
> @@ -698,14 +724,19 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> const struct mtd_partition *parts,
> int nr_parts)
> {
> + const char * const *part_probe_types;
> struct mtd_partitions parsed;
> int ret;
>
> mtd_set_dev_defaults(mtd);
>
> + part_probe_types = mtd_of_get_probes(mtd_get_of_node(mtd));
> + if (!part_probe_types)
> + part_probe_types = types;
> +
How about doing it the other way around:
if (part_probe_types)
types = part_probe_types;
> memset(&parsed, 0, sizeof(parsed));
>
> - ret = parse_mtd_partitions(mtd, types, &parsed, parser_data);
> + ret = parse_mtd_partitions(mtd, part_probe_types, &parsed, parser_data);
this way you don't need this change
> if ((ret < 0 || parsed.nr_parts == 0) && parts && nr_parts) {
> /* Fall back to driver-provided partitions */
> parsed = (struct mtd_partitions){
> @@ -720,6 +751,9 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> memset(&parsed, 0, sizeof(parsed));
> }
>
> + if (part_probe_types != types)
> + mtd_of_free_probes(part_probe_types);
and here you simply have:
mtd_of_free_probes(part_probe_types);
> +
> ret = mtd_add_device_partitions(mtd, &parsed);
> if (ret)
> goto out;
--
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 V3 3/3] mtd: physmap_of: drop duplicated support for linux,part-probe property
From: Boris Brezillon @ 2017-04-09 11:04 UTC (permalink / raw)
To: Rafał Miłecki
Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Cyrille Pitchen, Rob Herring, Mark Rutland, Frank Rowand,
Linus Walleij, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki
In-Reply-To: <20170331114016.26858-3-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, 31 Mar 2017 13:40:16 +0200
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> Now support for linux,part-probe has been added to the MTD core there is
> no need to duplicate support for it in physmap_of.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> drivers/mtd/maps/physmap_of.c | 46 +------------------------------------------
> 1 file changed, 1 insertion(+), 45 deletions(-)
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 14e8909c9955..49dbb7235848 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -114,47 +114,9 @@ static struct mtd_info *obsolete_probe(struct platform_device *dev,
> static const char * const part_probe_types_def[] = {
> "cmdlinepart", "RedBoot", "ofpart", "ofoldpart", NULL };
>
> -static const char * const *of_get_probes(struct device_node *dp)
> -{
> - const char *cp;
> - int cplen;
> - unsigned int l;
> - unsigned int count;
> - const char **res;
> -
> - cp = of_get_property(dp, "linux,part-probe", &cplen);
> - if (cp == NULL)
> - return part_probe_types_def;
> -
> - count = 0;
> - for (l = 0; l != cplen; l++)
> - if (cp[l] == 0)
> - count++;
> -
> - res = kzalloc((count + 1)*sizeof(*res), GFP_KERNEL);
> - if (!res)
> - return NULL;
> - count = 0;
> - while (cplen > 0) {
> - res[count] = cp;
> - l = strlen(cp) + 1;
> - cp += l;
> - cplen -= l;
> - count++;
> - }
> - return res;
> -}
> -
> -static void of_free_probes(const char * const *probes)
> -{
> - if (probes != part_probe_types_def)
> - kfree(probes);
> -}
> -
> static const struct of_device_id of_flash_match[];
> static int of_flash_probe(struct platform_device *dev)
> {
> - const char * const *part_probe_types;
> const struct of_device_id *match;
> struct device_node *dp = dev->dev.of_node;
> struct resource res;
> @@ -320,14 +282,8 @@ static int of_flash_probe(struct platform_device *dev)
>
> info->cmtd->dev.parent = &dev->dev;
> mtd_set_of_node(info->cmtd, dp);
> - part_probe_types = of_get_probes(dp);
> - if (!part_probe_types) {
> - err = -ENOMEM;
> - goto err_out;
> - }
> - mtd_device_parse_register(info->cmtd, part_probe_types, NULL,
> + mtd_device_parse_register(info->cmtd, part_probe_types_def, NULL,
> NULL, 0);
> - of_free_probes(part_probe_types);
>
> kfree(mtd_list);
>
--
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 01/13] devicetree/bindings: display: Document common panel properties
From: Emil Velikov @ 2017-04-09 11:47 UTC (permalink / raw)
To: Laurent Pinchart
Cc: ML dri-devel, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree, Tomi Valkeinen
In-Reply-To: <1479526093-7014-2-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Hi Laurent,
Pardon for reviving this old thread. I've noticed a couple of things
which might want some attention.
On 19 November 2016 at 03:28, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> +
> +- panel-timing: Most display panels are restricted to a single resolution and
> + require specific display timings. The panel-timing subnode expresses those
> + timings as specified in the timing subnode section of the display timing
> + bindings defined in
> + Documentation/devicetree/bindings/display/display-timing.txt.
Cannot find such a file in linux-next. Perhaps you meant
Documentation/devicetree/bindings/display/panel/display-timing.txt?
Documentation/devicetree/bindings/display/panel/panel.txt includes a
"rotation" property, which we might want to fold here.
Regards,
Emil
--
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 1/2] leds: Add driver for Qualcomm LPG
From: Jacek Anaszewski @ 2017-04-09 12:32 UTC (permalink / raw)
To: Pavel Machek, Bjorn Andersson
Cc: Rob Herring, Richard Purdie, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170408133904.GA9020@amd>
Hi,
On 04/08/2017 03:39 PM, Pavel Machek wrote:
> Hi!
>
>> [..]
>>>> For the patterns I don't know how a trigger for this would look like,
>>>> how would setting the pattern of a trigger be propagated down to the
>>>> hardware?
>>>
>>> We'd need a new op and API similar to blink_set()/led_blink_set().
>>>
>>
>> I've tried to find different LED circuits with some sort of pattern
>> generator in an attempt to figure out how to design this interface, but
>> turned out to be quite hard to find examples; the three I can compare
>> are:
>>
>> * LP5xx series "implements" pattern generation by executing code.
>>
>> * Qualcomm LPG iterates over 2-64 brightness-values in a pattern, at a
>> fixed rate with knobs to configure what happens before starting and
>> after finishing iterating over the defined values. It does not support
>> smooth transitions between values.
>>
>> * AS3676 supports a pattern of 32 values controlling if the output
>> should be enabled or disabled for each 32.5ms (or 250ms) time period.
>> The delay before repeating the pattern can be configured. It support
>> smooth transitions between the states.
>>
>>
>> So, while I think I see how you would like to architect this interface I
>> am not sure how to figure out the details.
>>
>> The pattern definition would have to be expressive enough to support the
>> features of LP5xx and direct enough to support the limited AS3676. It
>> would likely have to express transitions, so that the LPG could generate
>> intermediate steps (and we will have to adapt the resolution of the
>> ramps based on the other LPGs in the system).
>>
>> How do we do with patterns that are implementable by the LP5xx but are
>> not with the LPG? Should we reject those or should we do some sort of
>> best-effort approach in the kernel?
>
> Lets say you get series of
>
> (red, green, blue, delta_t )
In order to make it possible we'd have to have a means for mapping
LED class devices to red, green and blue. In effect I see the problem
of introducing a new mechanism for creating compound LED class device
out of existing LED class devices as the first one to address.
Once we have compound LED class device, that would expose an interface
for operating on the particular color brightnesses, then we can build
upon it the pattern engine. Actually, the same compound LED mechanism
would be necessary for defining blink patterns for strings of monochrome
LEDs.
> points, meaning "in delta_t msec, change color to red, green,
> blue. Lets ignore other channels for now. delta_t of 0 would be step
> change. Would such interface work for you?
>
> Simple compiler from this to LP5XX code should not be hard to
> do. AS3676 ... I'm not sure what to do, AFAICT it is too limited.
Our new API for setting blink patterns could be defined so that it
does not guarantee setting the requested pattern, but applies only
what the hardware can support and returns the applied settings.
E.g. this way the driver could reduce the requested brightness
transition resolution.
There is also a question if we should provide software fallback
for patterns not supported by the hardware and to what extent.
In case of blink support for a single LED we do that but in case
of more complex patterns it would require more complex logic,
and at least for now I'd avoid it.
--
Best regards,
Jacek Anaszewski
--
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
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