On 06/11/2013 07:28 PM, Lars-Peter Clausen wrote: > On 06/11/2013 08:16 PM, Jonathan Cameron wrote: >> On 06/11/2013 05:56 PM, Lars-Peter Clausen wrote: >>> This patch adds support for the AD7303. The AD7303 is a simple 2 channel 8 bit >>> DAC with an SPI interface. >>> >> >> Do we need an explicit, 'Use the reference regulator' element in device tree >> /platform data? (I know this is done in existing drivers) Or would a simple >> lack of having one specified be sufficient? > > Yes, the problem is that if the regulator framework is disabled or > CONFIG_REGULATOR_DUMMY is set regulator_get() returns a valid (dummy) > regulator. So we can't distinguish between whether a regulator is present or > not in that case. But we need to configure the device according to whether > there is an external ref or not. Of course scale reporting won't work if the > regulator framework is disabled, but at least the device works still fine > otherwise. > >> Are there real cases where >> a board is designed with one, but we want to not use it (other than bugs >> of course ;) Doesn't seem to be documented. > > No, if you don't want to use it even though it is on the board just don't > specify it. E.g. in the devicetree case we use the presence of the property > whether to use the internal or external reference. > Fair enough and thanks for clarifying this. >> >> Otherwise, whether you put a blank line before return statements is a bit >> random in this driver. >> >> Rest is fine and I'm only bring the above up because its that sort of an evening ;) >>> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx> Applied to the togreg branch of iio.git Thanks, >>> --- >>> .../devicetree/bindings/iio/dac/ad7303.txt | 23 ++ >>> drivers/iio/dac/Kconfig | 10 + >>> drivers/iio/dac/Makefile | 1 + >>> drivers/iio/dac/ad7303.c | 315 +++++++++++++++++++++ >>> include/linux/platform_data/ad7303.h | 21 ++ >>> 5 files changed, 370 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/dac/ad7303.txt >>> create mode 100644 drivers/iio/dac/ad7303.c >>> create mode 100644 include/linux/platform_data/ad7303.h >>> >>> diff --git a/Documentation/devicetree/bindings/iio/dac/ad7303.txt b/Documentation/devicetree/bindings/iio/dac/ad7303.txt >>> new file mode 100644 >>> index 0000000..914610f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/dac/ad7303.txt >>> @@ -0,0 +1,23 @@ >>> +Analog Devices AD7303 DAC device driver >>> + >>> +Required properties: >>> + - compatible: Must be "adi,ad7303" >>> + - reg: SPI chip select number for the device >>> + - spi-max-frequency: Max SPI frequency to use (< 30000000) >>> + - Vdd-supply: Phandle to the Vdd power supply >>> + >>> +Optional properties: >>> + - REF-supply: Phandle to the external reference voltage supply. This should >>> + only be set if there is an external reference voltage connected to the REF >>> + pin. If the property is not set Vdd/2 is used as the reference voltage. >>> + >>> +Example: >>> + >>> + ad7303@4 { >>> + compatible = "adi,ad7303"; >>> + reg = <4>; >>> + spi-max-frequency = <10000000>; >>> + Vdd-supply = <&vdd_supply>; >>> + adi,use-external-reference; >> above is a I think undocumented? >> >>> + REF-supply = <&vref_supply>; >>> + }; >>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig >>> index b61160b..c9c33ce 100644 >>> --- a/drivers/iio/dac/Kconfig >>> +++ b/drivers/iio/dac/Kconfig >>> @@ -130,6 +130,16 @@ config AD5686 >>> To compile this driver as a module, choose M here: the >>> module will be called ad5686. >>> >>> +config AD7303 >>> + tristate "Analog Devices Analog Devices AD7303 DAC driver" >>> + depends on SPI >>> + help >>> + Say yes here to build support for Analog Devices AD7303 Digital to Analog >>> + Converters (DAC). >>> + >>> + To compile this driver as module choose M here: the module will be called >>> + ad7303. >>> + >>> config MAX517 >>> tristate "Maxim MAX517/518/519 DAC driver" >>> depends on I2C >>> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile >>> index 5b528eb..c8d7ab6 100644 >>> --- a/drivers/iio/dac/Makefile >>> +++ b/drivers/iio/dac/Makefile >>> @@ -14,5 +14,6 @@ obj-$(CONFIG_AD5755) += ad5755.o >>> obj-$(CONFIG_AD5764) += ad5764.o >>> obj-$(CONFIG_AD5791) += ad5791.o >>> obj-$(CONFIG_AD5686) += ad5686.o >>> +obj-$(CONFIG_AD7303) += ad7303.o >>> obj-$(CONFIG_MAX517) += max517.o >>> obj-$(CONFIG_MCP4725) += mcp4725.o >>> diff --git a/drivers/iio/dac/ad7303.c b/drivers/iio/dac/ad7303.c >>> new file mode 100644 >>> index 0000000..85aeef6 >>> --- /dev/null >>> +++ b/drivers/iio/dac/ad7303.c >>> @@ -0,0 +1,315 @@ >>> +/* >>> + * AD7303 Digital to analog converters driver >>> + * >>> + * Copyright 2013 Analog Devices Inc. >>> + * >>> + * Licensed under the GPL-2. >>> + */ >>> + >>> +#include <linux/err.h> >>> +#include <linux/module.h> >>> +#include <linux/kernel.h> >>> +#include <linux/spi/spi.h> >>> +#include <linux/slab.h> >>> +#include <linux/sysfs.h> >>> +#include <linux/regulator/consumer.h> >>> +#include <linux/of.h> >>> + >>> +#include <linux/iio/iio.h> >>> +#include <linux/iio/sysfs.h> >>> + >>> +#include <linux/platform_data/ad7303.h> >>> + >>> +#define AD7303_CFG_EXTERNAL_VREF BIT(15) >>> +#define AD7303_CFG_POWER_DOWN(ch) BIT(11 + (ch)) >>> +#define AD7303_CFG_ADDR_OFFSET 10 >>> + >>> +#define AD7303_CMD_UPDATE_DAC (0x3 << 8) >>> + >>> +/** >>> + * struct ad7303_state - driver instance specific data >>> + * @spi: the device for this driver instance >>> + * @config: cached config register value >>> + * @dac_cache: current DAC raw value (chip does not support readback) >>> + * @data: spi transfer buffer >>> + */ >>> + >>> +struct ad7303_state { >>> + struct spi_device *spi; >>> + uint16_t config; >>> + uint8_t dac_cache[2]; >>> + >>> + struct regulator *vdd_reg; >>> + struct regulator *vref_reg; >>> + >>> + /* >>> + * DMA (thus cache coherency maintenance) requires the >>> + * transfer buffers to live in their own cache lines. >>> + */ >>> + __be16 data ____cacheline_aligned; >>> +}; >>> + >>> +static int ad7303_write(struct ad7303_state *st, unsigned int chan, >>> + uint8_t val) >>> +{ >>> + st->data = cpu_to_be16(AD7303_CMD_UPDATE_DAC | >>> + (chan << AD7303_CFG_ADDR_OFFSET) | >>> + st->config | val); >>> + >>> + return spi_write(st->spi, &st->data, sizeof(st->data)); >>> +} >>> + >>> +static ssize_t ad7303_read_dac_powerdown(struct iio_dev *indio_dev, >>> + uintptr_t private, const struct iio_chan_spec *chan, char *buf) >>> +{ >>> + struct ad7303_state *st = iio_priv(indio_dev); >>> + >>> + return sprintf(buf, "%d\n", (bool)(st->config & >>> + AD7303_CFG_POWER_DOWN(chan->channel))); >>> +} >>> + >>> +static ssize_t ad7303_write_dac_powerdown(struct iio_dev *indio_dev, >>> + uintptr_t private, const struct iio_chan_spec *chan, const char *buf, >>> + size_t len) >>> +{ >>> + struct ad7303_state *st = iio_priv(indio_dev); >>> + bool pwr_down; >>> + int ret; >>> + >>> + ret = strtobool(buf, &pwr_down); >>> + if (ret) >>> + return ret; >>> + >>> + mutex_lock(&indio_dev->mlock); >>> + >>> + if (pwr_down) >>> + st->config |= AD7303_CFG_POWER_DOWN(chan->channel); >>> + else >>> + st->config &= ~AD7303_CFG_POWER_DOWN(chan->channel); >>> + >>> + /* There is no noop cmd which allows us to only update the powerdown >>> + * mode, so just write one of the DAC channels again */ >>> + ad7303_write(st, chan->channel, st->dac_cache[chan->channel]); >>> + >>> + mutex_unlock(&indio_dev->mlock); >>> + return ret ? ret : len; >>> +} >>> + >>> +static int ad7303_get_vref(struct ad7303_state *st, >>> + struct iio_chan_spec const *chan) >>> +{ >>> + int ret; >>> + >>> + if (st->config & AD7303_CFG_EXTERNAL_VREF) >>> + return regulator_get_voltage(st->vref_reg); >>> + >>> + ret = regulator_get_voltage(st->vdd_reg); >>> + if (ret < 0) >>> + return ret; >>> + return ret / 2; >>> +} >>> + >>> +static int ad7303_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, int *val, int *val2, long info) >>> +{ >>> + struct ad7303_state *st = iio_priv(indio_dev); >>> + int vref_uv; >>> + >>> + switch (info) { >>> + case IIO_CHAN_INFO_RAW: >>> + *val = st->dac_cache[chan->channel]; >>> + return IIO_VAL_INT; >>> + case IIO_CHAN_INFO_SCALE: >>> + vref_uv = ad7303_get_vref(st, chan); >>> + if (vref_uv < 0) >>> + return vref_uv; >>> + >>> + *val = 2 * vref_uv / 1000; >>> + *val2 = chan->scan_type.realbits; >>> + >>> + return IIO_VAL_FRACTIONAL_LOG2; >>> + default: >>> + break; >>> + } >>> + return -EINVAL; >>> +} >>> + >>> +static int ad7303_write_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, int val, int val2, long mask) >>> +{ >>> + struct ad7303_state *st = iio_priv(indio_dev); >>> + int ret; >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + if (val >= (1 << chan->scan_type.realbits) || val < 0) >>> + return -EINVAL; >>> + >>> + mutex_lock(&indio_dev->mlock); >>> + ret = ad7303_write(st, chan->address, val); >>> + if (ret == 0) >>> + st->dac_cache[chan->channel] = val; >>> + mutex_unlock(&indio_dev->mlock); >>> + break; >>> + default: >>> + ret = -EINVAL; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +static const struct iio_info ad7303_info = { >>> + .read_raw = ad7303_read_raw, >>> + .write_raw = ad7303_write_raw, >>> + .driver_module = THIS_MODULE, >>> +}; >>> + >>> +static const struct iio_chan_spec_ext_info ad7303_ext_info[] = { >>> + { >>> + .name = "powerdown", >>> + .read = ad7303_read_dac_powerdown, >>> + .write = ad7303_write_dac_powerdown, >>> + }, >>> + { }, >>> +}; >>> + >>> +#define AD7303_CHANNEL(chan) { \ >>> + .type = IIO_VOLTAGE, \ >>> + .indexed = 1, \ >>> + .output = 1, \ >>> + .channel = (chan), \ >>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ >>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ >>> + .address = (chan), \ >>> + .scan_type = { \ >>> + .sign = 'u', \ >>> + .realbits = '8', \ >>> + .storagebits = '8', \ >>> + .shift = '0', \ >>> + }, \ >>> + .ext_info = ad7303_ext_info, \ >>> +} >>> + >>> +static const struct iio_chan_spec ad7303_channels[] = { >>> + AD7303_CHANNEL(0), >>> + AD7303_CHANNEL(1), >>> +}; >>> + >>> +static int ad7303_probe(struct spi_device *spi) >>> +{ >>> + const struct spi_device_id *id = spi_get_device_id(spi); >>> + struct iio_dev *indio_dev; >>> + struct ad7303_state *st; >>> + bool ext_ref; >>> + int ret; >>> + >>> + indio_dev = iio_device_alloc(sizeof(*st)); >>> + if (indio_dev == NULL) >>> + return -ENOMEM; >>> + >>> + st = iio_priv(indio_dev); >>> + spi_set_drvdata(spi, indio_dev); >>> + >>> + st->spi = spi; >>> + >>> + st->vdd_reg = regulator_get(&spi->dev, "Vdd"); >>> + if (IS_ERR(st->vdd_reg)) { >>> + ret = PTR_ERR(st->vdd_reg); >>> + goto err_free; >>> + } >>> + >>> + ret = regulator_enable(st->vdd_reg); >>> + if (ret) >>> + goto err_put_vdd_reg; >>> + >>> + if (spi->dev.of_node) { >>> + ext_ref = of_property_read_bool(spi->dev.of_node, >>> + "REF-supply"); >>> + } else { >>> + struct ad7303_platform_data *pdata = spi->dev.platform_data; >>> + if (pdata && pdata->use_external_ref) >>> + ext_ref = true; >>> + else >>> + ext_ref = false; >>> + } >>> + >>> + if (ext_ref) { >>> + st->vref_reg = regulator_get(&spi->dev, "REF"); >>> + if (IS_ERR(st->vref_reg)) >>> + goto err_disable_vdd_reg; >>> + >>> + ret = regulator_enable(st->vref_reg); >>> + if (ret) >>> + goto err_put_vref_reg; >>> + >>> + st->config |= AD7303_CFG_EXTERNAL_VREF; >>> + } >>> + >>> + indio_dev->dev.parent = &spi->dev; >>> + indio_dev->name = id->name; >>> + indio_dev->info = &ad7303_info; >>> + indio_dev->modes = INDIO_DIRECT_MODE; >>> + indio_dev->channels = ad7303_channels; >>> + indio_dev->num_channels = ARRAY_SIZE(ad7303_channels); >>> + >>> + ret = iio_device_register(indio_dev); >>> + if (ret) >>> + goto err_disable_vref_reg; >>> + >>> + return 0; >>> + >>> +err_disable_vref_reg: >>> + if (st->vref_reg) >>> + regulator_disable(st->vref_reg); >>> +err_put_vref_reg: >>> + if (st->vref_reg) >>> + regulator_put(st->vref_reg); >>> +err_disable_vdd_reg: >>> + regulator_disable(st->vdd_reg); >>> +err_put_vdd_reg: >>> + regulator_put(st->vdd_reg); >>> +err_free: >>> + iio_device_free(indio_dev); >>> + >>> + return ret; >>> +} >>> + >>> +static int ad7303_remove(struct spi_device *spi) >>> +{ >>> + struct iio_dev *indio_dev = spi_get_drvdata(spi); >>> + struct ad7303_state *st = iio_priv(indio_dev); >>> + >>> + iio_device_unregister(indio_dev); >>> + >>> + if (st->vref_reg) { >>> + regulator_disable(st->vref_reg); >>> + regulator_put(st->vref_reg); >>> + } >>> + regulator_disable(st->vdd_reg); >>> + regulator_put(st->vdd_reg); >>> + >>> + iio_device_free(indio_dev); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct spi_device_id ad7303_spi_ids[] = { >>> + { "ad7303", 0 }, >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(spi, ad7303_spi_ids); >>> + >>> +static struct spi_driver ad7303_driver = { >>> + .driver = { >>> + .name = "ad7303", >>> + .owner = THIS_MODULE, >>> + }, >>> + .probe = ad7303_probe, >>> + .remove = ad7303_remove, >>> + .id_table = ad7303_spi_ids, >>> +}; >>> +module_spi_driver(ad7303_driver); >>> + >>> +MODULE_AUTHOR("Lars-Peter Clausen <lars@xxxxxxxxxx>"); >>> +MODULE_DESCRIPTION("Analog Devices AD7303 DAC driver"); >>> +MODULE_LICENSE("GPL v2"); >>> diff --git a/include/linux/platform_data/ad7303.h b/include/linux/platform_data/ad7303.h >>> new file mode 100644 >>> index 0000000..de6a7a6 >>> --- /dev/null >>> +++ b/include/linux/platform_data/ad7303.h >>> @@ -0,0 +1,21 @@ >>> +/* >>> + * Analog Devices AD7303 DAC driver >>> + * >>> + * Copyright 2013 Analog Devices Inc. >>> + * >>> + * Licensed under the GPL-2. >>> + */ >>> + >>> +#ifndef __IIO_ADC_AD7303_H__ >>> +#define __IIO_ADC_AD7303_H__ >>> + >>> +/** >>> + * struct ad7303_platform_data - AD7303 platform data >>> + * @use_external_ref: If set to true use an external voltage reference connected >>> + * to the REF pin, otherwise use the internal reference derived from Vdd. >>> + */ >>> +struct ad7303_platform_data { >>> + bool use_external_ref; >>> +}; >>> + >>> +#endif >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html