mxs-lradc: Add support for current sources
Subsystems (3)
cover mxs-lradc: Add support for current sources
Patch 1/3 changes the driver and updates the binding documentation (I guess it is still in staging.)
Patches 2/3 and 3/3 add the devicetree nodes to imx23 and imx28 boards. I'd like to get input whether this is actually desired. On boards where these regulators would never be enabled this costs a few extra bytes of RAM for allocation of the device data, because the nodes can't be easily removed in .dts files which are including the .dtsi files. The alternative is to add the new nodes to many .dts files, which would be a lot code duplication.
Harald Geyer (3):
iio: mxs-lradc: Add regulators for current sources
ARM: dts: imx23: Provide regulators for the current sources of the
LRADC
ARM: dts: imx28: Provide regulators for the current sources of the
LRADC
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ arch/arm/boot/dts/imx23.dtsi | 8 ++ arch/arm/boot/dts/imx28.dtsi | 8 ++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 5 files changed, 198 insertions(+)
Signature · 1 line
2.1.4
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie. min_uA == max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway.
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +29 -0
@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:
@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:
@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };
drivers/iio/adc/Kconfig +1 -0
@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER
drivers/iio/adc/mxs-lradc.c +152 -0
200 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
Quoted 7 lines · Harald Geyer
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>
Quoted 8 lines
---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie.min_uA==max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway.
I am tempted to block this patch and ask you to properly split the mxs-lradc driver into MFD with touchscreen and IIO part and only then add the regulator bits. The lradc is becoming a katamari of ad-hoc misplaced functionality.
Quoted 282 lines
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +29
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFERdrivers/iio/adc/mxs-lradc.c +152
-0@@ -40,6 +40,10 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/sysfs.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#define DRIVER_NAME "mxs-lradc"#define LRADC_MAX_DELAY_CHANS 4@@ -261,6 +265,9 @@ struct mxs_lradc {unsigned over_sample_delay;/* time in clocks to wait after the plates where switched */ unsigned settling_delay; + +struct regulator_desc isrc0; +struct regulator_desc isrc1; };#define LRADC_CTRL0 0x00@@ -305,6 +312,11 @@ struct mxs_lradc {#define LRADC_CTRL2 0x20#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24#define LRADC_CTRL2_TEMPSENSE_PWD BIT(15) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE1 BIT(9) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE0 BIT(8) +#define LRADC_CTRL2_TEMP_ISRC1_OFFSET 4 +#define LRADC_CTRL2_TEMP_ISRC0_OFFSET 0 +#define LRADC_CTRL2_TEMP_ISRC_MASK 0x0f#define LRADC_STATUS 0x40#define LRADC_STATUS_TOUCH_DETECT_RAW BIT(0)@@ -1383,6 +1395,109 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { .validate_scan_mask = &mxs_lradc_validate_scan_mask, }; +static int mxs_lradc_regulator_is_enabled (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE0 ; +else if (dev->desc == &lradc->isrc1) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE1 ; + +/* This should never happen */ +return -ENODEV ; +} + +#define LRADC_REGVALUE2uA (regval, offset) \ + (20 * ((regval >> offset) & LRADC_CTRL2_TEMP_ISRC_MASK)) + +static int mxs_lradc_regulator_get_current_limit (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC0_OFFSET ); +else if (dev->desc == &lradc->isrc1) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC1_OFFSET ); + +/* This should never happen */ +return -ENODEV ; +} + +static int mxs_lradc_regulator_enable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_disable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_set_current_limit (struct regulator_dev *dev, +int min_uA,int max_uA) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int offset, value; + +if (dev->desc == &lradc->isrc0) + offset =LRADC_CTRL2_TEMP_ISRC0_OFFSET ; +else if (dev->desc == &lradc->isrc1) + offset =LRADC_CTRL2_TEMP_ISRC1_OFFSET ; +else +/* This should never happen */ +return -ENODEV ; + + value = min_uA /20 ; +if (min_uA %20 ) + value++; +if (value *20 > max_uA) +return -EINVAL ; +if (value & ~LRADC_CTRL2_TEMP_ISRC_MASK ) +/* This should never happen */ +return -EPERM ; + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_ISRC_MASK << offset, +LRADC_CTRL2 ); +mxs_lradc_reg_set (lradc, value << offset,LRADC_CTRL2 ); + +return 0 ; +} + +static struct regulator_ops mxs_lradc_regulator_current_ops = { + .enable = mxs_lradc_regulator_enable, + .is_enabled = mxs_lradc_regulator_is_enabled, + .disable = mxs_lradc_regulator_disable, + .get_current_limit = mxs_lradc_regulator_get_current_limit, + .set_current_limit = mxs_lradc_regulator_set_current_limit, +}; +/* * Driver initialization */ @@ -1519,6 +1634,10 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i)); + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 | +LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); }static const struct of_device_id mxs_lradc_dt_ids[] = {@@ -1592,6 +1711,32 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,return 0 ; } +static void mxs_lradc_reg_helper (struct device_node *np,const char *name, +struct regulator_config *conf, +struct regulator_desc *desc) +{ +struct regulator_dev *ret; + + conf->of_node =of_get_child_by_name (np, name); +if (!conf->of_node) +return ; + + desc->name = name; + desc->owner =THIS_MODULE ; + desc->type =REGULATOR_CURRENT ; + desc->ops = &mxs_lradc_regulator_current_ops; + + conf->init_data =of_get_regulator_init_data (conf->dev, conf->of_node, + desc); + ret =devm_regulator_register (conf->dev, desc, conf); +if (IS_ERR (ret)) +/* Just pretend the regulator isn't there */ +dev_err (conf->dev,"Failed to register regulator %s: %ld\n" , + desc->name,PTR_ERR (ret)); + +of_node_put (conf->of_node); +} +static int mxs_lradc_probe (struct platform_device *pdev) { const struct of_device_id *of_id =@@ -1603,6 +1748,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)struct mxs_lradc *lradc;struct iio_dev *iio;struct resource *iores; +struct regulator_config regconf;int ret =0 , touch_ret;int i, s;u64 scale_uv;@@ -1727,6 +1873,12 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; } +/* Setup regulator devices for current source. */ + regconf.dev = dev; + regconf.driver_data = lradc; +mxs_lradc_reg_helper (node,"ISRC0" , ®conf, &lradc->isrc0); +mxs_lradc_reg_helper (node,"ISRC1" , ®conf, &lradc->isrc1); +return 0 ; err_ts:
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
HI
Quoted 17 lines · Marek Vasut
Quoted 7 lines · Harald Geyer
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>Quoted 8 lines
---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie.min_uA==max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway.
Quoted 4 lines
I am tempted to block this patch and ask you to properly split the mxs-lradc driver into MFD with touchscreen and IIO part and only then add the regulator bits. The lradc is becoming a katamari of ad-hoc misplaced functionality.
I almost finished, I already split the driver into MFD, sorry for the delay. Can this wait a little bit?
Thanks,
Ksenija
Quoted 291 lines
Quoted 272 lines
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +29
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFERdrivers/iio/adc/mxs-lradc.c +146
-0@@ -40,6 +40,10 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/sysfs.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#define DRIVER_NAME "mxs-lradc"#define LRADC_MAX_DELAY_CHANS 4@@ -261,6 +265,9 @@ struct mxs_lradc {unsigned over_sample_delay;/* time in clocks to wait after the plates where switched */ unsigned settling_delay; + +struct regulator_desc isrc0; +struct regulator_desc isrc1; };#define LRADC_CTRL0 0x00@@ -305,6 +312,11 @@ struct mxs_lradc {#define LRADC_CTRL2 0x20#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24#define LRADC_CTRL2_TEMPSENSE_PWD BIT(15) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE1 BIT(9) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE0 BIT(8) +#define LRADC_CTRL2_TEMP_ISRC1_OFFSET 4 +#define LRADC_CTRL2_TEMP_ISRC0_OFFSET 0 +#define LRADC_CTRL2_TEMP_ISRC_MASK 0x0f#define LRADC_STATUS 0x40#define LRADC_STATUS_TOUCH_DETECT_RAW BIT(0)@@ -1383,6 +1395,109 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { .validate_scan_mask = &mxs_lradc_validate_scan_mask, }; +static int mxs_lradc_regulator_is_enabled (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE0 ; +else if (dev->desc == &lradc->isrc1) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE1 ; + +/* This should never happen */ +return -ENODEV ; +} + +#define LRADC_REGVALUE2uA (regval, offset) \ + (20 * ((regval >> offset) & LRADC_CTRL2_TEMP_ISRC_MASK)) + +static int mxs_lradc_regulator_get_current_limit (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC0_OFFSET ); +else if (dev->desc == &lradc->isrc1) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC1_OFFSET ); + +/* This should never happen */ +return -ENODEV ; +} + +static int mxs_lradc_regulator_enable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_disable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_set_current_limit (struct regulator_dev *dev, +int min_uA,int max_uA) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int offset, value; + +if (dev->desc == &lradc->isrc0) + offset =LRADC_CTRL2_TEMP_ISRC0_OFFSET ; +else if (dev->desc == &lradc->isrc1) + offset =LRADC_CTRL2_TEMP_ISRC1_OFFSET ; +else +/* This should never happen */ +return -ENODEV ; + + value = min_uA /20 ; +if (min_uA %20 ) + value++; +if (value *20 > max_uA) +return -EINVAL ; +if (value & ~LRADC_CTRL2_TEMP_ISRC_MASK ) +/* This should never happen */ +return -EPERM ; + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_ISRC_MASK << offset, +LRADC_CTRL2 ); +mxs_lradc_reg_set (lradc, value << offset,LRADC_CTRL2 ); + +return 0 ; +} + +static struct regulator_ops mxs_lradc_regulator_current_ops = { + .enable = mxs_lradc_regulator_enable, + .is_enabled = mxs_lradc_regulator_is_enabled, + .disable = mxs_lradc_regulator_disable, + .get_current_limit = mxs_lradc_regulator_get_current_limit, + .set_current_limit = mxs_lradc_regulator_set_current_limit, +}; +/* * Driver initialization */ @@ -1519,6 +1634,10 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i)); + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 | +LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); }static const struct of_device_id mxs_lradc_dt_ids[] = {@@ -1592,6 +1711,32 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,return 0 ; } +static void mxs_lradc_reg_helper (struct device_node *np,const char *name, +struct regulator_config *conf, +struct regulator_desc *desc) +{ +struct regulator_dev *ret; + + conf->of_node =of_get_child_by_name (np, name); +if (!conf->of_node) +return ; + + desc->name = name; + desc->owner =THIS_MODULE ; + desc->type =REGULATOR_CURRENT ; + desc->ops = &mxs_lradc_regulator_current_ops; + + conf->init_data =of_get_regulator_init_data (conf->dev, conf->of_node, + desc); + ret =devm_regulator_register (conf->dev, desc, conf); +if (IS_ERR (ret)) +/* Just pretend the regulator isn't there */ +dev_err (conf->dev,"Failed to register regulator %s: %ld\n" , + desc->name,PTR_ERR (ret)); + +of_node_put (conf->of_node); +} +static int mxs_lradc_probe (struct platform_device *pdev) { const struct of_device_id *of_id =@@ -1603,6 +1748,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)struct mxs_lradc *lradc;struct iio_dev *iio;struct resource *iores; +struct regulator_config regconf;int ret =0 , touch_ret;int i, s;u64 scale_uv;@@ -1727,6 +1873,12 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; }drivers/iio/adc/mxs-lradc.c static int mxs_lradc_probe(struct platform_device *pdev) · line 1884
Quoted 10 lines
drivers/iio/adc/mxs-lradc.c +6
-0+/* Setup regulator devices for current source. */ + regconf.dev = dev; + regconf.driver_data = lradc; +mxs_lradc_reg_helper (node,"ISRC0" , ®conf, &lradc->isrc0); +mxs_lradc_reg_helper (node,"ISRC1" , ®conf, &lradc->isrc1); +return 0 ; err_ts:--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
[CCing Mark and Liam again]
Quoted 25 lines · Ksenija Stanojevi?
Quoted 24 lines · Marek Vasut
Quoted 22 lines · Harald Geyer
The hardware has two current sources ISRC0 and ISRC1 to allow measuring
resistors without additional circuitry. This commit makes them available
as regulators.Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very
well: Typically
consumers will want to set a defined current, ie.min_uA==max_uA, but they
can't without help from configuration data, because the valid values aren't
reported by the API for current regulators. I have been thinking about
extending the API, but currently AFAIK no such consumers exist and most
users, like myself, will force the regulator to a defined value in devicetree anyway.
Quoted 5 lines
Quoted 5 lines
I am tempted to block this patch and ask you to properly split the mxs-lradc driver into MFD with touchscreen and IIO part and only then
add the regulator bits. The lradc is becoming a katamari of ad-hoc misplaced functionality.
I know. However I'm afraid working this into a proper MFD is above my skills
and I don't feel responsible for the status quo either.
Quoted 2 lines
I almost finished, I already split the driver into MFD, sorry for the delay.
Wow, looks like I have a lot of luck today.
Quoted 1 line
Can this wait a little bit?
Sure. I now have a working kernel for my purposes, so no hurry on my part.
CC my when you submit your code or maybe even pick my patch into your series,
if you feel like it.
Thanks,
Harald
Quoted 327 lines
Thanks,
KsenijaQuoted 324 lines
Quoted 312 lines
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)(quoted diff) +0
-0
a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtb/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtindex 555fb11..983952c 100644
---a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
+++b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt(quoted diff) +0
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowedvalue is
1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms)
+- ISRC0: A node describing the regulator of internal current source 0
+- ISRC1: A node describing the regulator of internal current source 1Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator bindingdocumentation.
+ Valid values are from 0 to 300 in steps
of 20.Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to currentregulators.
Example for i.MX23 SoC:
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +20
-0@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFERdrivers/iio/adc/mxs-lradc.c +36
-0@@ -40,6 +40,10 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/sysfs.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#define DRIVER_NAME "mxs-lradc"#define LRADC_MAX_DELAY_CHANS 4@@ -261,6 +265,9 @@ struct mxs_lradc {unsigned over_sample_delay;/* time in clocks to wait after the plates where switched */ unsigned settling_delay; + +struct regulator_desc isrc0; +struct regulator_desc isrc1; };#define LRADC_CTRL0 0x00@@ -305,6 +312,11 @@ struct mxs_lradc {#define LRADC_CTRL2 0x20#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24#define LRADC_CTRL2_TEMPSENSE_PWD BIT(15) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE1 BIT(9) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE0 BIT(8) +#define LRADC_CTRL2_TEMP_ISRC1_OFFSET 4 +#define LRADC_CTRL2_TEMP_ISRC0_OFFSET 0 +#define LRADC_CTRL2_TEMP_ISRC_MASK 0x0f#define LRADC_STATUS 0x40#define LRADC_STATUS_TOUCH_DETECT_RAW BIT(0)@@ -1383,6 +1395,109 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { .validate_scan_mask = &mxs_lradc_validate_scan_mask, }; +static int mxs_lradc_regulator_is_enabled (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE0 ; +else if (dev->desc == &lradc->isrc1) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE1 ; + +/* This should never happen */ +return -ENODEV ; +} + +#define LRADC_REGVALUE2uA (regval, offset) \ + (20 * ((regval >> offset) & LRADC_CTRL2_TEMP_ISRC_MASK)) + +static int mxs_lradc_regulator_get_current_limit (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) + return LRADC_REGVALUE2uA(reg,
LRADC_CTRL2_TEMP_ISRC0_OFFSET);drivers/iio/adc/mxs-lradc.c +2
-0+else if (dev->desc == &lradc->isrc1) + return LRADC_REGVALUE2uA(reg,
LRADC_CTRL2_TEMP_ISRC1_OFFSET);drivers/iio/adc/mxs-lradc.c +11
-0+ +/* This should never happen */ +return -ENODEV ; +} + +static int mxs_lradc_regulator_enable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) + mxs_lradc_reg_set(lradc,
LRADC_CTRL2_TEMP_SENSOR_IENABLE0,drivers/iio/adc/mxs-lradc.c +3
-0+ LRADC_CTRL2); + else if (dev->desc == &lradc->isrc1) + mxs_lradc_reg_set(lradc,
LRADC_CTRL2_TEMP_SENSOR_IENABLE1,drivers/iio/adc/mxs-lradc.c +14
-0+ LRADC_CTRL2); + else +/* This should never happen */ + return -ENODEV; + +return 0 ; +} + +static int mxs_lradc_regulator_disable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) + mxs_lradc_reg_clear(lradc,
LRADC_CTRL2_TEMP_SENSOR_IENABLE0,drivers/iio/adc/mxs-lradc.c +3
-0+ LRADC_CTRL2); + else if (dev->desc == &lradc->isrc1) + mxs_lradc_reg_clear(lradc,
LRADC_CTRL2_TEMP_SENSOR_IENABLE1,drivers/iio/adc/mxs-lradc.c +9
-0+ LRADC_CTRL2); + else +/* This should never happen */ + return -ENODEV; + +return 0 ; +} + +static int mxs_lradc_regulator_set_current_limit(struct
regulator_dev*dev,
+ intmin_uA, intmax_uA)drivers/iio/adc/mxs-lradc.c +22
-0+{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int offset, value; + +if (dev->desc == &lradc->isrc0) + offset =LRADC_CTRL2_TEMP_ISRC0_OFFSET ; +else if (dev->desc == &lradc->isrc1) + offset =LRADC_CTRL2_TEMP_ISRC1_OFFSET ; +else +/* This should never happen */ +return -ENODEV ; + + value = min_uA /20 ; +if (min_uA %20 ) + value++; +if (value *20 > max_uA) +return -EINVAL ; +if (value & ~LRADC_CTRL2_TEMP_ISRC_MASK ) +/* This should never happen */ +return -EPERM ; + + mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_ISRC_MASK <<offset,
drivers/iio/adc/mxs-lradc.c +18
-0+ LRADC_CTRL2); +mxs_lradc_reg_set (lradc, value << offset,LRADC_CTRL2 ); + +return 0 ; +} + +static struct regulator_ops mxs_lradc_regulator_current_ops = { + .enable = mxs_lradc_regulator_enable, + .is_enabled = mxs_lradc_regulator_is_enabled, + .disable = mxs_lradc_regulator_disable, + .get_current_limit = mxs_lradc_regulator_get_current_limit, + .set_current_limit = mxs_lradc_regulator_set_current_limit, +}; +/* * Driver initialization */ @@ -1519,6 +1634,10 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i)); + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 | +LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); }static const struct
of_device_idmxs_lradc_dt_ids[] = {drivers/iio/adc/mxs-lradc.c +21
-0@@ -1592,6 +1711,32 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc, return 0; } +static void mxs_lradc_reg_helper (struct device_node *np,const char *name, +struct regulator_config *conf, +struct regulator_desc *desc) +{ +struct regulator_dev *ret; + + conf->of_node =of_get_child_by_name (np, name); +if (!conf->of_node) +return ; + + desc->name = name; + desc->owner =THIS_MODULE ; + desc->type =REGULATOR_CURRENT ; + desc->ops = &mxs_lradc_regulator_current_ops; + + conf->init_data =of_get_regulator_init_data (conf->dev, conf->of_node, + desc); + ret =devm_regulator_register (conf->dev, desc, conf); +if (IS_ERR (ret)) +/* Just pretend the regulator isn't there */ + dev_err(conf->dev, "Failed to register regulator %s:%ld\n",
drivers/iio/adc/mxs-lradc.c +6
-0+ desc->name, PTR_ERR(ret)); + +of_node_put (conf->of_node); +} +static int mxs_lradc_probe (struct platform_device *pdev) { const struct of_device_id *of_id =@@ -1603,6 +1748,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)struct mxs_lradc *lradc;struct iio_dev *iio;struct resource *iores; +struct regulator_config regconf;int ret =0 , touch_ret;int i, s;u64 scale_uv;drivers/iio/adc/mxs-lradc.c +6
-0@@ -1727,6 +1873,12 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; } +/* Setup regulator devices for current source. */ + regconf.dev = dev; + regconf.driver_data = lradc; +mxs_lradc_reg_helper (node,"ISRC0" , ®conf, &lradc->isrc0); +mxs_lradc_reg_helper (node,"ISRC1" , ®conf, &lradc->isrc1); +return 0 ;Quoted 2 lines
err_ts:--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 4 lines
If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or bitcoin 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS
Jonathan Cameron
maintainerPATCH 1/3 iio: mxs-lradc: Add regulators for current sources
Quoted 26 lines · Harald Geyer
[CCing Mark and Liam again]
Quoted 18 lines · Ksenija Stanojevi?
Quoted 17 lines · Marek Vasut
Quoted 15 lines · Harald Geyer
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie.min_uA==max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway.Quoted 4 lines
Quoted 4 lines
I am tempted to block this patch and ask you to properly split the mxs-lradc driver into MFD with touchscreen and IIO part and only then add the regulator bits. The lradc is becoming a katamari of ad-hoc misplaced functionality.
Quoted 6 lines
I know. However I'm afraid working this into a proper MFD is above my skills and I don't feel responsible for the status quo either.
Quoted 1 line
I almost finished, I already split the driver into MFD, sorry for the delay.
Wow, looks like I have a lot of luck today.
Indeed! Glad to here you have this underway Ksenija. Let me know if you want any other patches held until this is done.
For now I'm assuming that Stefan's series won't cause too much trouble, so I'll probably apply those, but will hold anything more major until your patches show up.
Thanks,
Jonathan
Quoted 305 lines
Quoted 2 lines
Can this wait a little bit?
Sure. I now have a working kernel for my purposes, so no hurry on my part. CC my when you submit your code or maybe even pick my patch into your series, if you feel like it.
Thanks,
HaraldQuoted 294 lines
Thanks,
KsenijaQuoted 291 lines
Quoted 282 lines
.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +29
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFERdrivers/iio/adc/mxs-lradc.c +152
-0@@ -40,6 +40,10 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/sysfs.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#define DRIVER_NAME "mxs-lradc"#define LRADC_MAX_DELAY_CHANS 4@@ -261,6 +265,9 @@ struct mxs_lradc {unsigned over_sample_delay;/* time in clocks to wait after the plates where switched */ unsigned settling_delay; + +struct regulator_desc isrc0; +struct regulator_desc isrc1; };#define LRADC_CTRL0 0x00@@ -305,6 +312,11 @@ struct mxs_lradc {#define LRADC_CTRL2 0x20#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24#define LRADC_CTRL2_TEMPSENSE_PWD BIT(15) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE1 BIT(9) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE0 BIT(8) +#define LRADC_CTRL2_TEMP_ISRC1_OFFSET 4 +#define LRADC_CTRL2_TEMP_ISRC0_OFFSET 0 +#define LRADC_CTRL2_TEMP_ISRC_MASK 0x0f#define LRADC_STATUS 0x40#define LRADC_STATUS_TOUCH_DETECT_RAW BIT(0)@@ -1383,6 +1395,109 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { .validate_scan_mask = &mxs_lradc_validate_scan_mask, }; +static int mxs_lradc_regulator_is_enabled (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE0 ; +else if (dev->desc == &lradc->isrc1) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE1 ; + +/* This should never happen */ +return -ENODEV ; +} + +#define LRADC_REGVALUE2uA (regval, offset) \ + (20 * ((regval >> offset) & LRADC_CTRL2_TEMP_ISRC_MASK)) + +static int mxs_lradc_regulator_get_current_limit (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC0_OFFSET ); +else if (dev->desc == &lradc->isrc1) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC1_OFFSET ); + +/* This should never happen */ +return -ENODEV ; +} + +static int mxs_lradc_regulator_enable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_disable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_set_current_limit (struct regulator_dev *dev, +int min_uA,int max_uA) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int offset, value; + +if (dev->desc == &lradc->isrc0) + offset =LRADC_CTRL2_TEMP_ISRC0_OFFSET ; +else if (dev->desc == &lradc->isrc1) + offset =LRADC_CTRL2_TEMP_ISRC1_OFFSET ; +else +/* This should never happen */ +return -ENODEV ; + + value = min_uA /20 ; +if (min_uA %20 ) + value++; +if (value *20 > max_uA) +return -EINVAL ; +if (value & ~LRADC_CTRL2_TEMP_ISRC_MASK ) +/* This should never happen */ +return -EPERM ; + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_ISRC_MASK << offset, +LRADC_CTRL2 ); +mxs_lradc_reg_set (lradc, value << offset,LRADC_CTRL2 ); + +return 0 ; +} + +static struct regulator_ops mxs_lradc_regulator_current_ops = { + .enable = mxs_lradc_regulator_enable, + .is_enabled = mxs_lradc_regulator_is_enabled, + .disable = mxs_lradc_regulator_disable, + .get_current_limit = mxs_lradc_regulator_get_current_limit, + .set_current_limit = mxs_lradc_regulator_set_current_limit, +}; +/* * Driver initialization */ @@ -1519,6 +1634,10 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i)); + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 | +LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); }static const struct of_device_id mxs_lradc_dt_ids[] = {@@ -1592,6 +1711,32 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,return 0 ; } +static void mxs_lradc_reg_helper (struct device_node *np,const char *name, +struct regulator_config *conf, +struct regulator_desc *desc) +{ +struct regulator_dev *ret; + + conf->of_node =of_get_child_by_name (np, name); +if (!conf->of_node) +return ; + + desc->name = name; + desc->owner =THIS_MODULE ; + desc->type =REGULATOR_CURRENT ; + desc->ops = &mxs_lradc_regulator_current_ops; + + conf->init_data =of_get_regulator_init_data (conf->dev, conf->of_node, + desc); + ret =devm_regulator_register (conf->dev, desc, conf); +if (IS_ERR (ret)) +/* Just pretend the regulator isn't there */ +dev_err (conf->dev,"Failed to register regulator %s: %ld\n" , + desc->name,PTR_ERR (ret)); + +of_node_put (conf->of_node); +} +static int mxs_lradc_probe (struct platform_device *pdev) { const struct of_device_id *of_id =@@ -1603,6 +1748,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)struct mxs_lradc *lradc;struct iio_dev *iio;struct resource *iores; +struct regulator_config regconf;int ret =0 , touch_ret;int i, s;u64 scale_uv;@@ -1727,6 +1873,12 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; } +/* Setup regulator devices for current source. */ + regconf.dev = dev; + regconf.driver_data = lradc; +mxs_lradc_reg_helper (node,"ISRC0" , ®conf, &lradc->isrc0); +mxs_lradc_reg_helper (node,"ISRC1" , ®conf, &lradc->isrc1); +return 0 ; err_ts:Signature · 6 lines
Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
In reply to Harald GeyerCCing Mark and Liam.
I guess I shouldn't rely blindly on get_maintainer.pl ...
Quoted 322 lines · Harald Geyer
The hardware has two current sources ISRC0 and ISRC1 to allow measuring
resistors without additional circuitry. This commit makes them available
as regulators.Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very well:
Typically
consumers will want to set a defined current, ie.min_uA==max_uA, but they
can't without help from configuration data, because the valid values aren't
reported by the API for current regulators. I have been thinking about
extending the API, but currently AFAIK no such consumers exist and most
users, like myself, will force the regulator to a defined value in devicetree anyway..../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)diff --git
a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtb/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtindex 555fb11..983952c 100644Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +0
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowedvalue is
1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms)
+- ISRC0: A node describing the regulator of internal current source 0
+- ISRC1: A node describing the regulator of internal current source 1Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator bindingdocumentation.
+ Valid values are from 0 to 300 in steps of
20.Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +23
-0+ +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; }; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFERdrivers/iio/adc/mxs-lradc.c +115
-0@@ -40,6 +40,10 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/sysfs.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/of_regulator.h> +#define DRIVER_NAME "mxs-lradc"#define LRADC_MAX_DELAY_CHANS 4@@ -261,6 +265,9 @@ struct mxs_lradc {unsigned over_sample_delay;/* time in clocks to wait after the plates where switched */ unsigned settling_delay; + +struct regulator_desc isrc0; +struct regulator_desc isrc1; };#define LRADC_CTRL0 0x00@@ -305,6 +312,11 @@ struct mxs_lradc {#define LRADC_CTRL2 0x20#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24#define LRADC_CTRL2_TEMPSENSE_PWD BIT(15) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE1 BIT(9) +#define LRADC_CTRL2_TEMP_SENSOR_IENABLE0 BIT(8) +#define LRADC_CTRL2_TEMP_ISRC1_OFFSET 4 +#define LRADC_CTRL2_TEMP_ISRC0_OFFSET 0 +#define LRADC_CTRL2_TEMP_ISRC_MASK 0x0f#define LRADC_STATUS 0x40#define LRADC_STATUS_TOUCH_DETECT_RAW BIT(0)@@ -1383,6 +1395,109 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { .validate_scan_mask = &mxs_lradc_validate_scan_mask, }; +static int mxs_lradc_regulator_is_enabled (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE0 ; +else if (dev->desc == &lradc->isrc1) +return reg &LRADC_CTRL2_TEMP_SENSOR_IENABLE1 ; + +/* This should never happen */ +return -ENODEV ; +} + +#define LRADC_REGVALUE2uA (regval, offset) \ + (20 * ((regval >> offset) & LRADC_CTRL2_TEMP_ISRC_MASK)) + +static int mxs_lradc_regulator_get_current_limit (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int reg =readl (lradc->base +LRADC_CTRL2 ); + +if (dev->desc == &lradc->isrc0) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC0_OFFSET ); +else if (dev->desc == &lradc->isrc1) +return LRADC_REGVALUE2uA (reg,LRADC_CTRL2_TEMP_ISRC1_OFFSET ); + +/* This should never happen */ +return -ENODEV ; +} + +static int mxs_lradc_regulator_enable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_set (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_disable (struct regulator_dev *dev) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); + +if (dev->desc == &lradc->isrc0) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 , +LRADC_CTRL2 ); +else if (dev->desc == &lradc->isrc1) +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); +else +/* This should never happen */ +return -ENODEV ; + +return 0 ; +} + +static int mxs_lradc_regulator_set_current_limit (struct regulator_dev *dev, +int min_uA,int max_uA) +{ +struct mxs_lradc *lradc =rdev_get_drvdata (dev); +int offset, value; + +if (dev->desc == &lradc->isrc0) + offset =LRADC_CTRL2_TEMP_ISRC0_OFFSET ; +else if (dev->desc == &lradc->isrc1) + offset =LRADC_CTRL2_TEMP_ISRC1_OFFSET ; +else +/* This should never happen */ +return -ENODEV ; + + value = min_uA /20 ; +if (min_uA %20 ) + value++; +if (value *20 > max_uA) +return -EINVAL ; +if (value & ~LRADC_CTRL2_TEMP_ISRC_MASK ) +/* This should never happen */ +return -EPERM ; + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_ISRC_MASK << offset, +LRADC_CTRL2 ); +mxs_lradc_reg_set (lradc, value << offset,LRADC_CTRL2 ); + +return 0 ; +} + +static struct regulator_ops mxs_lradc_regulator_current_ops = { + .enable = mxs_lradc_regulator_enable, + .is_enabled = mxs_lradc_regulator_is_enabled, + .disable = mxs_lradc_regulator_disable, + .get_current_limit = mxs_lradc_regulator_get_current_limit, + .set_current_limit = mxs_lradc_regulator_set_current_limit, +}; +/*
* Driver initialization
*/drivers/iio/adc/mxs-lradc.c +4
-0@@ -1519,6 +1634,10 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)for (i =0 ; i <LRADC_MAX_DELAY_CHANS ; i++)mxs_lradc_reg_wrt (lradc,0 ,LRADC_DELAY (i)); + +mxs_lradc_reg_clear (lradc,LRADC_CTRL2_TEMP_SENSOR_IENABLE0 | +LRADC_CTRL2_TEMP_SENSOR_IENABLE1 , +LRADC_CTRL2 ); }static const struct
of_device_idmxs_lradc_dt_ids[] = {drivers/iio/adc/mxs-lradc.c +26
-0@@ -1592,6 +1711,32 @@ static int mxs_lradc_probe_touchscreen(structmxs_lradc *lradc, return 0; } +static void mxs_lradc_reg_helper (struct device_node *np,const char *name, +struct regulator_config *conf, +struct regulator_desc *desc) +{ +struct regulator_dev *ret; + + conf->of_node =of_get_child_by_name (np, name); +if (!conf->of_node) +return ; + + desc->name = name; + desc->owner =THIS_MODULE ; + desc->type =REGULATOR_CURRENT ; + desc->ops = &mxs_lradc_regulator_current_ops; + + conf->init_data =of_get_regulator_init_data (conf->dev, conf->of_node, + desc); + ret =devm_regulator_register (conf->dev, desc, conf); +if (IS_ERR (ret)) +/* Just pretend the regulator isn't there */ +dev_err (conf->dev,"Failed to register regulator %s: %ld\n" , + desc->name,PTR_ERR (ret)); + +of_node_put (conf->of_node); +} +static int
mxs_lradc_probe(struct platform_device *pdev){drivers/iio/adc/mxs-lradc.c +1
-0const struct of_device_id *of_id =@@ -1603,6 +1748,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)struct mxs_lradc *lradc;struct iio_dev *iio;struct resource *iores; +struct regulator_config regconf;int ret =0 , touch_ret;int i, s;drivers/iio/adc/mxs-lradc.c +6
-0u64 scale_uv;@@ -1727,6 +1873,12 @@ static int mxs_lradc_probe(struct platform_device *pdev) goto err_ts; } +/* Setup regulator devices for current source. */ + regconf.dev = dev; + regconf.driver_data = lradc; +mxs_lradc_reg_helper (node,"ISRC0" , ®conf, &lradc->isrc0); +mxs_lradc_reg_helper (node,"ISRC1" , ®conf, &lradc->isrc1); +return 0 ;
err_ts:
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
In reply to Harald GeyerHi Harald,
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 45 lines
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie.min_uA==max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway..../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +9
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>;
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt Example for i.MX23 SoC: · line 52
Quoted 10 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +10
-0+ + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + };
IMHO the binding should represent the direct connection between the current source and the physical channel. I think a node label isn't enough. Unfortunately i don't have an idea to do it the "right way".
Quoted 23 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +10
-0}; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +0
-0
drivers/iio/adc/Kconfig config MXS_LRADC · line 322
Quoted 4 lines
drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATOR
Any chance to avoid this dependency for an optional feature?
Stefan
Quoted 3 lines
drivers/iio/adc/Kconfig +0
-0select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
Hi Stefan!
Quoted 69 lines · Stefan Wahren
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 68 lines
The hardware has two current sources ISRC0 and ISRC1 to allow measuring
resistors without additional circuitry. This commit makes them available
as regulators.Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very well:
Typically
consumers will want to set a defined current, ie.min_uA==max_uA, but they
can't without help from configuration data, because the valid values aren't
reported by the API for current regulators. I have been thinking about
extending the API, but currently AFAIK no such consumers exist and most
users, like myself, will force the regulator to a defined value in devicetree anyway..../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)(quoted diff) +0
-0
a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtb/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtindex 555fb11..983952c 100644
---a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
+++b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt(quoted diff) +0
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowedvalue is
1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms)
+- ISRC0: A node describing the regulator of internal current source 0
+- ISRC1: A node describing the regulator of internal current source 1Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator bindingdocumentation.
+ Valid values are from 0 to 300 in steps
of 20.Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +9
-0+ +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 {
Quoted 8 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt Example for i.MX23 SoC: · line 52
Quoted 4 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +4
-0+ regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + };IMHO the binding should represent the direct connection between the current source and the physical channel. I think a node label isn't enough. Unfortunately i don't have an idea to do it the "right way".
I strictly followed the names of the data sheet here: imx23 and imx28 reference manuals both call the current sources ISRC0 and ISRC1 even if they are attached to different channels of the ADC. I think inventing some ISRC6 for imx28 would do more harm then good.
Quoted 21 lines
Quoted 21 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +10
-0}; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +0
-1--- a/drivers/iio/adc/Kconfig
Quoted 8 lines
drivers/iio/adc/Kconfig config MXS_LRADC · line 322
Quoted 6 lines
+++ b/drivers/iio/adc/Kconfig
drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATORAny chance to avoid this dependency for an optional feature?
I haven't looked into it yet, but I expect that that the mfd conversion will avoid this neatly.
Thanks,
Harald
Quoted 5 lines
Stefan
Quoted 3 lines
drivers/iio/adc/Kconfig +0
-0select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER
Signature · 4 lines
If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or bitcoin 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS
Jonathan Cameron
maintainerPATCH 1/3 iio: mxs-lradc: Add regulators for current sources
Quoted 64 lines · Harald Geyer
Hi Stefan!
Quoted 52 lines · Stefan Wahren
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 51 lines
The hardware has two current sources ISRC0 and ISRC1 to allow measuring resistors without additional circuitry. This commit makes them available as regulators.
Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very well: Typically consumers will want to set a defined current, ie.min_uA==max_uA, but they can't without help from configuration data, because the valid values aren't reported by the API for current regulators. I have been thinking about extending the API, but currently AFAIK no such consumers exist and most users, like myself, will force the regulator to a defined value in devicetree anyway..../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +15
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample. Allowed value is 1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms) +- ISRC0: A node describing the regulator of internal current source 0 +- ISRC1: A node describing the regulator of internal current source 1 + +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator binding documentation. + Valid values are from 0 to 300 in steps of 20. + +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to current regulators. Example for i.MX23 SoC:@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 {Quoted 8 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt Example for i.MX23 SoC: · line 52
Quoted 4 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +4
-0+ regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + };IMHO the binding should represent the direct connection between the current source and the physical channel. I think a node label isn't enough. Unfortunately i don't have an idea to do it the "right way".
Quoted 4 lines
I strictly followed the names of the data sheet here: imx23 and imx28 reference manuals both call the current sources ISRC0 and ISRC1 even if they are attached to different channels of the ADC. I think inventing some ISRC6 for imx28 would do more harm then good.
So just to check... These two current sources are effectively pushing current out of the ADC input pin? Assumption being that this causes an appropriate voltage across a component between there and 0V?
I wonder if we are better off not treating these as regulators at all - whilst you could in theory use them for general purposes, it would be a bit nuts and sometimes the 'it is nuts argument' is enough to mean we don't support something :)
I wouldn't have any problem with an additional attribute for the adc channels - maybe something like...
in_voltage6_sourcecurrent? Then the control could be pushed to userspace.
Or if it only makes sense to do it as a fixed value in DT then define optional child nodes for the individual channels, with the relevant ones have current-source-microamp or something like that as an parameter.
What do you think?
Jonathan
Quoted 43 lines
Quoted 22 lines
Quoted 22 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +10
-0}; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +0
-1--- a/drivers/iio/adc/KconfigQuoted 8 lines
drivers/iio/adc/Kconfig config MXS_LRADC · line 322
Quoted 6 lines
+++ b/drivers/iio/adc/Kconfig
drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATORAny chance to avoid this dependency for an optional feature?
I haven't looked into it yet, but I expect that that the mfd conversion will avoid this neatly.
Thanks,
HaraldQuoted 5 lines
Stefan
Quoted 3 lines
drivers/iio/adc/Kconfig +0
-0select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER
PATCH 1/3 iio: mxs-lradc: Add regulators for current sources
Quoted 87 lines · Jonathan Cameron
Quoted 81 lines · Harald Geyer
Quoted 71 lines · Stefan Wahren
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 70 lines
The hardware has two current sources ISRC0 and ISRC1 to allow measuring
resistors without additional circuitry. This commit makes them available
as regulators.Tested on an imx233-olinuxino board.
Signed-off-by Harald Geyer <[email protected]>---
The current regulator API doesn't fit this type of device very
well: Typically
consumers will want to set a defined current, ie.min_uA==max_uA, but they
can't without help from configuration data, because the valid values aren't
reported by the API for current regulators. I have been thinking about
extending the API, but currently AFAIK no such consumers exist and most
users, like myself, will force the regulator to a defined value in devicetree anyway..../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 3 files changed, 182 insertions(+)(quoted diff) +0
-0
a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtb/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txtindex 555fb11..983952c 100644
---a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
+++b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt(quoted diff) +0
-0@@ -19,6 +19,15 @@ Optional properties: - fsl,settling: delay between plate switch to next sample.Allowed value is
1 ... 2047. It counts at 2 kHz and its default is 10 (= 5 ms)
+- ISRC0: A node describing the regulator of internal current source 0
+- ISRC1: A node describing the regulator of internal current source 1Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Required properties for the ISRCx sub-nodes: +- regulator-max-microamp: See standard regulator bindingdocumentation.
+ Valid values are from 0 to 300 in steps
of 20.Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ +Optional properties for the ISRCx sub-nodes: +Any standard regulator properties that apply to currentregulators.
Example for i.MX23 SoC:
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +7
-0@@ -31,6 +40,16 @@ Example for i.MX23 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <120>;Quoted 8 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt Example for i.MX23 SoC: · line 52
Quoted 3 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +3
-0+ regulator-min-microamp = <120>; + regulator-always-on; + };IMHO the binding should represent the direct connection between the current source and the physical channel. I think a node label isn't enough. Unfortunately i don't have an idea to do it the "right way".
Quoted 5 lines
I strictly followed the names of the data sheet here: imx23 and imx28
reference manuals both call the current sources ISRC0 and ISRC1 even if they are attached to different channels of the ADC. I think inventing some ISRC6 for imx28 would do more harm then good.
Quoted 4 lines
So just to check... These two current sources are effectively pushing current out of the ADC input pin? Assumption being that this causes an appropriate voltage across a component between there and 0V?
Exactly.
Quoted 4 lines
I wonder if we are better off not treating these as regulators at all - whilst you could in theory use them for general purposes, it would be a bit nuts and sometimes the 'it is nuts argument' is enough to mean we don't support something :)
Yeah, it would be some funny 4-bit DAC ...
Quoted 5 lines
I wouldn't have any problem with an additional attribute for the adc channels - maybe something like...
in_voltage6_sourcecurrent? Then the control could be pushed to userspace.
I'm very open to that idea. Not having yet an other subsystem involved would simplify some things a lot.
Quoted 4 lines
Or if it only makes sense to do it as a fixed value in DT then define optional child nodes for the individual channels, with the relevant ones have current-source-microamp or something like that as an parameter.
My research in thermistors is still ongoing (so I don't have a kernel driver yet) but I think for thermistors with exponential temperature dependence dynamic setting of the current source will be necessary to get high resolution across the whole temperature range.
Of course many applications wont need this. For checks like "Is my battery over-heating?" a simple fixed current from DT will to well, so I guess we should support both.
Quoted 1 line
What do you think?
As I already stated in an other subthread, I'm not entirely happy with the regulator approach. Having everything inside iio would make supporting complex use cases easier in the future, I guess.
Also IIO devices are much easier to control from user space then regulators, which might be desired for this type of device.
OTOH I'm not sure how many other devices out there have a similar feature. I'd rather not invent a new API for just very few devices. IMO iio already suffers from the complexity of the sysfs API. Maybe somebody with a good overview of ADCs available on the market can provide some input here.
Also of course there are lots of ADCs out there without internal current source and those could be used with thermistors too but would need a regulator anyway. So maybe eventually we would end up with both solutions supported, which I'd avoid if at all possible. (I guess we could work around the last issue by providing some generic driver to pull an iio channel and a regulator together - do we like that level of abstraction?)
If we start our own API to control current sources we might end up pretty much reimplementing the regulator API for the single consumer case.
I'd like to have input from more people on the above points before going one way or the other.
Thanks,
Harald
Quoted 45 lines
Jonathan
Quoted 43 lines
Quoted 22 lines
Quoted 22 lines
Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt +10
-0}; Example for i.MX28 SoC:@@ -44,4 +63,14 @@ Example for i.MX28 SoC: fsl,ave-ctrl = <4>; fsl,ave-delay = <2>; fsl,settling = <10>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <120>; + regulator-min-microamp = <120>; + regulator-always-on; + }; };drivers/iio/adc/Kconfig +0
-1--- a/drivers/iio/adc/KconfigQuoted 8 lines
drivers/iio/adc/Kconfig config MXS_LRADC · line 322
Quoted 6 lines
+++ b/drivers/iio/adc/Kconfig
drivers/iio/adc/Kconfig +1
-0@@ -319,6 +319,7 @@ config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on (ARCH_MXS || COMPILE_TEST) && HAS_IOMEM depends on INPUT + depends on REGULATORAny chance to avoid this dependency for an optional feature?
I haven't looked into it yet, but I expect that that the mfd conversion will avoid this neatly.
Thanks,
HaraldQuoted 5 lines
Stefan
Quoted 3 lines
drivers/iio/adc/Kconfig +0
-0select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER
Signature · 4 lines
If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or bitcoin 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS
PATCH 2/3 ARM: dts: imx23: Provide regulators for the current sources of the LRADC
In reply to Harald GeyerThis adds the devicetree nodes to make the mxs-lradc driver register regulator devices for the integrated current sources.
Tested on an imx23-olinuxino board.
---
Note: with this configuration the regulator devices will be present but disabled by default.
arch/arm/boot/dts/imx23.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/arm/boot/dts/imx23.dtsi +8 -0
@@ -471,6 +471,14 @@ status = "disabled"; clocks = <&clks 26>; #io-channel-cells = <1>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc1: ISRC1 { + regulator-max-microamp = <300>; + }; }; spdif at 80054000 {
Signature · 1 line
2.1.4
PATCH 3/3 ARM: dts: imx28: Provide regulators for the current sources of the LRADC
In reply to Harald GeyerThis adds the devicetree nodes to make the mxs-lradc driver register regulator devices for the integrated current sources.
---
I couln't test this patch on actual HW, but it is a straight forward port from the imx23 case. With only a funny change in the node label, to match the naming convention of the IMX28 Reference Manual.
arch/arm/boot/dts/imx28.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/arm/boot/dts/imx28.dtsi +8 -0
@@ -1070,6 +1070,14 @@ status = "disabled"; clocks = <&clks 41>; #io-channel-cells = <1>; + + isrc_lradc0: ISRC0 { + regulator-max-microamp = <300>; + }; + + isrc_lradc6: ISRC1 { + regulator-max-microamp = <300>; + }; }; spdif: spdif at 80054000 {
Signature · 1 line
2.1.4
cover mxs-lradc: Add support for current sources
In reply to Harald GeyerHi Harald,
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 10 lines
Patch 1/3 changes the driver and updates the binding documentation (I guess it is still in staging.)
Patches 2/3 and 3/3 add the devicetree nodes to imx23 and imx28 boards. I'd like to get input whether this is actually desired. On boards where these regulators would never be enabled this costs a few extra bytes of RAM for allocation of the device data, because the nodes can't be easily removed in .dts files which are including the .dtsi files. The alternative is to add the new nodes to many .dts files, which would be a lot code duplication.
if i get it right the real intention of this patch series is to make the mxs-lradc provide resistance values instead of voltages. So how about dropping the whole regulator stuff and provide the values as IIO_RESISTANCE via iio interface?
Btw this feature should be only added to dts files where is actually used.
Regards
Stefan
Quoted 15 lines
Harald Geyer (3):
iio: mxs-lradc: Add regulators for current sources
ARM: dts: imx23: Provide regulators for the current sources of the
LRADC
ARM: dts: imx28: Provide regulators for the current sources of the
LRADC.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ arch/arm/boot/dts/imx23.dtsi | 8 ++ arch/arm/boot/dts/imx28.dtsi | 8 ++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 5 files changed, 198 insertions(+)
cover mxs-lradc: Add support for current sources
Hi Stefan!
Quoted 20 lines · Stefan Wahren
Hi Harald,
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 16 lines
Patch 1/3 changes the driver and updates the binding documentation (I guess it is still in staging.)
Patches 2/3 and 3/3 add the devicetree nodes to imx23 and imx28 boards.
I'd like to get input whether this is actually desired. On boards where
these regulators would never be enabled this costs a few extra bytes of
RAM for allocation of the device data, because the nodes can't be easily
removed in .dts files which are including the .dtsi files. The alternative
is to add the new nodes to many .dts files, which would be a lot code
duplication.
Quoted 3 lines
if i get it right the real intention of this patch series is to make the
mxs-lradc provide resistance values instead of voltages.
... or even temperature values if a thermistor is connected.
Quoted 3 lines
So how about
dropping the whole regulator stuff and provide the values asIIO_RESISTANCEvia iio interface?
Can you elaborate on this?
My thinking is that there should be a thermistor (or whatever else) driver,
that is a consumer of the regulator and a consumer of the IIO_VOLTAGE iio channel and provides a new device with an IIO_RESISTANCE or IIO_TEMP
channel. Maybe there is a simpler solution, that I'm missing?
Actually I'm not 100% happy with the above solution myself, because if we
start supporting devices that act as an iio-multiplexer (some device that
is an iio consumer and provides many new iio channels and can control via gpios which of it child channels is actually routed to the upstream device) I don't know how to properly manage the regulator device. However since this is only hypothetical ATM, I think we don't have to worry about this too much.
Quoted 2 lines
Btw this feature should be only added to dts files where is actually used.
Ok, so how do we figure out which boards these are?
I use this on the olinuxino and I guess all evaluation boards support the
use case too. The others I don't know enough about to be sure and I guess
it's not a good idea to just wait until somebody speaks up and complains
that the feature is missing on board X ...
Thanks,
Harald
Quoted 19 lines
Regards
StefanQuoted 16 lines
Harald Geyer (3):
iio: mxs-lradc: Add regulators for current sources
ARM: dts: imx23: Provide regulators for the current sources of the
LRADC
ARM: dts: imx28: Provide regulators for the current sources of the
LRADC.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ arch/arm/boot/dts/imx23.dtsi | 8 ++ arch/arm/boot/dts/imx28.dtsi | 8 ++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 5 files changed, 198 insertions(+)
Signature · 4 lines
If you want to support my work: see http://friends.ccbib.org/harald/supporting/ or donate via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w or bitcoin 1FUtd8T9jRN1rFz63vZz7s2fDtB6d6A7aS
Jonathan Cameron
maintainercover mxs-lradc: Add support for current sources
Quoted 26 lines · Harald Geyer
Hi Stefan!
Quoted 14 lines · Stefan Wahren
Hi Harald,
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 10 lines
Patch 1/3 changes the driver and updates the binding documentation (I guess it is still in staging.)
Patches 2/3 and 3/3 add the devicetree nodes to imx23 and imx28 boards. I'd like to get input whether this is actually desired. On boards where these regulators would never be enabled this costs a few extra bytes of RAM for allocation of the device data, because the nodes can't be easily removed in .dts files which are including the .dtsi files. The alternative is to add the new nodes to many .dts files, which would be a lot code duplication.
Quoted 2 lines
if i get it right the real intention of this patch series is to make the mxs-lradc provide resistance values instead of voltages.
... or even temperature values if a thermistor is connected.
Quoted 3 lines
So how about
dropping the whole regulator stuff and provide the values asIIO_RESISTANCEvia iio interface?
Quoted 6 lines
Can you elaborate on this?
My thinking is that there should be a thermistor (or whatever else) driver, that is a consumer of the regulator and a consumer of the
IIO_VOLTAGEiio channel and provides a new device with anIIO_RESISTANCEorIIO_TEMPchannel. Maybe there is a simpler solution, that I'm missing?
I agree with that approach - need a way to describe the upstream hardware - so will need a thermistor driver
Quoted 6 lines
Actually I'm not 100% happy with the above solution myself, because if we start supporting devices that act as an iio-multiplexer (some device that is an iio consumer and provides many new iio channels and can control via gpios which of it child channels is actually routed to the upstream device) I don't know how to properly manage the regulator device.
Hmm. Are you talking about muxing the regulator as well? That will get complex indeed. Might be possible to cheat and imply the regulators are always connected to all inputs... or do you want to dynamically change the regulator output? That gets messy fast - though in theory might be possible...
Quoted 33 lines
However since this is only hypothetical ATM, I think we don't have to worry about this too much.
Quoted 1 line
Btw this feature should be only added to dts files where is actually used.
Ok, so how do we figure out which boards these are?
I use this on the olinuxino and I guess all evaluation boards support the use case too. The others I don't know enough about to be sure and I guess it's not a good idea to just wait until somebody speaks up and complains that the feature is missing on board X ...Thanks,
HaraldQuoted 18 lines
Regards
StefanQuoted 15 lines
Harald Geyer (3):
iio: mxs-lradc: Add regulators for current sources
ARM: dts: imx23: Provide regulators for the current sources of the
LRADC
ARM: dts: imx28: Provide regulators for the current sources of the
LRADC.../bindings/staging/iio/adc/mxs-lradc.txt | 29 ++++ arch/arm/boot/dts/imx23.dtsi | 8 ++ arch/arm/boot/dts/imx28.dtsi | 8 ++ drivers/iio/adc/Kconfig | 1 + drivers/iio/adc/mxs-lradc.c | 152 +++++++++++++++++++++ 5 files changed, 198 insertions(+)
cover mxs-lradc: Add support for current sources
Quoted 21 lines · Jonathan Cameron
Quoted 7 lines · Harald Geyer
My thinking is that there should be a thermistor (or whatever else) driver,
that is a consumer of the regulator and a consumer of theIIO_VOLTAGE
iio channel and provides a new device with anIIO_RESISTANCEorIIO_TEMP
channel. Maybe there is a simpler solution, that I'm missing?I agree with that approach - need a way to describe the upstream hardware - so
will need a thermistor driverQuoted 10 lines
Actually I'm not 100% happy with the above solution myself, because if we
start supporting devices that act as an iio-multiplexer (some device that
is an iio consumer and provides many new iio channels and can control
via gpios which of it child channels is actually routed to the upstream
device) I don't know how to properly manage the regulator device.
Quoted 9 lines
Hmm. Are you talking about muxing the regulator as well? That will get
complex indeed. Might be possible to cheat and imply the regulators are
always connected to all inputs... or do you want to dynamically change
the regulator output? That gets messy fast - though in theory might be
possible...
Yes, that's a good summary of what I'm thinking. As there is no iio-multiplexer yet, this is sort of an academic question. But it's kind of an obvious thing to want, so we shouldn't be surprised when somebody actually starts working on that.
Thanks,
Harald
cover mxs-lradc: Add support for current sources
In reply to Jonathan CameronHi Harald,
Am 01.05.2016 um 20:02 schrieb Jonathan Cameron:
Quoted 24 lines · Harald Geyer
Quoted 23 lines
Hi Stefan!
Quoted 13 lines · Stefan Wahren
Hi Harald,
Am 22.04.2016 um 15:52 schrieb Harald Geyer:
Quoted 10 lines
Patch 1/3 changes the driver and updates the binding documentation (I guess it is still in staging.)
Patches 2/3 and 3/3 add the devicetree nodes to imx23 and imx28 boards. I'd like to get input whether this is actually desired. On boards where these regulators would never be enabled this costs a few extra bytes of RAM for allocation of the device data, because the nodes can't be easily removed in .dts files which are including the .dtsi files. The alternative is to add the new nodes to many .dts files, which would be a lot code duplication.
Quoted 2 lines
if i get it right the real intention of this patch series is to make the mxs-lradc provide resistance values instead of voltages.
... or even temperature values if a thermistor is connected.
Quoted 3 lines
So how about
dropping the whole regulator stuff and provide the values asIIO_RESISTANCEvia iio interface?
Quoted 8 lines
Quoted 6 lines
Can you elaborate on this?
My thinking is that there should be a thermistor (or whatever else) driver, that is a consumer of the regulator and a consumer of the
IIO_VOLTAGEiio channel and provides a new device with anIIO_RESISTANCEorIIO_TEMPchannel. Maybe there is a simpler solution, that I'm missing?I agree with that approach - need a way to describe the upstream hardware - so will need a thermistor driver
okay, this makes more sense to me. In this case it would be good to have this new driver in this patch series. So it's possible to verify the matching binding.
Quoted 5 lines
Quoted 5 lines
Actually I'm not 100% happy with the above solution myself, because if we start supporting devices that act as an iio-multiplexer (some device that is an iio consumer and provides many new iio channels and can control via gpios which of it child channels is actually routed to the upstream device) I don't know how to properly manage the regulator device.
Quoted 14 lines
Hmm. Are you talking about muxing the regulator as well? That will get complex indeed. Might be possible to cheat and imply the regulators are always connected to all inputs... or do you want to dynamically change the regulator output? That gets messy fast - though in theory might be possible...
Quoted 9 lines
However since this is only hypothetical ATM, I think we don't have to worry about this too much.
Quoted 1 line
Btw this feature should be only added to dts files where is actually used.
Ok, so how do we figure out which boards these are?
I use this on the olinuxino and I guess all evaluation boards support the use case too. The others I don't know enough about to be sure and I guess it's not a good idea to just wait until somebody speaks up and complains that the feature is missing on board X ...
The imx23 dtsi file specifies features on SoC level and i think this should be kept. We shouldn't implement features for ADC pins without deeper knowledge about these boards.
Maybe we could introduce a new dtsi file in order to reduce copy & paste. But at first the OlinuXino should be a good starting point ...
Regards
Stefan