Add devicetree support so that consumers can reference the channels
via devicetree, especially the power subdevice can make use of that
to provide voltage_now properties.
Andreas Kemnade (4):
dt-bindings: mfd: ricoh,rn5t618: ADC related nodes and properties
mfd: rn5t618: Add of compatibles for ADC and power
iio: rn5t618: Add devicetree support
power: supply: rn5t618: Add voltage_now property
.../bindings/mfd/ricoh,rn5t618.yaml | 53 ++++++++++++++++++
drivers/iio/adc/rn5t618-adc.c | 14 ++++-
drivers/mfd/rn5t618.c | 6 +-
drivers/power/supply/rn5t618_power.c | 56 +++++++++++++++++++
4 files changed, 126 insertions(+), 3 deletions(-)
--
2.30.2
Add ADC related nodes and properties. This will allow to wire
up ADC channels to consumers, especially to measure input voltages
by the power subdevice.
Signed-off-by: Andreas Kemnade <[email protected]>
---
.../bindings/mfd/ricoh,rn5t618.yaml | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
index 032a7fb0b4a7..185f87a14a54 100644
--- a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
+++ b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
@@ -73,6 +73,48 @@ properties:
description: |
See Documentation/devicetree/bindings/power/power-controller.txt
+ adc:
+ type: object
+
+ properties:
+ compatible:
+ enum:
+ - ricoh,rn5t618-adc
+ - ricoh,rc5t619-adc
+
+ "#io-channel-cells":
+ const: 1
+
+ additionalProperties: false
+
+ required:
+ - compatible
+ - "#io-channel-cells"
+
+ power:
+ type: object
+
+ properties:
+ compatible:
+ enum:
+ - ricoh,rn5t618-power
+ - ricoh,rc5t619-power
+
+ io-channels:
+ items:
+ - description: ADP Voltage Channel
+ - description: USB Voltage Channel
+
+ io-channel-names:
+ items:
+ - const: vadp
+ - const: vusb
+
+ additionalProperties: false
+
+ required:
+ - compatible
+
regulators:
type: object
@@ -96,6 +138,17 @@ examples:
interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
system-power-controller;
+ rn5t618_adc: adc {
+ compatible = "ricoh,rn5t618-adc";
+ #io-channel-cells = <1>;
+ };
+
+ power {
+ compatible = "ricoh,rn5t618-power";
+ io-channels = <&rn5t618_adc 2>, <&rn5t618_adc 3>;
+ io-channel-names = "vadp", "vusb";
+ };
+
regulators {
DCDC1 {
regulator-min-microvolt = <1050000>;
--
2.30.2
Read voltage_now via IIO and provide the property.
Signed-off-by: Andreas Kemnade <[email protected]>
---
drivers/power/supply/rn5t618_power.c | 56 ++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/drivers/power/supply/rn5t618_power.c b/drivers/power/supply/rn5t618_power.c
index 819061918b2a..b062208c8a91 100644
--- a/drivers/power/supply/rn5t618_power.c
+++ b/drivers/power/supply/rn5t618_power.c
@@ -9,10 +9,12 @@
#include <linux/device.h>
#include <linux/bitops.h>
#include <linux/errno.h>
+#include <linux/iio/consumer.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/mfd/rn5t618.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/regmap.h>
@@ -64,6 +66,8 @@ struct rn5t618_power_info {
struct power_supply *battery;
struct power_supply *usb;
struct power_supply *adp;
+ struct iio_channel *channel_vusb;
+ struct iio_channel *channel_vadp;
int irq;
};
@@ -77,6 +81,7 @@ static enum power_supply_usb_type rn5t618_usb_types[] = {
static enum power_supply_property rn5t618_usb_props[] = {
/* input current limit is not very accurate */
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_USB_TYPE,
POWER_SUPPLY_PROP_ONLINE,
@@ -85,6 +90,7 @@ static enum power_supply_property rn5t618_usb_props[] = {
static enum power_supply_property rn5t618_adp_props[] = {
/* input current limit is not very accurate */
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_ONLINE,
};
@@ -464,6 +470,16 @@ static int rn5t618_adp_get_property(struct power_supply *psy,
val->intval = FROM_CUR_REG(regval);
break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ if (!info->channel_vadp)
+ return -ENODATA;
+
+ ret = iio_read_channel_processed(info->channel_vadp, &val->intval);
+ if (ret < 0)
+ return ret;
+
+ val->intval *= 1000;
+ break;
default:
return -EINVAL;
}
@@ -589,6 +605,16 @@ static int rn5t618_usb_get_property(struct power_supply *psy,
val->intval = FROM_CUR_REG(regval);
}
break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ if (!info->channel_vusb)
+ return -ENODATA;
+
+ ret = iio_read_channel_processed(info->channel_vusb, &val->intval);
+ if (ret < 0)
+ return ret;
+
+ val->intval *= 1000;
+ break;
default:
return -EINVAL;
}
@@ -711,6 +737,28 @@ static int rn5t618_power_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
+ info->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb");
+ if (IS_ERR(info->channel_vusb)) {
+ ret = PTR_ERR(info->channel_vusb);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+
+ dev_warn(&pdev->dev, "could not request vusb iio channel (%d)",
+ ret);
+ info->channel_vusb = NULL;
+ }
+
+ info->channel_vadp = devm_iio_channel_get(&pdev->dev, "vadp");
+ if (IS_ERR(info->channel_vadp)) {
+ ret = PTR_ERR(info->channel_vadp);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+
+ dev_warn(&pdev->dev, "could not request vadp iio channel (%d)",
+ ret);
+ info->channel_vadp = NULL;
+ }
+
ret = regmap_read(info->rn5t618->regmap, RN5T618_CONTROL, &v);
if (ret)
return ret;
@@ -778,9 +826,17 @@ static int rn5t618_power_probe(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id rn5t618_power_of_match[] = {
+ {.compatible = "ricoh,rc5t619-power", },
+ {.compatible = "ricoh,rn5t618-power", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rn5t618_power_of_match);
+
static struct platform_driver rn5t618_power_driver = {
.driver = {
.name = "rn5t618-power",
+ .of_match_table = of_match_ptr(rn5t618_power_of_match),
},
.probe = rn5t618_power_probe,
};
--
2.30.2
Add devicetree support to be able to easily get the channels
from another device.
Signed-off-by: Andreas Kemnade <[email protected]>
---
drivers/iio/adc/rn5t618-adc.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/rn5t618-adc.c b/drivers/iio/adc/rn5t618-adc.c
index 7010c4276947..feba19f91574 100644
--- a/drivers/iio/adc/rn5t618-adc.c
+++ b/drivers/iio/adc/rn5t618-adc.c
@@ -12,6 +12,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/mfd/rn5t618.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/completion.h>
#include <linux/regmap.h>
@@ -218,6 +219,7 @@ static int rn5t618_adc_probe(struct platform_device *pdev)
init_completion(&adc->conv_completion);
iio_dev->name = dev_name(&pdev->dev);
+ iio_dev->dev.of_node = pdev->dev.of_node;
iio_dev->info = &rn5t618_adc_iio_info;
iio_dev->modes = INDIO_DIRECT_MODE;
iio_dev->channels = rn5t618_adc_iio_channels;
@@ -242,9 +244,19 @@ static int rn5t618_adc_probe(struct platform_device *pdev)
return devm_iio_device_register(adc->dev, iio_dev);
}
+#ifdef CONFIG_OF
+static const struct of_device_id rn5t618_adc_of_match[] = {
+ { .compatible = "ricoh,rc5t619-adc", },
+ { .compatible = "ricoh,rn5t618-adc", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, rn5t618_adc_of_match);
+#endif
+
static struct platform_driver rn5t618_adc_driver = {
.driver = {
- .name = "rn5t618-adc",
+ .name = "rn5t618-adc",
+ .of_match_table = of_match_ptr(rn5t618_adc_of_match),
},
.probe = rn5t618_adc_probe,
};
--
2.30.2
This allows having devicetree nodes for the subdevices.
Signed-off-by: Andreas Kemnade <[email protected]>
---
drivers/mfd/rn5t618.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
index 384acb459427..b916c7471ca3 100644
--- a/drivers/mfd/rn5t618.c
+++ b/drivers/mfd/rn5t618.c
@@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
};
static const struct mfd_cell rc5t619_cells[] = {
- { .name = "rn5t618-adc" },
- { .name = "rn5t618-power" },
+ { .name = "rn5t618-adc",
+ .of_compatible = "ricoh,rc5t619-adc" },
+ { .name = "rn5t618-power",
+ .of_compatible = "ricoh,rc5t619-power" },
{ .name = "rn5t618-regulator" },
{ .name = "rc5t619-rtc" },
{ .name = "rn5t618-wdt" },
--
2.30.2
Hi Andreas,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on power-supply/for-next]
[also build test ERROR on next-20210701]
[cannot apply to lee-mfd/for-mfd-next iio/togreg robh/for-next v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-c021-20210703 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
git checkout 99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
ld: drivers/regulator/bd71815-regulator.o: in function `set_hw_dvs_levels':
bd71815-regulator.c:(.text+0xbf): undefined reference to `rohm_regulator_set_dvs_levels'
ld: drivers/regulator/bd71815-regulator.o: in function `buck12_set_hw_dvs_levels':
bd71815-regulator.c:(.text+0x2be): undefined reference to `rohm_regulator_set_dvs_levels'
ld: drivers/power/supply/rn5t618_power.o: in function `rn5t618_adp_get_property':
>> rn5t618_power.c:(.text+0x34d): undefined reference to `iio_read_channel_processed'
ld: drivers/power/supply/rn5t618_power.o: in function `rn5t618_power_probe':
>> rn5t618_power.c:(.text+0x4a8): undefined reference to `devm_iio_channel_get'
>> ld: rn5t618_power.c:(.text+0x4c2): undefined reference to `devm_iio_channel_get'
ld: drivers/power/supply/rn5t618_power.o: in function `rn5t618_usb_get_property':
rn5t618_power.c:(.text+0x79d): undefined reference to `iio_read_channel_processed'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi Andreas,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on power-supply/for-next]
[also build test ERROR on next-20210701]
[cannot apply to lee-mfd/for-mfd-next iio/togreg robh/for-next v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-m031-20210702 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
git checkout 99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "devm_iio_channel_get" [drivers/power/supply/rn5t618_power.ko] undefined!
>> ERROR: modpost: "iio_read_channel_processed" [drivers/power/supply/rn5t618_power.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Hi,
oh, I forgot to add some depends IIO in the Makefile. I will wait some
days for human reviewers before spinning a v2.
Regards,
Andreas
On Sat, 3 Jul 2021 22:35:27 +0800
kernel test robot <[email protected]> wrote:
> Hi Andreas,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on power-supply/for-next]
> [also build test ERROR on next-20210701]
> [cannot apply to lee-mfd/for-mfd-next iio/togreg robh/for-next v5.13]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
> base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> config: i386-randconfig-m031-20210702 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # https://github.com/0day-ci/linux/commit/99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Andreas-Kemnade/mfd-rn5t618-Extend-ADC-support/20210703-164312
> git checkout 99953e2eb9d653eb8bc74eb482cef7c9fb4e69d7
> # save the attached .config to linux build tree
> make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
> >> ERROR: modpost: "devm_iio_channel_get" [drivers/power/supply/rn5t618_power.ko] undefined!
> >> ERROR: modpost: "iio_read_channel_processed" [drivers/power/supply/rn5t618_power.ko] undefined!
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
On Sat, 3 Jul 2021 10:42:20 +0200
Andreas Kemnade <[email protected]> wrote:
> Add devicetree support so that consumers can reference the channels
> via devicetree, especially the power subdevice can make use of that
> to provide voltage_now properties.
Does the mapping vary from board to board? Often these mappings are
internal to the chip so might as well be provided hard coded in the
relevant drivers rather than via DT. See drivers that have iio_map
structure arrays.
>
> Andreas Kemnade (4):
> dt-bindings: mfd: ricoh,rn5t618: ADC related nodes and properties
> mfd: rn5t618: Add of compatibles for ADC and power
> iio: rn5t618: Add devicetree support
> power: supply: rn5t618: Add voltage_now property
>
> .../bindings/mfd/ricoh,rn5t618.yaml | 53 ++++++++++++++++++
> drivers/iio/adc/rn5t618-adc.c | 14 ++++-
> drivers/mfd/rn5t618.c | 6 +-
> drivers/power/supply/rn5t618_power.c | 56 +++++++++++++++++++
> 4 files changed, 126 insertions(+), 3 deletions(-)
>
On Sat, 3 Jul 2021 10:42:21 +0200
Andreas Kemnade <[email protected]> wrote:
> Add ADC related nodes and properties. This will allow to wire
> up ADC channels to consumers, especially to measure input voltages
> by the power subdevice.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> ---
> .../bindings/mfd/ricoh,rn5t618.yaml | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> index 032a7fb0b4a7..185f87a14a54 100644
> --- a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> @@ -73,6 +73,48 @@ properties:
> description: |
> See Documentation/devicetree/bindings/power/power-controller.txt
>
> + adc:
> + type: object
> +
> + properties:
> + compatible:
> + enum:
> + - ricoh,rn5t618-adc
> + - ricoh,rc5t619-adc
> +
> + "#io-channel-cells":
> + const: 1
> +
> + additionalProperties: false
> +
> + required:
> + - compatible
> + - "#io-channel-cells"
Strictly required? If not used below (where it is optional)
then why do we require the ADC driver to provided the services?
I don't mind you leave it as it is though if you prefer - it doesn't
do any harm!
> +
> + power:
> + type: object
> +
> + properties:
> + compatible:
> + enum:
> + - ricoh,rn5t618-power
> + - ricoh,rc5t619-power
> +
> + io-channels:
> + items:
> + - description: ADP Voltage Channel
> + - description: USB Voltage Channel
> +
> + io-channel-names:
> + items:
> + - const: vadp
> + - const: vusb
> +
> + additionalProperties: false
> +
> + required:
> + - compatible
> +
> regulators:
> type: object
>
> @@ -96,6 +138,17 @@ examples:
> interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
> system-power-controller;
>
> + rn5t618_adc: adc {
> + compatible = "ricoh,rn5t618-adc";
> + #io-channel-cells = <1>;
> + };
> +
> + power {
> + compatible = "ricoh,rn5t618-power";
> + io-channels = <&rn5t618_adc 2>, <&rn5t618_adc 3>;
> + io-channel-names = "vadp", "vusb";
> + };
> +
> regulators {
> DCDC1 {
> regulator-min-microvolt = <1050000>;
On Sat, 3 Jul 2021 10:42:22 +0200
Andreas Kemnade <[email protected]> wrote:
> This allows having devicetree nodes for the subdevices.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> ---
> drivers/mfd/rn5t618.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> index 384acb459427..b916c7471ca3 100644
> --- a/drivers/mfd/rn5t618.c
> +++ b/drivers/mfd/rn5t618.c
> @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> };
>
> static const struct mfd_cell rc5t619_cells[] = {
> - { .name = "rn5t618-adc" },
> - { .name = "rn5t618-power" },
> + { .name = "rn5t618-adc",
> + .of_compatible = "ricoh,rc5t619-adc" },
Odd to have a name of 618 and a compatible of 619. Why?
Definitely deserves a comment if this is necessary for some reason!
> + { .name = "rn5t618-power",
> + .of_compatible = "ricoh,rc5t619-power" },
> { .name = "rn5t618-regulator" },
> { .name = "rc5t619-rtc" },
> { .name = "rn5t618-wdt" },
On Sat, 3 Jul 2021 10:42:23 +0200
Andreas Kemnade <[email protected]> wrote:
> Add devicetree support to be able to easily get the channels
> from another device.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> ---
> drivers/iio/adc/rn5t618-adc.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/rn5t618-adc.c b/drivers/iio/adc/rn5t618-adc.c
> index 7010c4276947..feba19f91574 100644
> --- a/drivers/iio/adc/rn5t618-adc.c
> +++ b/drivers/iio/adc/rn5t618-adc.c
> @@ -12,6 +12,7 @@
> #include <linux/init.h>
> #include <linux/module.h>
> #include <linux/mfd/rn5t618.h>
> +#include <linux/of_device.h>
Why?
mod_devicetable.h probably the one you want.
> #include <linux/platform_device.h>
> #include <linux/completion.h>
> #include <linux/regmap.h>
> @@ -218,6 +219,7 @@ static int rn5t618_adc_probe(struct platform_device *pdev)
> init_completion(&adc->conv_completion);
>
> iio_dev->name = dev_name(&pdev->dev);
> + iio_dev->dev.of_node = pdev->dev.of_node;
That should (now) be set by the IIO core for you via the devm_iio_device_register()
call below if it's not already set.
> iio_dev->info = &rn5t618_adc_iio_info;
> iio_dev->modes = INDIO_DIRECT_MODE;
> iio_dev->channels = rn5t618_adc_iio_channels;
> @@ -242,9 +244,19 @@ static int rn5t618_adc_probe(struct platform_device *pdev)
> return devm_iio_device_register(adc->dev, iio_dev);
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id rn5t618_adc_of_match[] = {
> + { .compatible = "ricoh,rc5t619-adc", },
> + { .compatible = "ricoh,rn5t618-adc", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, rn5t618_adc_of_match);
> +#endif
> +
> static struct platform_driver rn5t618_adc_driver = {
> .driver = {
> - .name = "rn5t618-adc",
> + .name = "rn5t618-adc",
> + .of_match_table = of_match_ptr(rn5t618_adc_of_match),
Given cost of that table is totally trivial (and I'm trying to get rid
of this pattern in IIO as it's noisy and adds little :) drop the
of_match_ptr protection and the ifdefs above.
> },
> .probe = rn5t618_adc_probe,
> };
On Sat, 3 Jul 2021 10:42:24 +0200
Andreas Kemnade <[email protected]> wrote:
> Read voltage_now via IIO and provide the property.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
Hi Andreas,
One comment inline, but looks fine to me in general.
> ---
> drivers/power/supply/rn5t618_power.c | 56 ++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/power/supply/rn5t618_power.c b/drivers/power/supply/rn5t618_power.c
> index 819061918b2a..b062208c8a91 100644
> --- a/drivers/power/supply/rn5t618_power.c
> +++ b/drivers/power/supply/rn5t618_power.c
> @@ -9,10 +9,12 @@
> #include <linux/device.h>
> #include <linux/bitops.h>
> #include <linux/errno.h>
> +#include <linux/iio/consumer.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/mfd/rn5t618.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> #include <linux/regmap.h>
> @@ -64,6 +66,8 @@ struct rn5t618_power_info {
> struct power_supply *battery;
> struct power_supply *usb;
> struct power_supply *adp;
> + struct iio_channel *channel_vusb;
> + struct iio_channel *channel_vadp;
> int irq;
> };
>
> @@ -77,6 +81,7 @@ static enum power_supply_usb_type rn5t618_usb_types[] = {
> static enum power_supply_property rn5t618_usb_props[] = {
> /* input current limit is not very accurate */
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_USB_TYPE,
> POWER_SUPPLY_PROP_ONLINE,
> @@ -85,6 +90,7 @@ static enum power_supply_property rn5t618_usb_props[] = {
> static enum power_supply_property rn5t618_adp_props[] = {
> /* input current limit is not very accurate */
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_ONLINE,
> };
> @@ -464,6 +470,16 @@ static int rn5t618_adp_get_property(struct power_supply *psy,
>
> val->intval = FROM_CUR_REG(regval);
> break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + if (!info->channel_vadp)
> + return -ENODATA;
> +
> + ret = iio_read_channel_processed(info->channel_vadp, &val->intval);
> + if (ret < 0)
> + return ret;
> +
> + val->intval *= 1000;
> + break;
> default:
> return -EINVAL;
> }
> @@ -589,6 +605,16 @@ static int rn5t618_usb_get_property(struct power_supply *psy,
> val->intval = FROM_CUR_REG(regval);
> }
> break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + if (!info->channel_vusb)
> + return -ENODATA;
> +
> + ret = iio_read_channel_processed(info->channel_vusb, &val->intval);
> + if (ret < 0)
> + return ret;
> +
> + val->intval *= 1000;
> + break;
> default:
> return -EINVAL;
> }
> @@ -711,6 +737,28 @@ static int rn5t618_power_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, info);
>
> + info->channel_vusb = devm_iio_channel_get(&pdev->dev, "vusb");
> + if (IS_ERR(info->channel_vusb)) {
> + ret = PTR_ERR(info->channel_vusb);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_warn(&pdev->dev, "could not request vusb iio channel (%d)",
> + ret);
I wonder if you should distinguish between -ENODEV to indicate there wasn't
one specified vs something else when wrong. Might not be worth bothering
as there are not many things that could go wrong (things like small memory
allocations which will never fail..)
> + info->channel_vusb = NULL;
> + }
> +
> + info->channel_vadp = devm_iio_channel_get(&pdev->dev, "vadp");
> + if (IS_ERR(info->channel_vadp)) {
> + ret = PTR_ERR(info->channel_vadp);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_warn(&pdev->dev, "could not request vadp iio channel (%d)",
> + ret);
> + info->channel_vadp = NULL;
> + }
> +
> ret = regmap_read(info->rn5t618->regmap, RN5T618_CONTROL, &v);
> if (ret)
> return ret;
> @@ -778,9 +826,17 @@ static int rn5t618_power_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id rn5t618_power_of_match[] = {
> + {.compatible = "ricoh,rc5t619-power", },
> + {.compatible = "ricoh,rn5t618-power", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, rn5t618_power_of_match);
> +
> static struct platform_driver rn5t618_power_driver = {
> .driver = {
> .name = "rn5t618-power",
> + .of_match_table = of_match_ptr(rn5t618_power_of_match),
> },
> .probe = rn5t618_power_probe,
> };
Hi,
On Sat, 3 Jul 2021 17:04:05 +0100
Jonathan Cameron <[email protected]> wrote:
> On Sat, 3 Jul 2021 10:42:22 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > This allows having devicetree nodes for the subdevices.
> >
> > Signed-off-by: Andreas Kemnade <[email protected]>
> > ---
> > drivers/mfd/rn5t618.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > index 384acb459427..b916c7471ca3 100644
> > --- a/drivers/mfd/rn5t618.c
> > +++ b/drivers/mfd/rn5t618.c
> > @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> > };
> >
> > static const struct mfd_cell rc5t619_cells[] = {
> > - { .name = "rn5t618-adc" },
> > - { .name = "rn5t618-power" },
> > + { .name = "rn5t618-adc",
> > + .of_compatible = "ricoh,rc5t619-adc" },
>
> Odd to have a name of 618 and a compatible of 619. Why?
> Definitely deserves a comment if this is necessary for some reason!
>
Background of this whole thing:
Both RN5T618 and RC5T619 have an ADC. I would expect the driver to work
for both but I cannot prove that. No RN5T618 here to test. Maybe it
requires some quirks, probably not. The only hint I have is that
a) I use register definitions added to the kernel for RN5T618 support
b) public datasheets looks same regarding the ADC.
c) out-of-tree code for the RN5T618 does not give a clear picture, it
shows no differences but is not complete enough to judge.
To avoid introducing untested things, I am only adding it to the
rc5t619_cell list. I would really hope that someone does some rn5t618
tests... Because everything else which is both for the RN5T618 and
RC5T619 uses rn5t618 as a name, I continued that practise.
The subdevice driver also gets the information whether it is a rn5t618
or rc5t619 via rn5t618->variant, so it can handle things.
> > + { .name = "rn5t618-power",
> > + .of_compatible = "ricoh,rc5t619-power" },
Similar situation here.
> > { .name = "rn5t618-regulator" },
> > { .name = "rc5t619-rtc" },
and this one definitively only exists in the rc5t619.
> > { .name = "rn5t618-wdt" },
>
>
Regards,
Andreas
Hi,
On Sat, 3 Jul 2021 16:59:50 +0100
Jonathan Cameron <[email protected]> wrote:
> On Sat, 3 Jul 2021 10:42:20 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > Add devicetree support so that consumers can reference the channels
> > via devicetree, especially the power subdevice can make use of that
> > to provide voltage_now properties.
>
> Does the mapping vary from board to board? Often these mappings are
> internal to the chip so might as well be provided hard coded in the
> relevant drivers rather than via DT. See drivers that have iio_map
> structure arrays.
>
Most things are internal to the chip, but
AIN1/AIN0 are external and could be connected to anything.
Regards,
Andreas
On Sat, 3 Jul 2021 17:02:45 +0100
Jonathan Cameron <[email protected]> wrote:
> On Sat, 3 Jul 2021 10:42:21 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > Add ADC related nodes and properties. This will allow to wire
> > up ADC channels to consumers, especially to measure input voltages
> > by the power subdevice.
> >
> > Signed-off-by: Andreas Kemnade <[email protected]>
> > ---
> > .../bindings/mfd/ricoh,rn5t618.yaml | 53 +++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > index 032a7fb0b4a7..185f87a14a54 100644
> > --- a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > @@ -73,6 +73,48 @@ properties:
> > description: |
> > See Documentation/devicetree/bindings/power/power-controller.txt
> >
> > + adc:
> > + type: object
> > +
> > + properties:
> > + compatible:
> > + enum:
> > + - ricoh,rn5t618-adc
> > + - ricoh,rc5t619-adc
> > +
> > + "#io-channel-cells":
> > + const: 1
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - compatible
> > + - "#io-channel-cells"
>
> Strictly required? If not used below (where it is optional)
> then why do we require the ADC driver to provided the services?
>
> I don't mind you leave it as it is though if you prefer - it doesn't
> do any harm!
>
ok, it is not that strictly required.
Regards,
Andreas
On Sat, 3 Jul 2021 18:39:40 +0200
Andreas Kemnade <[email protected]> wrote:
> Hi,
>
> On Sat, 3 Jul 2021 16:59:50 +0100
> Jonathan Cameron <[email protected]> wrote:
>
> > On Sat, 3 Jul 2021 10:42:20 +0200
> > Andreas Kemnade <[email protected]> wrote:
> >
> > > Add devicetree support so that consumers can reference the channels
> > > via devicetree, especially the power subdevice can make use of that
> > > to provide voltage_now properties.
> >
> > Does the mapping vary from board to board? Often these mappings are
> > internal to the chip so might as well be provided hard coded in the
> > relevant drivers rather than via DT. See drivers that have iio_map
> > structure arrays.
> >
> Most things are internal to the chip, but
> AIN1/AIN0 are external and could be connected to anything.
>
hmm, iio_map stuff looks nice, so before messing with devicetree,
I could solve 90% of the problem by just using iio_map? For my use
cases it is enough to have the internal stuff at the moment. That would
simplify stuff a lot.
So I could go forward with the iio_map stuff now, and if there is a use
case for AIN1/0, the devicetree stuff can be added later?
Regards,
Andreas
On Sat, 3 Jul 2021 18:55:40 +0200
Andreas Kemnade <[email protected]> wrote:
> On Sat, 3 Jul 2021 18:39:40 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > Hi,
> >
> > On Sat, 3 Jul 2021 16:59:50 +0100
> > Jonathan Cameron <[email protected]> wrote:
> >
> > > On Sat, 3 Jul 2021 10:42:20 +0200
> > > Andreas Kemnade <[email protected]> wrote:
> > >
> > > > Add devicetree support so that consumers can reference the channels
> > > > via devicetree, especially the power subdevice can make use of that
> > > > to provide voltage_now properties.
> > >
> > > Does the mapping vary from board to board? Often these mappings are
> > > internal to the chip so might as well be provided hard coded in the
> > > relevant drivers rather than via DT. See drivers that have iio_map
> > > structure arrays.
> > >
> > Most things are internal to the chip, but
> > AIN1/AIN0 are external and could be connected to anything.
> >
> hmm, iio_map stuff looks nice, so before messing with devicetree,
> I could solve 90% of the problem by just using iio_map? For my use
> cases it is enough to have the internal stuff at the moment. That would
> simplify stuff a lot.
>
> So I could go forward with the iio_map stuff now, and if there is a use
> case for AIN1/0, the devicetree stuff can be added later?
I was just thinking the same. I 'think' that it will first try to find
a mapping via device tree and then use the iio_map stuff.
So you can probably get away with a mixture of the two.
Worth testing that works though (hook up iio-hwmon to AIN0 perhaps whilst
also using the iio_map approach).
I might be completely wrong though and am not aware of anyone currently
doing this...
Jonathan
>
> Regards,
> Andreas
On Sat, 3 Jul 2021 18:31:11 +0200
Andreas Kemnade <[email protected]> wrote:
> Hi,
>
> On Sat, 3 Jul 2021 17:04:05 +0100
> Jonathan Cameron <[email protected]> wrote:
>
> > On Sat, 3 Jul 2021 10:42:22 +0200
> > Andreas Kemnade <[email protected]> wrote:
> >
> > > This allows having devicetree nodes for the subdevices.
> > >
> > > Signed-off-by: Andreas Kemnade <[email protected]>
> > > ---
> > > drivers/mfd/rn5t618.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > > index 384acb459427..b916c7471ca3 100644
> > > --- a/drivers/mfd/rn5t618.c
> > > +++ b/drivers/mfd/rn5t618.c
> > > @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> > > };
> > >
> > > static const struct mfd_cell rc5t619_cells[] = {
> > > - { .name = "rn5t618-adc" },
> > > - { .name = "rn5t618-power" },
> > > + { .name = "rn5t618-adc",
> > > + .of_compatible = "ricoh,rc5t619-adc" },
> >
> > Odd to have a name of 618 and a compatible of 619. Why?
> > Definitely deserves a comment if this is necessary for some reason!
> >
> Background of this whole thing:
> Both RN5T618 and RC5T619 have an ADC. I would expect the driver to work
> for both but I cannot prove that. No RN5T618 here to test. Maybe it
> requires some quirks, probably not. The only hint I have is that
> a) I use register definitions added to the kernel for RN5T618 support
> b) public datasheets looks same regarding the ADC.
> c) out-of-tree code for the RN5T618 does not give a clear picture, it
> shows no differences but is not complete enough to judge.
>
> To avoid introducing untested things, I am only adding it to the
> rc5t619_cell list. I would really hope that someone does some rn5t618
> tests... Because everything else which is both for the RN5T618 and
> RC5T619 uses rn5t618 as a name, I continued that practise.
>
> The subdevice driver also gets the information whether it is a rn5t618
> or rc5t619 via rn5t618->variant, so it can handle things.
>
> > > + { .name = "rn5t618-power",
> > > + .of_compatible = "ricoh,rc5t619-power" },
>
> Similar situation here.
>
> > > { .name = "rn5t618-regulator" },
> > > { .name = "rc5t619-rtc" },
> and this one definitively only exists in the rc5t619.
All sounds reasonable to me.
Dt binding wise we'd normally handle this with a double compatible,
with the more part specific one first. That way we can diverge later
if we need to, but don't need to care about it until an identified need
has occurred.
compatible = "ricoh,rc5t619-adc", "ricoh,rc5t618-adc"; (or something like that)
Anyhow, for now perhaps a comment to express briefly what you state above.
>
> > > { .name = "rn5t618-wdt" },
> >
> >
>
> Regards,
> Andreas
On Sat, 03 Jul 2021, Andreas Kemnade wrote:
> This allows having devicetree nodes for the subdevices.
>
> Signed-off-by: Andreas Kemnade <[email protected]>
> ---
> drivers/mfd/rn5t618.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> index 384acb459427..b916c7471ca3 100644
> --- a/drivers/mfd/rn5t618.c
> +++ b/drivers/mfd/rn5t618.c
> @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> };
>
> static const struct mfd_cell rc5t619_cells[] = {
> - { .name = "rn5t618-adc" },
> - { .name = "rn5t618-power" },
> + { .name = "rn5t618-adc",
> + .of_compatible = "ricoh,rc5t619-adc" },
> + { .name = "rn5t618-power",
> + .of_compatible = "ricoh,rc5t619-power" },
If you're converting entries from single to multi-line, you should
place the coding lines one different ones to the brackets.
Also, if ordering is unimportant, could you move the multi-line
entries to the bottom please?
> { .name = "rn5t618-regulator" },
> { .name = "rc5t619-rtc" },
> { .name = "rn5t618-wdt" },
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 03 Jul 2021, Jonathan Cameron wrote:
> On Sat, 3 Jul 2021 10:42:22 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > This allows having devicetree nodes for the subdevices.
> >
> > Signed-off-by: Andreas Kemnade <[email protected]>
> > ---
> > drivers/mfd/rn5t618.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > index 384acb459427..b916c7471ca3 100644
> > --- a/drivers/mfd/rn5t618.c
> > +++ b/drivers/mfd/rn5t618.c
> > @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> > };
> >
> > static const struct mfd_cell rc5t619_cells[] = {
> > - { .name = "rn5t618-adc" },
> > - { .name = "rn5t618-power" },
> > + { .name = "rn5t618-adc",
> > + .of_compatible = "ricoh,rc5t619-adc" },
>
> Odd to have a name of 618 and a compatible of 619. Why?
> Definitely deserves a comment if this is necessary for some reason!
Actually this is the norm. We have lots of drivers named after the
*first* device they supported before expansion.
> > + { .name = "rn5t618-power",
> > + .of_compatible = "ricoh,rc5t619-power" },
> > { .name = "rn5t618-regulator" },
> > { .name = "rc5t619-rtc" },
> > { .name = "rn5t618-wdt" },
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
On Mon, 5 Jul 2021 08:36:08 +0100
Lee Jones <[email protected]> wrote:
> On Sat, 03 Jul 2021, Jonathan Cameron wrote:
>
> > On Sat, 3 Jul 2021 10:42:22 +0200
> > Andreas Kemnade <[email protected]> wrote:
> >
> > > This allows having devicetree nodes for the subdevices.
> > >
> > > Signed-off-by: Andreas Kemnade <[email protected]>
> > > ---
> > > drivers/mfd/rn5t618.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > > index 384acb459427..b916c7471ca3 100644
> > > --- a/drivers/mfd/rn5t618.c
> > > +++ b/drivers/mfd/rn5t618.c
> > > @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> > > };
> > >
> > > static const struct mfd_cell rc5t619_cells[] = {
> > > - { .name = "rn5t618-adc" },
> > > - { .name = "rn5t618-power" },
> > > + { .name = "rn5t618-adc",
> > > + .of_compatible = "ricoh,rc5t619-adc" },
> >
> > Odd to have a name of 618 and a compatible of 619. Why?
> > Definitely deserves a comment if this is necessary for some reason!
>
> Actually this is the norm. We have lots of drivers named after the
> *first* device they supported before expansion.
Ah. I'd missed that this cells array is specific to the 5t619, though if
the driver is the same I'd also expect it to be needed for the 5t618 entry.
>
> > > + { .name = "rn5t618-power",
> > > + .of_compatible = "ricoh,rc5t619-power" },
> > > { .name = "rn5t618-regulator" },
> > > { .name = "rc5t619-rtc" },
> > > { .name = "rn5t618-wdt" },
> >
>
On Mon, 5 Jul 2021 09:31:29 +0100
Jonathan Cameron <[email protected]> wrote:
> On Mon, 5 Jul 2021 08:36:08 +0100
> Lee Jones <[email protected]> wrote:
>
> > On Sat, 03 Jul 2021, Jonathan Cameron wrote:
> >
> > > On Sat, 3 Jul 2021 10:42:22 +0200
> > > Andreas Kemnade <[email protected]> wrote:
> > >
> > > > This allows having devicetree nodes for the subdevices.
> > > >
> > > > Signed-off-by: Andreas Kemnade <[email protected]>
> > > > ---
> > > > drivers/mfd/rn5t618.c | 6 ++++--
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
> > > > index 384acb459427..b916c7471ca3 100644
> > > > --- a/drivers/mfd/rn5t618.c
> > > > +++ b/drivers/mfd/rn5t618.c
> > > > @@ -24,8 +24,10 @@ static const struct mfd_cell rn5t618_cells[] = {
> > > > };
> > > >
> > > > static const struct mfd_cell rc5t619_cells[] = {
> > > > - { .name = "rn5t618-adc" },
> > > > - { .name = "rn5t618-power" },
> > > > + { .name = "rn5t618-adc",
> > > > + .of_compatible = "ricoh,rc5t619-adc" },
> > >
> > > Odd to have a name of 618 and a compatible of 619. Why?
> > > Definitely deserves a comment if this is necessary for some reason!
> >
> > Actually this is the norm. We have lots of drivers named after the
> > *first* device they supported before expansion.
>
> Ah. I'd missed that this cells array is specific to the 5t619, though if
> the driver is the same I'd also expect it to be needed for the 5t618 entry.
>
Well, yes, it is needed for the 5t618 also. But if I would add it, it
would be untested. And that second shorter array is also used for the
rn5t567 which does not have an ADC, So I we need three arrays there.
Regards,
Andreas
On Sun, 4 Jul 2021 17:10:23 +0100
Jonathan Cameron <[email protected]> wrote:
> On Sat, 3 Jul 2021 18:55:40 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > On Sat, 3 Jul 2021 18:39:40 +0200
> > Andreas Kemnade <[email protected]> wrote:
> >
> > > Hi,
> > >
> > > On Sat, 3 Jul 2021 16:59:50 +0100
> > > Jonathan Cameron <[email protected]> wrote:
> > >
> > > > On Sat, 3 Jul 2021 10:42:20 +0200
> > > > Andreas Kemnade <[email protected]> wrote:
> > > >
> > > > > Add devicetree support so that consumers can reference the channels
> > > > > via devicetree, especially the power subdevice can make use of that
> > > > > to provide voltage_now properties.
> > > >
> > > > Does the mapping vary from board to board? Often these mappings are
> > > > internal to the chip so might as well be provided hard coded in the
> > > > relevant drivers rather than via DT. See drivers that have iio_map
> > > > structure arrays.
> > > >
> > > Most things are internal to the chip, but
> > > AIN1/AIN0 are external and could be connected to anything.
> > >
> > hmm, iio_map stuff looks nice, so before messing with devicetree,
> > I could solve 90% of the problem by just using iio_map? For my use
> > cases it is enough to have the internal stuff at the moment. That would
> > simplify stuff a lot.
> >
> > So I could go forward with the iio_map stuff now, and if there is a use
> > case for AIN1/0, the devicetree stuff can be added later?
>
> I was just thinking the same. I 'think' that it will first try to find
> a mapping via device tree and then use the iio_map stuff.
>
> So you can probably get away with a mixture of the two.
> Worth testing that works though (hook up iio-hwmon to AIN0 perhaps whilst
> also using the iio_map approach).
>
> I might be completely wrong though and am not aware of anyone currently
> doing this...
>
I tested that approach, It works, so I will first post a series with
just the iio_map stuff and later the devicetree stuff.
Regards,
Andreas
On Sat, Jul 03, 2021 at 05:02:45PM +0100, Jonathan Cameron wrote:
> On Sat, 3 Jul 2021 10:42:21 +0200
> Andreas Kemnade <[email protected]> wrote:
>
> > Add ADC related nodes and properties. This will allow to wire
> > up ADC channels to consumers, especially to measure input voltages
> > by the power subdevice.
> >
> > Signed-off-by: Andreas Kemnade <[email protected]>
> > ---
> > .../bindings/mfd/ricoh,rn5t618.yaml | 53 +++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > index 032a7fb0b4a7..185f87a14a54 100644
> > --- a/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ricoh,rn5t618.yaml
> > @@ -73,6 +73,48 @@ properties:
> > description: |
> > See Documentation/devicetree/bindings/power/power-controller.txt
> >
> > + adc:
> > + type: object
> > +
> > + properties:
> > + compatible:
> > + enum:
> > + - ricoh,rn5t618-adc
> > + - ricoh,rc5t619-adc
> > +
> > + "#io-channel-cells":
> > + const: 1
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - compatible
> > + - "#io-channel-cells"
>
> Strictly required? If not used below (where it is optional)
> then why do we require the ADC driver to provided the services?
>
> I don't mind you leave it as it is though if you prefer - it doesn't
> do any harm!
The device is either a provider or it is not regardless of whether
there's a consumer, so I prefer this to be required. Also, if a consumer
is in an overlay, then it is easier if we can rely on #io-channel-cells
being present already.
Rob