* [PATCH v2 1/4] iio: ad5755: add support for dt bindings @ 2016-02-03 14:31 ` Sean Nyekjaer 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: Sean Nyekjaer, devicetree-u79uwXL29TY76Z2rM5mHXA Still missing a way to provide default modes and settings for the driver from dt, but I see this as a start. Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> --- Changes since v1: - none drivers/iio/dac/ad5755.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c index bfb350a..e1b6e78 100644 --- a/drivers/iio/dac/ad5755.c +++ b/drivers/iio/dac/ad5755.c @@ -607,6 +607,16 @@ static const struct spi_device_id ad5755_id[] = { }; MODULE_DEVICE_TABLE(spi, ad5755_id); +static const struct of_device_id ad5755_of_match[] = { + { .compatible = "adi,ad5755" }, + { .compatible = "adi,ad5755-1" }, + { .compatible = "adi,ad5757" }, + { .compatible = "adi,ad5735" }, + { .compatible = "adi,ad5737" }, + { } +}; +MODULE_DEVICE_TABLE(of, ad5755_of_match); + static struct spi_driver ad5755_driver = { .driver = { .name = "ad5755", -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 1/4] iio: ad5755: add support for dt bindings @ 2016-02-03 14:31 ` Sean Nyekjaer 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio; +Cc: Sean Nyekjaer, devicetree Still missing a way to provide default modes and settings for the driver from dt, but I see this as a start. Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> --- Changes since v1: - none drivers/iio/dac/ad5755.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c index bfb350a..e1b6e78 100644 --- a/drivers/iio/dac/ad5755.c +++ b/drivers/iio/dac/ad5755.c @@ -607,6 +607,16 @@ static const struct spi_device_id ad5755_id[] = { }; MODULE_DEVICE_TABLE(spi, ad5755_id); +static const struct of_device_id ad5755_of_match[] = { + { .compatible = "adi,ad5755" }, + { .compatible = "adi,ad5755-1" }, + { .compatible = "adi,ad5757" }, + { .compatible = "adi,ad5735" }, + { .compatible = "adi,ad5737" }, + { } +}; +MODULE_DEVICE_TABLE(of, ad5755_of_match); + static struct spi_driver ad5755_driver = { .driver = { .name = "ad5755", -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1454509864-32285-1-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>]
* [PATCH v2 2/4] iio: ad5755: Add DT binding documentation 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-03 14:31 ` Sean Nyekjaer -1 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: Sean Nyekjaer, devicetree-u79uwXL29TY76Z2rM5mHXA Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> --- Changes since v1: - none .../devicetree/bindings/iio/dac/ad5755.txt | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt new file mode 100644 index 0000000..4c91583 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt @@ -0,0 +1,24 @@ +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver + +Required properties: + - compatible: Has to contain one of the following: + adi,ad5755 + adi,ad5755-1 + adi,ad5757 + adi,ad5735 + adi,ad5737 + + - reg: spi chip select number for the device + - spi-cpha or spi-cpol: is the only modes that is supported + +Recommended properties: + - spi-max-frequency: Definition as per + Documentation/devicetree/bindings/spi/spi-bus.txt + +Example: +dac@0 { + compatible = "adi,ad5755"; + reg = <0>; + spi-max-frequency = <1000000>; + spi-cpha; +}; -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] iio: ad5755: Add DT binding documentation @ 2016-02-03 14:31 ` Sean Nyekjaer 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio; +Cc: Sean Nyekjaer, devicetree Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> --- Changes since v1: - none .../devicetree/bindings/iio/dac/ad5755.txt | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt new file mode 100644 index 0000000..4c91583 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt @@ -0,0 +1,24 @@ +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver + +Required properties: + - compatible: Has to contain one of the following: + adi,ad5755 + adi,ad5755-1 + adi,ad5757 + adi,ad5735 + adi,ad5737 + + - reg: spi chip select number for the device + - spi-cpha or spi-cpol: is the only modes that is supported + +Recommended properties: + - spi-max-frequency: Definition as per + Documentation/devicetree/bindings/spi/spi-bus.txt + +Example: +dac@0 { + compatible = "adi,ad5755"; + reg = <0>; + spi-max-frequency = <1000000>; + spi-cpha; +}; -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1454509864-32285-2-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>]
* [PATCH v2 3/4] iio: ad5755: added full support for devicetree 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-03 14:31 ` Sean Nyekjaer -1 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: Sean Nyekjaer, devicetree-u79uwXL29TY76Z2rM5mHXA Devicetree can provide platform data Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> --- Changes since v1: - this patch added I'm adding dt bindings header file with the same info as, the platform data as the devicetree can't understand enums. Please give an idea to work around this :-) drivers/iio/dac/ad5755.c | 87 +++++++++++++++++++++++++++++++++++- include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 include/dt-bindings/iio/adi,ad5755.h diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c index e1b6e78..c1e2546 100644 --- a/drivers/iio/dac/ad5755.c +++ b/drivers/iio/dac/ad5755.c @@ -14,9 +14,11 @@ #include <linux/slab.h> #include <linux/sysfs.h> #include <linux/delay.h> +#include <linux/of.h> #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/platform_data/ad5755.h> +#include <dt-bindings/iio/adi,ad5755.h> #define AD5755_NUM_CHANNELS 4 @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = { }, }; +#ifdef CONFIG_OF +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct device_node *pp; + struct ad5755_platform_data *pdata; + unsigned int tmp; + unsigned int tmparray[3]; + int devnr; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->ext_dc_dc_compenstation_resistor = + of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor"); + + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) + pdata->dc_dc_phase = tmp; + else + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; + + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) + pdata->dc_dc_freq = tmp; + else + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; + + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) + pdata->dc_dc_maxv = tmp; + else + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; + + devnr = 0; + for_each_child_of_node(np, pp) { + if (devnr > AD5755_NUM_CHANNELS) { + dev_err(dev, "There is to many channels defined in DT\n"); + goto error_out; + } + + if (!of_property_read_u32(pp, "adi,mode", &tmp)) + pdata->dac[devnr].mode = tmp; + else + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; + + pdata->dac[devnr].ext_current_sense_resistor = + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); + + pdata->dac[devnr].enable_voltage_overrange = + of_property_read_bool(pp, "adi,enable_voltage_overrange"); + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { + pdata->dac[devnr].slew.enable = tmparray[0]; + pdata->dac[devnr].slew.rate = tmparray[1]; + pdata->dac[devnr].slew.step_size = tmparray[2]; + } else { + pdata->dac[devnr].slew.enable = false; + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; + } + + devnr++; + } + + return pdata; + +error_out: + devm_kfree(dev, pdata); + return NULL; +} +#else +static +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) +{ + return NULL; +} +#endif + static int ad5755_probe(struct spi_device *spi) { enum ad5755_type type = spi_get_device_id(spi)->driver_data; @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi) indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->num_channels = AD5755_NUM_CHANNELS; - if (!pdata) + if (spi->dev.of_node) + pdata = ad5755_parse_dt(&spi->dev); + else + pdata = spi->dev.platform_data; + + if (!pdata) { + dev_warn(&spi->dev, "no platform data? using default\n"); pdata = &ad5755_default_pdata; + } ret = ad5755_init_channels(indio_dev, pdata); if (ret) diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h new file mode 100644 index 0000000..117466b --- /dev/null +++ b/include/dt-bindings/iio/adi,ad5755.h @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only 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. + */ + +#ifndef _DT_BINDINGS_AD5755_H +#define _DT_BINDINGS_AD5755_H + +#define AD5755_MODE_VOLTAGE_0V_5V 0 +#define AD5755_MODE_VOLTAGE_0V_10V 1 +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V 2 +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V 3 +#define AD5755_MODE_CURRENT_4mA_20mA 4 +#define AD5755_MODE_CURRENT_0mA_20mA 5 +#define AD5755_MODE_CURRENT_0mA_24mA 6 + +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE 0 +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE 1 +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE 2 +#define AD5755_DC_DC_PHASE_90_DEGREE 3 + +#define AD5755_DC_DC_FREQ_250kHZ 0 +#define AD5755_DC_DC_FREQ_410kHZ 1 +#define AD5755_DC_DC_FREQ_650kHZ 2 + +#define AD5755_DC_DC_MAXV_23V 0 +#define AD5755_DC_DC_MAXV_24V5 1 +#define AD5755_DC_DC_MAXV_27V 2 +#define AD5755_DC_DC_MAXV_29V5 3 + +#define AD5755_SLEW_RATE_64k 0 +#define AD5755_SLEW_RATE_32k 1 +#define AD5755_SLEW_RATE_16k 2 +#define AD5755_SLEW_RATE_8k 3 +#define AD5755_SLEW_RATE_4k 4 +#define AD5755_SLEW_RATE_2k 5 +#define AD5755_SLEW_RATE_1k 6 +#define AD5755_SLEW_RATE_500 7 +#define AD5755_SLEW_RATE_250 8 +#define AD5755_SLEW_RATE_125 9 +#define AD5755_SLEW_RATE_64 10 +#define AD5755_SLEW_RATE_32 11 +#define AD5755_SLEW_RATE_16 12 +#define AD5755_SLEW_RATE_8 13 +#define AD5755_SLEW_RATE_4 14 +#define AD5755_SLEW_RATE_0_5 15 + +#define AD5755_SLEW_STEP_SIZE_1 0 +#define AD5755_SLEW_STEP_SIZE_2 1 +#define AD5755_SLEW_STEP_SIZE_4 2 +#define AD5755_SLEW_STEP_SIZE_8 3 +#define AD5755_SLEW_STEP_SIZE_16 4 +#define AD5755_SLEW_STEP_SIZE_32 5 +#define AD5755_SLEW_STEP_SIZE_64 6 +#define AD5755_SLEW_STEP_SIZE_128 7 +#define AD5755_SLEW_STEP_SIZE_256 8 + +#endif /* _DT_BINDINGS_AD5755_H */ -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] iio: ad5755: added full support for devicetree @ 2016-02-03 14:31 ` Sean Nyekjaer 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio; +Cc: Sean Nyekjaer, devicetree Devicetree can provide platform data Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> --- Changes since v1: - this patch added I'm adding dt bindings header file with the same info as, the platform data as the devicetree can't understand enums. Please give an idea to work around this :-) drivers/iio/dac/ad5755.c | 87 +++++++++++++++++++++++++++++++++++- include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 include/dt-bindings/iio/adi,ad5755.h diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c index e1b6e78..c1e2546 100644 --- a/drivers/iio/dac/ad5755.c +++ b/drivers/iio/dac/ad5755.c @@ -14,9 +14,11 @@ #include <linux/slab.h> #include <linux/sysfs.h> #include <linux/delay.h> +#include <linux/of.h> #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/platform_data/ad5755.h> +#include <dt-bindings/iio/adi,ad5755.h> #define AD5755_NUM_CHANNELS 4 @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = { }, }; +#ifdef CONFIG_OF +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct device_node *pp; + struct ad5755_platform_data *pdata; + unsigned int tmp; + unsigned int tmparray[3]; + int devnr; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata->ext_dc_dc_compenstation_resistor = + of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor"); + + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) + pdata->dc_dc_phase = tmp; + else + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; + + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) + pdata->dc_dc_freq = tmp; + else + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; + + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) + pdata->dc_dc_maxv = tmp; + else + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; + + devnr = 0; + for_each_child_of_node(np, pp) { + if (devnr > AD5755_NUM_CHANNELS) { + dev_err(dev, "There is to many channels defined in DT\n"); + goto error_out; + } + + if (!of_property_read_u32(pp, "adi,mode", &tmp)) + pdata->dac[devnr].mode = tmp; + else + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; + + pdata->dac[devnr].ext_current_sense_resistor = + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); + + pdata->dac[devnr].enable_voltage_overrange = + of_property_read_bool(pp, "adi,enable_voltage_overrange"); + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { + pdata->dac[devnr].slew.enable = tmparray[0]; + pdata->dac[devnr].slew.rate = tmparray[1]; + pdata->dac[devnr].slew.step_size = tmparray[2]; + } else { + pdata->dac[devnr].slew.enable = false; + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; + } + + devnr++; + } + + return pdata; + +error_out: + devm_kfree(dev, pdata); + return NULL; +} +#else +static +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) +{ + return NULL; +} +#endif + static int ad5755_probe(struct spi_device *spi) { enum ad5755_type type = spi_get_device_id(spi)->driver_data; @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi) indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->num_channels = AD5755_NUM_CHANNELS; - if (!pdata) + if (spi->dev.of_node) + pdata = ad5755_parse_dt(&spi->dev); + else + pdata = spi->dev.platform_data; + + if (!pdata) { + dev_warn(&spi->dev, "no platform data? using default\n"); pdata = &ad5755_default_pdata; + } ret = ad5755_init_channels(indio_dev, pdata); if (ret) diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h new file mode 100644 index 0000000..117466b --- /dev/null +++ b/include/dt-bindings/iio/adi,ad5755.h @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only 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. + */ + +#ifndef _DT_BINDINGS_AD5755_H +#define _DT_BINDINGS_AD5755_H + +#define AD5755_MODE_VOLTAGE_0V_5V 0 +#define AD5755_MODE_VOLTAGE_0V_10V 1 +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V 2 +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V 3 +#define AD5755_MODE_CURRENT_4mA_20mA 4 +#define AD5755_MODE_CURRENT_0mA_20mA 5 +#define AD5755_MODE_CURRENT_0mA_24mA 6 + +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE 0 +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE 1 +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE 2 +#define AD5755_DC_DC_PHASE_90_DEGREE 3 + +#define AD5755_DC_DC_FREQ_250kHZ 0 +#define AD5755_DC_DC_FREQ_410kHZ 1 +#define AD5755_DC_DC_FREQ_650kHZ 2 + +#define AD5755_DC_DC_MAXV_23V 0 +#define AD5755_DC_DC_MAXV_24V5 1 +#define AD5755_DC_DC_MAXV_27V 2 +#define AD5755_DC_DC_MAXV_29V5 3 + +#define AD5755_SLEW_RATE_64k 0 +#define AD5755_SLEW_RATE_32k 1 +#define AD5755_SLEW_RATE_16k 2 +#define AD5755_SLEW_RATE_8k 3 +#define AD5755_SLEW_RATE_4k 4 +#define AD5755_SLEW_RATE_2k 5 +#define AD5755_SLEW_RATE_1k 6 +#define AD5755_SLEW_RATE_500 7 +#define AD5755_SLEW_RATE_250 8 +#define AD5755_SLEW_RATE_125 9 +#define AD5755_SLEW_RATE_64 10 +#define AD5755_SLEW_RATE_32 11 +#define AD5755_SLEW_RATE_16 12 +#define AD5755_SLEW_RATE_8 13 +#define AD5755_SLEW_RATE_4 14 +#define AD5755_SLEW_RATE_0_5 15 + +#define AD5755_SLEW_STEP_SIZE_1 0 +#define AD5755_SLEW_STEP_SIZE_2 1 +#define AD5755_SLEW_STEP_SIZE_4 2 +#define AD5755_SLEW_STEP_SIZE_8 3 +#define AD5755_SLEW_STEP_SIZE_16 4 +#define AD5755_SLEW_STEP_SIZE_32 5 +#define AD5755_SLEW_STEP_SIZE_64 6 +#define AD5755_SLEW_STEP_SIZE_128 7 +#define AD5755_SLEW_STEP_SIZE_256 8 + +#endif /* _DT_BINDINGS_AD5755_H */ -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1454509864-32285-3-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>]
* [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-03 14:31 ` Sean Nyekjaer -1 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: Sean Nyekjaer, devicetree-u79uwXL29TY76Z2rM5mHXA Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> --- Changes since v1: - this patch added .../devicetree/bindings/iio/dac/ad5755.txt | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt index 4c91583..3aac20d 100644 --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt @@ -15,10 +15,93 @@ Recommended properties: - spi-max-frequency: Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt +Optional properties: +See include/dt-bindings/iio/ad5755.h + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor + - adi,dc_dc_phase: can be set to + AD5755_DC_DC_PHASE_ALL_SAME_EDGE + AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE + AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE + AD5755_DC_DC_PHASE_90_DEGREE + - adi,dc_dc_freq: + AD5755_DC_DC_FREQ_250kHZ + AD5755_DC_DC_FREQ_410kHZ + AD5755_DC_DC_FREQ_650kHZ + - adi,dc_dc_maxv: + AD5755_DC_DC_MAXV_23V + AD5755_DC_DC_MAXV_24V5 + AD5755_DC_DC_MAXV_27V + AD5755_DC_DC_MAXV_29V5 + +Optional for every channel: + - adi,mode: + AD5755_MODE_VOLTAGE_0V_5V + AD5755_MODE_VOLTAGE_0V_10V + AD5755_MODE_VOLTAGE_PLUSMINUS_5V + AD5755_MODE_VOLTAGE_PLUSMINUS_10V + AD5755_MODE_CURRENT_4mA_20mA + AD5755_MODE_CURRENT_0mA_20mA + AD5755_MODE_CURRENT_0mA_24mA + - adi,ext_current_sense_resistor: boolean set if using external + current sense resistor + - adi,enable_voltage_overrange: boolean enable voltage overrange + - adi,slew: Array of slewrate settings should contain 3 fields: + 1: Should be either 0 or 1 in order to enable or disable slew + 2: Slew rate settings: + AD5755_SLEW_RATE_64k + AD5755_SLEW_RATE_32k + AD5755_SLEW_RATE_16k + AD5755_SLEW_RATE_8k + AD5755_SLEW_RATE_4k + AD5755_SLEW_RATE_2k + AD5755_SLEW_RATE_1k + AD5755_SLEW_RATE_500 + AD5755_SLEW_RATE_250 + AD5755_SLEW_RATE_125 + AD5755_SLEW_RATE_64 + AD5755_SLEW_RATE_32 + AD5755_SLEW_RATE_16 + AD5755_SLEW_RATE_8 + AD5755_SLEW_RATE_4 + AD5755_SLEW_RATE_0_5 + 3: Slew step size: + AD5755_SLEW_STEP_SIZE_1 + AD5755_SLEW_STEP_SIZE_2 + AD5755_SLEW_STEP_SIZE_4 + AD5755_SLEW_STEP_SIZE_8 + AD5755_SLEW_STEP_SIZE_16 + AD5755_SLEW_STEP_SIZE_32 + AD5755_SLEW_STEP_SIZE_64 + AD5755_SLEW_STEP_SIZE_128 + AD5755_SLEW_STEP_SIZE_256 + Example: dac@0 { compatible = "adi,ad5755"; reg = <0>; spi-max-frequency = <1000000>; spi-cpha; + adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>; + adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>; + adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>; + channel@0 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@1 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@2 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@3 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; }; -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation @ 2016-02-03 14:31 ` Sean Nyekjaer 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjaer @ 2016-02-03 14:31 UTC (permalink / raw) To: linux-iio; +Cc: Sean Nyekjaer, devicetree Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> --- Changes since v1: - this patch added .../devicetree/bindings/iio/dac/ad5755.txt | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt index 4c91583..3aac20d 100644 --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt @@ -15,10 +15,93 @@ Recommended properties: - spi-max-frequency: Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt +Optional properties: +See include/dt-bindings/iio/ad5755.h + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor + - adi,dc_dc_phase: can be set to + AD5755_DC_DC_PHASE_ALL_SAME_EDGE + AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE + AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE + AD5755_DC_DC_PHASE_90_DEGREE + - adi,dc_dc_freq: + AD5755_DC_DC_FREQ_250kHZ + AD5755_DC_DC_FREQ_410kHZ + AD5755_DC_DC_FREQ_650kHZ + - adi,dc_dc_maxv: + AD5755_DC_DC_MAXV_23V + AD5755_DC_DC_MAXV_24V5 + AD5755_DC_DC_MAXV_27V + AD5755_DC_DC_MAXV_29V5 + +Optional for every channel: + - adi,mode: + AD5755_MODE_VOLTAGE_0V_5V + AD5755_MODE_VOLTAGE_0V_10V + AD5755_MODE_VOLTAGE_PLUSMINUS_5V + AD5755_MODE_VOLTAGE_PLUSMINUS_10V + AD5755_MODE_CURRENT_4mA_20mA + AD5755_MODE_CURRENT_0mA_20mA + AD5755_MODE_CURRENT_0mA_24mA + - adi,ext_current_sense_resistor: boolean set if using external + current sense resistor + - adi,enable_voltage_overrange: boolean enable voltage overrange + - adi,slew: Array of slewrate settings should contain 3 fields: + 1: Should be either 0 or 1 in order to enable or disable slew + 2: Slew rate settings: + AD5755_SLEW_RATE_64k + AD5755_SLEW_RATE_32k + AD5755_SLEW_RATE_16k + AD5755_SLEW_RATE_8k + AD5755_SLEW_RATE_4k + AD5755_SLEW_RATE_2k + AD5755_SLEW_RATE_1k + AD5755_SLEW_RATE_500 + AD5755_SLEW_RATE_250 + AD5755_SLEW_RATE_125 + AD5755_SLEW_RATE_64 + AD5755_SLEW_RATE_32 + AD5755_SLEW_RATE_16 + AD5755_SLEW_RATE_8 + AD5755_SLEW_RATE_4 + AD5755_SLEW_RATE_0_5 + 3: Slew step size: + AD5755_SLEW_STEP_SIZE_1 + AD5755_SLEW_STEP_SIZE_2 + AD5755_SLEW_STEP_SIZE_4 + AD5755_SLEW_STEP_SIZE_8 + AD5755_SLEW_STEP_SIZE_16 + AD5755_SLEW_STEP_SIZE_32 + AD5755_SLEW_STEP_SIZE_64 + AD5755_SLEW_STEP_SIZE_128 + AD5755_SLEW_STEP_SIZE_256 + Example: dac@0 { compatible = "adi,ad5755"; reg = <0>; spi-max-frequency = <1000000>; spi-cpha; + adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>; + adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>; + adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>; + channel@0 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@1 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@2 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; + channel@3 { + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; + adi,ext_current_sense_resistor; + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; + }; }; -- 2.7.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
[parent not found: <1454509864-32285-4-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>]
* Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-06 18:30 ` Jonathan Cameron -1 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2016-02-06 18:30 UTC (permalink / raw) To: Sean Nyekjaer, linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Lars-Peter Clausen On 03/02/16 14:31, Sean Nyekjaer wrote: > Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> Hmm. Complex beast. Going to want Lars to look at this in particular. Also a device tree review would be good on this one. My personal gut feeling is not to wrap up things that are really numeric with names, but rather to provide docs on the valid values for them. Not sure what is the preferred by the device tree guys?? Jonathan > --- > Changes since v1: > - this patch added > > .../devicetree/bindings/iio/dac/ad5755.txt | 83 ++++++++++++++++++++++ > 1 file changed, 83 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > index 4c91583..3aac20d 100644 > --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt > +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > @@ -15,10 +15,93 @@ Recommended properties: > - spi-max-frequency: Definition as per > Documentation/devicetree/bindings/spi/spi-bus.txt > > +Optional properties: > +See include/dt-bindings/iio/ad5755.h > + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor > + - adi,dc_dc_phase: can be set to > + AD5755_DC_DC_PHASE_ALL_SAME_EDGE > + AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE > + AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE > + AD5755_DC_DC_PHASE_90_DEGREE > + - adi,dc_dc_freq: > + AD5755_DC_DC_FREQ_250kHZ > + AD5755_DC_DC_FREQ_410kHZ > + AD5755_DC_DC_FREQ_650kHZ Can we not just make this numeric with a set of valid values specified in this doc? > + - adi,dc_dc_maxv: > + AD5755_DC_DC_MAXV_23V > + AD5755_DC_DC_MAXV_24V5 > + AD5755_DC_DC_MAXV_27V > + AD5755_DC_DC_MAXV_29V5 Same on this. > + > +Optional for every channel: > + - adi,mode: > + AD5755_MODE_VOLTAGE_0V_5V > + AD5755_MODE_VOLTAGE_0V_10V > + AD5755_MODE_VOLTAGE_PLUSMINUS_5V > + AD5755_MODE_VOLTAGE_PLUSMINUS_10V > + AD5755_MODE_CURRENT_4mA_20mA > + AD5755_MODE_CURRENT_0mA_20mA > + AD5755_MODE_CURRENT_0mA_24mA Again, could break this into two numeric values and have options specified in the doc + reject any invalid ones. > + - adi,ext_current_sense_resistor: boolean set if using external > + current sense resistor > + - adi,enable_voltage_overrange: boolean enable voltage overrange > + - adi,slew: Array of slewrate settings should contain 3 fields: > + 1: Should be either 0 or 1 in order to enable or disable slew > + 2: Slew rate settings: > + AD5755_SLEW_RATE_64k > + AD5755_SLEW_RATE_32k > + AD5755_SLEW_RATE_16k > + AD5755_SLEW_RATE_8k > + AD5755_SLEW_RATE_4k > + AD5755_SLEW_RATE_2k > + AD5755_SLEW_RATE_1k > + AD5755_SLEW_RATE_500 > + AD5755_SLEW_RATE_250 > + AD5755_SLEW_RATE_125 > + AD5755_SLEW_RATE_64 > + AD5755_SLEW_RATE_32 > + AD5755_SLEW_RATE_16 > + AD5755_SLEW_RATE_8 > + AD5755_SLEW_RATE_4 > + AD5755_SLEW_RATE_0_5 > + 3: Slew step size: > + AD5755_SLEW_STEP_SIZE_1 > + AD5755_SLEW_STEP_SIZE_2 > + AD5755_SLEW_STEP_SIZE_4 > + AD5755_SLEW_STEP_SIZE_8 > + AD5755_SLEW_STEP_SIZE_16 > + AD5755_SLEW_STEP_SIZE_32 > + AD5755_SLEW_STEP_SIZE_64 > + AD5755_SLEW_STEP_SIZE_128 > + AD5755_SLEW_STEP_SIZE_256 Same with these. > + > Example: > dac@0 { > compatible = "adi,ad5755"; > reg = <0>; > spi-max-frequency = <1000000>; > spi-cpha; > + adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>; > + adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>; > + adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>; > + channel@0 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@1 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@2 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@3 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > }; > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation @ 2016-02-06 18:30 ` Jonathan Cameron 0 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2016-02-06 18:30 UTC (permalink / raw) To: Sean Nyekjaer, linux-iio Cc: devicetree, Lars-Peter Clausen, Lars-Peter Clausen, Hartmut Knaack, Daniel Baluta On 03/02/16 14:31, Sean Nyekjaer wrote: > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> Hmm. Complex beast. Going to want Lars to look at this in particular. Also a device tree review would be good on this one. My personal gut feeling is not to wrap up things that are really numeric with names, but rather to provide docs on the valid values for them. Not sure what is the preferred by the device tree guys?? Jonathan > --- > Changes since v1: > - this patch added > > .../devicetree/bindings/iio/dac/ad5755.txt | 83 ++++++++++++++++++++++ > 1 file changed, 83 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > index 4c91583..3aac20d 100644 > --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt > +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > @@ -15,10 +15,93 @@ Recommended properties: > - spi-max-frequency: Definition as per > Documentation/devicetree/bindings/spi/spi-bus.txt > > +Optional properties: > +See include/dt-bindings/iio/ad5755.h > + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor > + - adi,dc_dc_phase: can be set to > + AD5755_DC_DC_PHASE_ALL_SAME_EDGE > + AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE > + AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE > + AD5755_DC_DC_PHASE_90_DEGREE > + - adi,dc_dc_freq: > + AD5755_DC_DC_FREQ_250kHZ > + AD5755_DC_DC_FREQ_410kHZ > + AD5755_DC_DC_FREQ_650kHZ Can we not just make this numeric with a set of valid values specified in this doc? > + - adi,dc_dc_maxv: > + AD5755_DC_DC_MAXV_23V > + AD5755_DC_DC_MAXV_24V5 > + AD5755_DC_DC_MAXV_27V > + AD5755_DC_DC_MAXV_29V5 Same on this. > + > +Optional for every channel: > + - adi,mode: > + AD5755_MODE_VOLTAGE_0V_5V > + AD5755_MODE_VOLTAGE_0V_10V > + AD5755_MODE_VOLTAGE_PLUSMINUS_5V > + AD5755_MODE_VOLTAGE_PLUSMINUS_10V > + AD5755_MODE_CURRENT_4mA_20mA > + AD5755_MODE_CURRENT_0mA_20mA > + AD5755_MODE_CURRENT_0mA_24mA Again, could break this into two numeric values and have options specified in the doc + reject any invalid ones. > + - adi,ext_current_sense_resistor: boolean set if using external > + current sense resistor > + - adi,enable_voltage_overrange: boolean enable voltage overrange > + - adi,slew: Array of slewrate settings should contain 3 fields: > + 1: Should be either 0 or 1 in order to enable or disable slew > + 2: Slew rate settings: > + AD5755_SLEW_RATE_64k > + AD5755_SLEW_RATE_32k > + AD5755_SLEW_RATE_16k > + AD5755_SLEW_RATE_8k > + AD5755_SLEW_RATE_4k > + AD5755_SLEW_RATE_2k > + AD5755_SLEW_RATE_1k > + AD5755_SLEW_RATE_500 > + AD5755_SLEW_RATE_250 > + AD5755_SLEW_RATE_125 > + AD5755_SLEW_RATE_64 > + AD5755_SLEW_RATE_32 > + AD5755_SLEW_RATE_16 > + AD5755_SLEW_RATE_8 > + AD5755_SLEW_RATE_4 > + AD5755_SLEW_RATE_0_5 > + 3: Slew step size: > + AD5755_SLEW_STEP_SIZE_1 > + AD5755_SLEW_STEP_SIZE_2 > + AD5755_SLEW_STEP_SIZE_4 > + AD5755_SLEW_STEP_SIZE_8 > + AD5755_SLEW_STEP_SIZE_16 > + AD5755_SLEW_STEP_SIZE_32 > + AD5755_SLEW_STEP_SIZE_64 > + AD5755_SLEW_STEP_SIZE_128 > + AD5755_SLEW_STEP_SIZE_256 Same with these. > + > Example: > dac@0 { > compatible = "adi,ad5755"; > reg = <0>; > spi-max-frequency = <1000000>; > spi-cpha; > + adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>; > + adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>; > + adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>; > + channel@0 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@1 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@2 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > + channel@3 { > + adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>; > + adi,ext_current_sense_resistor; > + adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>; > + }; > }; > ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <56B63BAC.2040001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation 2016-02-06 18:30 ` Jonathan Cameron @ 2016-02-08 17:37 ` Rob Herring -1 siblings, 0 replies; 20+ messages in thread From: Rob Herring @ 2016-02-08 17:37 UTC (permalink / raw) To: Jonathan Cameron Cc: Sean Nyekjaer, linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Lars-Peter Clausen, Hartmut Knaack, Daniel Baluta On Sat, Feb 06, 2016 at 06:30:04PM +0000, Jonathan Cameron wrote: > On 03/02/16 14:31, Sean Nyekjaer wrote: > > Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> > Hmm. Complex beast. Going to want Lars to look at this in particular. > Also a device tree review would be good on this one. > > My personal gut feeling is not to wrap up things that are really numeric > with names, but rather to provide docs on the valid values for them. > Not sure what is the preferred by the device tree guys?? Agreed. I'm not such a fan of defines in DT. Also, if there is some possibility that bindings could be generic, I prefer to see them use the native units rather than the enumerated register values. The downside is adding the translation into the driver. These look like they could possibly be generic DAC properties. Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation @ 2016-02-08 17:37 ` Rob Herring 0 siblings, 0 replies; 20+ messages in thread From: Rob Herring @ 2016-02-08 17:37 UTC (permalink / raw) To: Jonathan Cameron Cc: Sean Nyekjaer, linux-iio, devicetree, Lars-Peter Clausen, Hartmut Knaack, Daniel Baluta On Sat, Feb 06, 2016 at 06:30:04PM +0000, Jonathan Cameron wrote: > On 03/02/16 14:31, Sean Nyekjaer wrote: > > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> > Hmm. Complex beast. Going to want Lars to look at this in particular. > Also a device tree review would be good on this one. > > My personal gut feeling is not to wrap up things that are really numeric > with names, but rather to provide docs on the valid values for them. > Not sure what is the preferred by the device tree guys?? Agreed. I'm not such a fan of defines in DT. Also, if there is some possibility that bindings could be generic, I prefer to see them use the native units rather than the enumerated register values. The downside is adding the translation into the driver. These look like they could possibly be generic DAC properties. Rob ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-09 8:22 ` Sean Nyekjær -1 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjær @ 2016-02-09 8:22 UTC (permalink / raw) To: linux-iio-u79uwXL29TY76Z2rM5mHXA, Lars-Peter Clausen Cc: devicetree-u79uwXL29TY76Z2rM5mHXA On 2016-02-03 15:31, Sean Nyekjaer wrote: > Devicetree can provide platform data > > Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> Lars do you have time to look at this before i'm fixing the comments from Jonathan and Rob :-) Sean > --- > Changes since v1: > - this patch added > > I'm adding dt bindings header file with the same info as, the platform data as > the devicetree can't understand enums. > Please give an idea to work around this :-) > > drivers/iio/dac/ad5755.c | 87 +++++++++++++++++++++++++++++++++++- > include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++ > 2 files changed, 152 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/iio/adi,ad5755.h > > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c > index e1b6e78..c1e2546 100644 > --- a/drivers/iio/dac/ad5755.c > +++ b/drivers/iio/dac/ad5755.c > @@ -14,9 +14,11 @@ > #include <linux/slab.h> > #include <linux/sysfs.h> > #include <linux/delay.h> > +#include <linux/of.h> > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > #include <linux/platform_data/ad5755.h> > +#include <dt-bindings/iio/adi,ad5755.h> > > #define AD5755_NUM_CHANNELS 4 > > @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = { > }, > }; > > +#ifdef CONFIG_OF > +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct device_node *pp; > + struct ad5755_platform_data *pdata; > + unsigned int tmp; > + unsigned int tmparray[3]; > + int devnr; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + pdata->ext_dc_dc_compenstation_resistor = > + of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor"); > + > + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) > + pdata->dc_dc_phase = tmp; > + else > + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; > + > + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) > + pdata->dc_dc_freq = tmp; > + else > + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; > + > + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) > + pdata->dc_dc_maxv = tmp; > + else > + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; > + > + devnr = 0; > + for_each_child_of_node(np, pp) { > + if (devnr > AD5755_NUM_CHANNELS) { > + dev_err(dev, "There is to many channels defined in DT\n"); > + goto error_out; > + } > + > + if (!of_property_read_u32(pp, "adi,mode", &tmp)) > + pdata->dac[devnr].mode = tmp; > + else > + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; > + > + pdata->dac[devnr].ext_current_sense_resistor = > + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); > + > + pdata->dac[devnr].enable_voltage_overrange = > + of_property_read_bool(pp, "adi,enable_voltage_overrange"); > + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { > + pdata->dac[devnr].slew.enable = tmparray[0]; > + pdata->dac[devnr].slew.rate = tmparray[1]; > + pdata->dac[devnr].slew.step_size = tmparray[2]; > + } else { > + pdata->dac[devnr].slew.enable = false; > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; > + } > + > + devnr++; > + } > + > + return pdata; > + > +error_out: > + devm_kfree(dev, pdata); > + return NULL; > +} > +#else > +static > +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > static int ad5755_probe(struct spi_device *spi) > { > enum ad5755_type type = spi_get_device_id(spi)->driver_data; > @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->num_channels = AD5755_NUM_CHANNELS; > > - if (!pdata) > + if (spi->dev.of_node) > + pdata = ad5755_parse_dt(&spi->dev); > + else > + pdata = spi->dev.platform_data; > + > + if (!pdata) { > + dev_warn(&spi->dev, "no platform data? using default\n"); > pdata = &ad5755_default_pdata; > + } > > ret = ad5755_init_channels(indio_dev, pdata); > if (ret) > diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h > new file mode 100644 > index 0000000..117466b > --- /dev/null > +++ b/include/dt-bindings/iio/adi,ad5755.h > @@ -0,0 +1,66 @@ > +/* > + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only 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. > + */ > + > +#ifndef _DT_BINDINGS_AD5755_H > +#define _DT_BINDINGS_AD5755_H > + > +#define AD5755_MODE_VOLTAGE_0V_5V 0 > +#define AD5755_MODE_VOLTAGE_0V_10V 1 > +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V 2 > +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V 3 > +#define AD5755_MODE_CURRENT_4mA_20mA 4 > +#define AD5755_MODE_CURRENT_0mA_20mA 5 > +#define AD5755_MODE_CURRENT_0mA_24mA 6 > + > +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE 0 > +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE 1 > +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE 2 > +#define AD5755_DC_DC_PHASE_90_DEGREE 3 > + > +#define AD5755_DC_DC_FREQ_250kHZ 0 > +#define AD5755_DC_DC_FREQ_410kHZ 1 > +#define AD5755_DC_DC_FREQ_650kHZ 2 > + > +#define AD5755_DC_DC_MAXV_23V 0 > +#define AD5755_DC_DC_MAXV_24V5 1 > +#define AD5755_DC_DC_MAXV_27V 2 > +#define AD5755_DC_DC_MAXV_29V5 3 > + > +#define AD5755_SLEW_RATE_64k 0 > +#define AD5755_SLEW_RATE_32k 1 > +#define AD5755_SLEW_RATE_16k 2 > +#define AD5755_SLEW_RATE_8k 3 > +#define AD5755_SLEW_RATE_4k 4 > +#define AD5755_SLEW_RATE_2k 5 > +#define AD5755_SLEW_RATE_1k 6 > +#define AD5755_SLEW_RATE_500 7 > +#define AD5755_SLEW_RATE_250 8 > +#define AD5755_SLEW_RATE_125 9 > +#define AD5755_SLEW_RATE_64 10 > +#define AD5755_SLEW_RATE_32 11 > +#define AD5755_SLEW_RATE_16 12 > +#define AD5755_SLEW_RATE_8 13 > +#define AD5755_SLEW_RATE_4 14 > +#define AD5755_SLEW_RATE_0_5 15 > + > +#define AD5755_SLEW_STEP_SIZE_1 0 > +#define AD5755_SLEW_STEP_SIZE_2 1 > +#define AD5755_SLEW_STEP_SIZE_4 2 > +#define AD5755_SLEW_STEP_SIZE_8 3 > +#define AD5755_SLEW_STEP_SIZE_16 4 > +#define AD5755_SLEW_STEP_SIZE_32 5 > +#define AD5755_SLEW_STEP_SIZE_64 6 > +#define AD5755_SLEW_STEP_SIZE_128 7 > +#define AD5755_SLEW_STEP_SIZE_256 8 > + > +#endif /* _DT_BINDINGS_AD5755_H */ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree @ 2016-02-09 8:22 ` Sean Nyekjær 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjær @ 2016-02-09 8:22 UTC (permalink / raw) To: linux-iio, Lars-Peter Clausen; +Cc: devicetree On 2016-02-03 15:31, Sean Nyekjaer wrote: > Devicetree can provide platform data > > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> Lars do you have time to look at this before i'm fixing the comments from Jonathan and Rob :-) Sean > --- > Changes since v1: > - this patch added > > I'm adding dt bindings header file with the same info as, the platform data as > the devicetree can't understand enums. > Please give an idea to work around this :-) > > drivers/iio/dac/ad5755.c | 87 +++++++++++++++++++++++++++++++++++- > include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++ > 2 files changed, 152 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/iio/adi,ad5755.h > > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c > index e1b6e78..c1e2546 100644 > --- a/drivers/iio/dac/ad5755.c > +++ b/drivers/iio/dac/ad5755.c > @@ -14,9 +14,11 @@ > #include <linux/slab.h> > #include <linux/sysfs.h> > #include <linux/delay.h> > +#include <linux/of.h> > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > #include <linux/platform_data/ad5755.h> > +#include <dt-bindings/iio/adi,ad5755.h> > > #define AD5755_NUM_CHANNELS 4 > > @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = { > }, > }; > > +#ifdef CONFIG_OF > +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct device_node *pp; > + struct ad5755_platform_data *pdata; > + unsigned int tmp; > + unsigned int tmparray[3]; > + int devnr; > + > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + pdata->ext_dc_dc_compenstation_resistor = > + of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor"); > + > + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) > + pdata->dc_dc_phase = tmp; > + else > + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; > + > + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) > + pdata->dc_dc_freq = tmp; > + else > + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; > + > + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) > + pdata->dc_dc_maxv = tmp; > + else > + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; > + > + devnr = 0; > + for_each_child_of_node(np, pp) { > + if (devnr > AD5755_NUM_CHANNELS) { > + dev_err(dev, "There is to many channels defined in DT\n"); > + goto error_out; > + } > + > + if (!of_property_read_u32(pp, "adi,mode", &tmp)) > + pdata->dac[devnr].mode = tmp; > + else > + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; > + > + pdata->dac[devnr].ext_current_sense_resistor = > + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); > + > + pdata->dac[devnr].enable_voltage_overrange = > + of_property_read_bool(pp, "adi,enable_voltage_overrange"); > + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { > + pdata->dac[devnr].slew.enable = tmparray[0]; > + pdata->dac[devnr].slew.rate = tmparray[1]; > + pdata->dac[devnr].slew.step_size = tmparray[2]; > + } else { > + pdata->dac[devnr].slew.enable = false; > + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; > + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; > + } > + > + devnr++; > + } > + > + return pdata; > + > +error_out: > + devm_kfree(dev, pdata); > + return NULL; > +} > +#else > +static > +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev) > +{ > + return NULL; > +} > +#endif > + > static int ad5755_probe(struct spi_device *spi) > { > enum ad5755_type type = spi_get_device_id(spi)->driver_data; > @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->num_channels = AD5755_NUM_CHANNELS; > > - if (!pdata) > + if (spi->dev.of_node) > + pdata = ad5755_parse_dt(&spi->dev); > + else > + pdata = spi->dev.platform_data; > + > + if (!pdata) { > + dev_warn(&spi->dev, "no platform data? using default\n"); > pdata = &ad5755_default_pdata; > + } > > ret = ad5755_init_channels(indio_dev, pdata); > if (ret) > diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h > new file mode 100644 > index 0000000..117466b > --- /dev/null > +++ b/include/dt-bindings/iio/adi,ad5755.h > @@ -0,0 +1,66 @@ > +/* > + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only 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. > + */ > + > +#ifndef _DT_BINDINGS_AD5755_H > +#define _DT_BINDINGS_AD5755_H > + > +#define AD5755_MODE_VOLTAGE_0V_5V 0 > +#define AD5755_MODE_VOLTAGE_0V_10V 1 > +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V 2 > +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V 3 > +#define AD5755_MODE_CURRENT_4mA_20mA 4 > +#define AD5755_MODE_CURRENT_0mA_20mA 5 > +#define AD5755_MODE_CURRENT_0mA_24mA 6 > + > +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE 0 > +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE 1 > +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE 2 > +#define AD5755_DC_DC_PHASE_90_DEGREE 3 > + > +#define AD5755_DC_DC_FREQ_250kHZ 0 > +#define AD5755_DC_DC_FREQ_410kHZ 1 > +#define AD5755_DC_DC_FREQ_650kHZ 2 > + > +#define AD5755_DC_DC_MAXV_23V 0 > +#define AD5755_DC_DC_MAXV_24V5 1 > +#define AD5755_DC_DC_MAXV_27V 2 > +#define AD5755_DC_DC_MAXV_29V5 3 > + > +#define AD5755_SLEW_RATE_64k 0 > +#define AD5755_SLEW_RATE_32k 1 > +#define AD5755_SLEW_RATE_16k 2 > +#define AD5755_SLEW_RATE_8k 3 > +#define AD5755_SLEW_RATE_4k 4 > +#define AD5755_SLEW_RATE_2k 5 > +#define AD5755_SLEW_RATE_1k 6 > +#define AD5755_SLEW_RATE_500 7 > +#define AD5755_SLEW_RATE_250 8 > +#define AD5755_SLEW_RATE_125 9 > +#define AD5755_SLEW_RATE_64 10 > +#define AD5755_SLEW_RATE_32 11 > +#define AD5755_SLEW_RATE_16 12 > +#define AD5755_SLEW_RATE_8 13 > +#define AD5755_SLEW_RATE_4 14 > +#define AD5755_SLEW_RATE_0_5 15 > + > +#define AD5755_SLEW_STEP_SIZE_1 0 > +#define AD5755_SLEW_STEP_SIZE_2 1 > +#define AD5755_SLEW_STEP_SIZE_4 2 > +#define AD5755_SLEW_STEP_SIZE_8 3 > +#define AD5755_SLEW_STEP_SIZE_16 4 > +#define AD5755_SLEW_STEP_SIZE_32 5 > +#define AD5755_SLEW_STEP_SIZE_64 6 > +#define AD5755_SLEW_STEP_SIZE_128 7 > +#define AD5755_SLEW_STEP_SIZE_256 8 > + > +#endif /* _DT_BINDINGS_AD5755_H */ ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <56B9A1B2.6030007-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>]
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree 2016-02-09 8:22 ` Sean Nyekjær @ 2016-02-09 20:30 ` Lars-Peter Clausen -1 siblings, 0 replies; 20+ messages in thread From: Lars-Peter Clausen @ 2016-02-09 20:30 UTC (permalink / raw) To: Sean Nyekjær, linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: devicetree-u79uwXL29TY76Z2rM5mHXA On 02/09/2016 09:22 AM, Sean Nyekjær wrote: > > On 2016-02-03 15:31, Sean Nyekjaer wrote: >> Devicetree can provide platform data >> >> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> > Lars do you have time to look at this before i'm fixing the comments from > Jonathan and Rob :-) The obvious thing is to use '-' instead of '_' since that is the DT convention. Other than that this non-trivial. This is the first binding for such a device and it is always hard to come up with a good ABI without knowing what other similar devices might need to be supported by the same bindings. And it is also not always clear what is hardware description and what is driver policy. The external resistor configuration should be clear, that's a description of the hardware whether the resistor exists or not. For the others it's not so clear. E.g. the DC-to-DC converter configuration, this is most certainly not a description of the hardware. But we could get away with it by saying this is a system level design decision and is made by the system designer based on the operating parameters of the hardware and hence is policy that belongs in the DT. It might make sense to follow what the regulator bindings do here, even though the regulator is not exposed externally. The configuration of the output pin (voltage level, current level, etc) is certainly influenced by the restrictions of the hardware itself. You don't want to allow a configuration that can damage the hardware. But on the other hand it is still a valid usecase for some applications to switch between two setups at runtime, which is something you said you need to be able to do. Here we might want to use something similar to pinctrl where the devicetree can offer a selection of valid configurations and the driver can switch between those configurations. - Lars [...] >> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) >> +{ >> + struct device_node *np = dev->of_node; >> + struct device_node *pp; >> + struct ad5755_platform_data *pdata; >> + unsigned int tmp; >> + unsigned int tmparray[3]; >> + int devnr; >> + >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) >> + return NULL; >> + >> + pdata->ext_dc_dc_compenstation_resistor = >> + of_property_read_bool(np, >> "adi,ext_dc_dc_compenstation_resistor"); >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) >> + pdata->dc_dc_phase = tmp; >> + else >> + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) >> + pdata->dc_dc_freq = tmp; >> + else >> + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) >> + pdata->dc_dc_maxv = tmp; >> + else >> + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; >> + >> + devnr = 0; >> + for_each_child_of_node(np, pp) { >> + if (devnr > AD5755_NUM_CHANNELS) { >> + dev_err(dev, "There is to many channels defined in DT\n"); >> + goto error_out; >> + } >> + >> + if (!of_property_read_u32(pp, "adi,mode", &tmp)) >> + pdata->dac[devnr].mode = tmp; >> + else >> + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; >> + >> + pdata->dac[devnr].ext_current_sense_resistor = >> + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); >> + >> + pdata->dac[devnr].enable_voltage_overrange = >> + of_property_read_bool(pp, "adi,enable_voltage_overrange"); >> + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { >> + pdata->dac[devnr].slew.enable = tmparray[0]; >> + pdata->dac[devnr].slew.rate = tmparray[1]; >> + pdata->dac[devnr].slew.step_size = tmparray[2]; >> + } else { >> + pdata->dac[devnr].slew.enable = false; >> + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; >> + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; >> + } >> + >> + devnr++; >> + } >> + >> + return pdata; [...] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree @ 2016-02-09 20:30 ` Lars-Peter Clausen 0 siblings, 0 replies; 20+ messages in thread From: Lars-Peter Clausen @ 2016-02-09 20:30 UTC (permalink / raw) To: Sean Nyekjær, linux-iio; +Cc: devicetree On 02/09/2016 09:22 AM, Sean Nyekjær wrote: > > On 2016-02-03 15:31, Sean Nyekjaer wrote: >> Devicetree can provide platform data >> >> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> > Lars do you have time to look at this before i'm fixing the comments from > Jonathan and Rob :-) The obvious thing is to use '-' instead of '_' since that is the DT convention. Other than that this non-trivial. This is the first binding for such a device and it is always hard to come up with a good ABI without knowing what other similar devices might need to be supported by the same bindings. And it is also not always clear what is hardware description and what is driver policy. The external resistor configuration should be clear, that's a description of the hardware whether the resistor exists or not. For the others it's not so clear. E.g. the DC-to-DC converter configuration, this is most certainly not a description of the hardware. But we could get away with it by saying this is a system level design decision and is made by the system designer based on the operating parameters of the hardware and hence is policy that belongs in the DT. It might make sense to follow what the regulator bindings do here, even though the regulator is not exposed externally. The configuration of the output pin (voltage level, current level, etc) is certainly influenced by the restrictions of the hardware itself. You don't want to allow a configuration that can damage the hardware. But on the other hand it is still a valid usecase for some applications to switch between two setups at runtime, which is something you said you need to be able to do. Here we might want to use something similar to pinctrl where the devicetree can offer a selection of valid configurations and the driver can switch between those configurations. - Lars [...] >> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) >> +{ >> + struct device_node *np = dev->of_node; >> + struct device_node *pp; >> + struct ad5755_platform_data *pdata; >> + unsigned int tmp; >> + unsigned int tmparray[3]; >> + int devnr; >> + >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) >> + return NULL; >> + >> + pdata->ext_dc_dc_compenstation_resistor = >> + of_property_read_bool(np, >> "adi,ext_dc_dc_compenstation_resistor"); >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) >> + pdata->dc_dc_phase = tmp; >> + else >> + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) >> + pdata->dc_dc_freq = tmp; >> + else >> + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; >> + >> + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) >> + pdata->dc_dc_maxv = tmp; >> + else >> + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; >> + >> + devnr = 0; >> + for_each_child_of_node(np, pp) { >> + if (devnr > AD5755_NUM_CHANNELS) { >> + dev_err(dev, "There is to many channels defined in DT\n"); >> + goto error_out; >> + } >> + >> + if (!of_property_read_u32(pp, "adi,mode", &tmp)) >> + pdata->dac[devnr].mode = tmp; >> + else >> + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; >> + >> + pdata->dac[devnr].ext_current_sense_resistor = >> + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); >> + >> + pdata->dac[devnr].enable_voltage_overrange = >> + of_property_read_bool(pp, "adi,enable_voltage_overrange"); >> + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { >> + pdata->dac[devnr].slew.enable = tmparray[0]; >> + pdata->dac[devnr].slew.rate = tmparray[1]; >> + pdata->dac[devnr].slew.step_size = tmparray[2]; >> + } else { >> + pdata->dac[devnr].slew.enable = false; >> + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; >> + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; >> + } >> + >> + devnr++; >> + } >> + >> + return pdata; [...] ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <56BA4C74.9030503-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>]
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree 2016-02-09 20:30 ` Lars-Peter Clausen @ 2016-02-16 13:46 ` Sean Nyekjær -1 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjær @ 2016-02-16 13:46 UTC (permalink / raw) To: Lars-Peter Clausen, linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: devicetree-u79uwXL29TY76Z2rM5mHXA On 2016-02-09 21:30, Lars-Peter Clausen wrote: > On 02/09/2016 09:22 AM, Sean Nyekjær wrote: >> On 2016-02-03 15:31, Sean Nyekjaer wrote: >>> Devicetree can provide platform data >>> >>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> >> Lars do you have time to look at this before i'm fixing the comments from >> Jonathan and Rob :-) > The obvious thing is to use '-' instead of '_' since that is the DT convention. I'm removing the defines in from the DT doc. > > Other than that this non-trivial. This is the first binding for such a > device and it is always hard to come up with a good ABI without knowing what > other similar devices might need to be supported by the same bindings. And > it is also not always clear what is hardware description and what is driver > policy. I was hoping we could cooperate on enabling this feature. It's not the majority of linux devices that are using board files any more. > > The external resistor configuration should be clear, that's a description of > the hardware whether the resistor exists or not. > > For the others it's not so clear. E.g. the DC-to-DC converter configuration, > this is most certainly not a description of the hardware. But we could get > away with it by saying this is a system level design decision and is made by > the system designer based on the operating parameters of the hardware and > hence is policy that belongs in the DT. It might make sense to follow what > the regulator bindings do here, even though the regulator is not exposed > externally. > > The configuration of the output pin (voltage level, current level, etc) is > certainly influenced by the restrictions of the hardware itself. You don't > want to allow a configuration that can damage the hardware. But on the other > hand it is still a valid usecase for some applications to switch between two > setups at runtime, which is something you said you need to be able to do. I can't why enabling configuration of the DAC from the DT is posing a greater risk of damaging hardware than the previous way of editing the board file. > Here we might want to use something similar to pinctrl where the devicetree > can offer a selection of valid configurations and the driver can switch > between those configurations. > > - Lars > > [...] I'll submit an PATCH v3 where i have removed the defines from the DT doc and introduce checks for the values passed from DT. /Sean >>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) >>> +{ >>> + struct device_node *np = dev->of_node; >>> + struct device_node *pp; >>> + struct ad5755_platform_data *pdata; >>> + unsigned int tmp; >>> + unsigned int tmparray[3]; >>> + int devnr; >>> + >>> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >>> + if (!pdata) >>> + return NULL; >>> + >>> + pdata->ext_dc_dc_compenstation_resistor = >>> + of_property_read_bool(np, >>> "adi,ext_dc_dc_compenstation_resistor"); >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) >>> + pdata->dc_dc_phase = tmp; >>> + else >>> + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) >>> + pdata->dc_dc_freq = tmp; >>> + else >>> + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) >>> + pdata->dc_dc_maxv = tmp; >>> + else >>> + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; >>> + >>> + devnr = 0; >>> + for_each_child_of_node(np, pp) { >>> + if (devnr > AD5755_NUM_CHANNELS) { >>> + dev_err(dev, "There is to many channels defined in DT\n"); >>> + goto error_out; >>> + } >>> + >>> + if (!of_property_read_u32(pp, "adi,mode", &tmp)) >>> + pdata->dac[devnr].mode = tmp; >>> + else >>> + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; >>> + >>> + pdata->dac[devnr].ext_current_sense_resistor = >>> + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); >>> + >>> + pdata->dac[devnr].enable_voltage_overrange = >>> + of_property_read_bool(pp, "adi,enable_voltage_overrange"); >>> + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { >>> + pdata->dac[devnr].slew.enable = tmparray[0]; >>> + pdata->dac[devnr].slew.rate = tmparray[1]; >>> + pdata->dac[devnr].slew.step_size = tmparray[2]; >>> + } else { >>> + pdata->dac[devnr].slew.enable = false; >>> + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; >>> + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; >>> + } >>> + >>> + devnr++; >>> + } >>> + >>> + return pdata; > [...] > -- > 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 [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree @ 2016-02-16 13:46 ` Sean Nyekjær 0 siblings, 0 replies; 20+ messages in thread From: Sean Nyekjær @ 2016-02-16 13:46 UTC (permalink / raw) To: Lars-Peter Clausen, linux-iio; +Cc: devicetree On 2016-02-09 21:30, Lars-Peter Clausen wrote: > On 02/09/2016 09:22 AM, Sean Nyekjær wrote: >> On 2016-02-03 15:31, Sean Nyekjaer wrote: >>> Devicetree can provide platform data >>> >>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> >> Lars do you have time to look at this before i'm fixing the comments from >> Jonathan and Rob :-) > The obvious thing is to use '-' instead of '_' since that is the DT convention. I'm removing the defines in from the DT doc. > > Other than that this non-trivial. This is the first binding for such a > device and it is always hard to come up with a good ABI without knowing what > other similar devices might need to be supported by the same bindings. And > it is also not always clear what is hardware description and what is driver > policy. I was hoping we could cooperate on enabling this feature. It's not the majority of linux devices that are using board files any more. > > The external resistor configuration should be clear, that's a description of > the hardware whether the resistor exists or not. > > For the others it's not so clear. E.g. the DC-to-DC converter configuration, > this is most certainly not a description of the hardware. But we could get > away with it by saying this is a system level design decision and is made by > the system designer based on the operating parameters of the hardware and > hence is policy that belongs in the DT. It might make sense to follow what > the regulator bindings do here, even though the regulator is not exposed > externally. > > The configuration of the output pin (voltage level, current level, etc) is > certainly influenced by the restrictions of the hardware itself. You don't > want to allow a configuration that can damage the hardware. But on the other > hand it is still a valid usecase for some applications to switch between two > setups at runtime, which is something you said you need to be able to do. I can't why enabling configuration of the DAC from the DT is posing a greater risk of damaging hardware than the previous way of editing the board file. > Here we might want to use something similar to pinctrl where the devicetree > can offer a selection of valid configurations and the driver can switch > between those configurations. > > - Lars > > [...] I'll submit an PATCH v3 where i have removed the defines from the DT doc and introduce checks for the values passed from DT. /Sean >>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev) >>> +{ >>> + struct device_node *np = dev->of_node; >>> + struct device_node *pp; >>> + struct ad5755_platform_data *pdata; >>> + unsigned int tmp; >>> + unsigned int tmparray[3]; >>> + int devnr; >>> + >>> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >>> + if (!pdata) >>> + return NULL; >>> + >>> + pdata->ext_dc_dc_compenstation_resistor = >>> + of_property_read_bool(np, >>> "adi,ext_dc_dc_compenstation_resistor"); >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp)) >>> + pdata->dc_dc_phase = tmp; >>> + else >>> + pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE; >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp)) >>> + pdata->dc_dc_freq = tmp; >>> + else >>> + pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ; >>> + >>> + if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp)) >>> + pdata->dc_dc_maxv = tmp; >>> + else >>> + pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V; >>> + >>> + devnr = 0; >>> + for_each_child_of_node(np, pp) { >>> + if (devnr > AD5755_NUM_CHANNELS) { >>> + dev_err(dev, "There is to many channels defined in DT\n"); >>> + goto error_out; >>> + } >>> + >>> + if (!of_property_read_u32(pp, "adi,mode", &tmp)) >>> + pdata->dac[devnr].mode = tmp; >>> + else >>> + pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA; >>> + >>> + pdata->dac[devnr].ext_current_sense_resistor = >>> + of_property_read_bool(pp, "adi,ext_current_sense_resistor"); >>> + >>> + pdata->dac[devnr].enable_voltage_overrange = >>> + of_property_read_bool(pp, "adi,enable_voltage_overrange"); >>> + if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) { >>> + pdata->dac[devnr].slew.enable = tmparray[0]; >>> + pdata->dac[devnr].slew.rate = tmparray[1]; >>> + pdata->dac[devnr].slew.step_size = tmparray[2]; >>> + } else { >>> + pdata->dac[devnr].slew.enable = false; >>> + pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k; >>> + pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1; >>> + } >>> + >>> + devnr++; >>> + } >>> + >>> + return pdata; > [...] > -- > 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 [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/4] iio: ad5755: Add DT binding documentation 2016-02-03 14:31 ` Sean Nyekjaer @ 2016-02-06 18:36 ` Jonathan Cameron -1 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2016-02-06 18:36 UTC (permalink / raw) To: Sean Nyekjaer, linux-iio-u79uwXL29TY76Z2rM5mHXA Cc: devicetree-u79uwXL29TY76Z2rM5mHXA On 03/02/16 14:31, Sean Nyekjaer wrote: > Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org> Please pick up Rob's Ack in any future versions of the series (I'm sending this as mostly as a reminder to myself if this one goes in as it is!) > --- > Changes since v1: > - none > > .../devicetree/bindings/iio/dac/ad5755.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt > > diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > new file mode 100644 > index 0000000..4c91583 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > @@ -0,0 +1,24 @@ > +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver > + > +Required properties: > + - compatible: Has to contain one of the following: > + adi,ad5755 > + adi,ad5755-1 > + adi,ad5757 > + adi,ad5735 > + adi,ad5737 > + > + - reg: spi chip select number for the device > + - spi-cpha or spi-cpol: is the only modes that is supported > + > +Recommended properties: > + - spi-max-frequency: Definition as per > + Documentation/devicetree/bindings/spi/spi-bus.txt > + > +Example: > +dac@0 { > + compatible = "adi,ad5755"; > + reg = <0>; > + spi-max-frequency = <1000000>; > + spi-cpha; > +}; > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/4] iio: ad5755: Add DT binding documentation @ 2016-02-06 18:36 ` Jonathan Cameron 0 siblings, 0 replies; 20+ messages in thread From: Jonathan Cameron @ 2016-02-06 18:36 UTC (permalink / raw) To: Sean Nyekjaer, linux-iio; +Cc: devicetree On 03/02/16 14:31, Sean Nyekjaer wrote: > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> Please pick up Rob's Ack in any future versions of the series (I'm sending this as mostly as a reminder to myself if this one goes in as it is!) > --- > Changes since v1: > - none > > .../devicetree/bindings/iio/dac/ad5755.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt > > diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > new file mode 100644 > index 0000000..4c91583 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt > @@ -0,0 +1,24 @@ > +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver > + > +Required properties: > + - compatible: Has to contain one of the following: > + adi,ad5755 > + adi,ad5755-1 > + adi,ad5757 > + adi,ad5735 > + adi,ad5737 > + > + - reg: spi chip select number for the device > + - spi-cpha or spi-cpol: is the only modes that is supported > + > +Recommended properties: > + - spi-max-frequency: Definition as per > + Documentation/devicetree/bindings/spi/spi-bus.txt > + > +Example: > +dac@0 { > + compatible = "adi,ad5755"; > + reg = <0>; > + spi-max-frequency = <1000000>; > + spi-cpha; > +}; > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-02-16 13:46 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 14:31 [PATCH v2 1/4] iio: ad5755: add support for dt bindings Sean Nyekjaer
2016-02-03 14:31 ` Sean Nyekjaer
[not found] ` <1454509864-32285-1-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
2016-02-03 14:31 ` [PATCH v2 2/4] iio: ad5755: Add DT binding documentation Sean Nyekjaer
2016-02-03 14:31 ` Sean Nyekjaer
[not found] ` <1454509864-32285-2-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
2016-02-03 14:31 ` [PATCH v2 3/4] iio: ad5755: added full support for devicetree Sean Nyekjaer
2016-02-03 14:31 ` Sean Nyekjaer
[not found] ` <1454509864-32285-3-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
2016-02-03 14:31 ` [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation Sean Nyekjaer
2016-02-03 14:31 ` Sean Nyekjaer
[not found] ` <1454509864-32285-4-git-send-email-sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
2016-02-06 18:30 ` Jonathan Cameron
2016-02-06 18:30 ` Jonathan Cameron
[not found] ` <56B63BAC.2040001-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-02-08 17:37 ` Rob Herring
2016-02-08 17:37 ` Rob Herring
2016-02-09 8:22 ` [PATCH v2 3/4] iio: ad5755: added full support for devicetree Sean Nyekjær
2016-02-09 8:22 ` Sean Nyekjær
[not found] ` <56B9A1B2.6030007-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
2016-02-09 20:30 ` Lars-Peter Clausen
2016-02-09 20:30 ` Lars-Peter Clausen
[not found] ` <56BA4C74.9030503-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2016-02-16 13:46 ` Sean Nyekjær
2016-02-16 13:46 ` Sean Nyekjær
2016-02-06 18:36 ` [PATCH v2 2/4] iio: ad5755: Add DT binding documentation Jonathan Cameron
2016-02-06 18:36 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.