Add support for Allwinner GPADC on D1/T113s/R329/T507 SoCs
Subsystems (4)
v2 cover Add support for Allwinner GPADC on D1/T113s/R329/T507 SoCs
This series adds support for general purpose ADC (GPADC) on new Allwinner's SoCs, such as D1, T113s, T507 and R329. The implemented driver provides basic functionality for getting ADC channels data.
Change History:
v2:
- Added lastch flag to avoid addition work for already selected channel
- Added reset assertion on module remove
- Added dynamic channel allocation and dropped iio_chan_spec arrays
- Changed IIO_CHAN_INFO_SCALE type to FRACTIONAL_LOG2
- Dropped separate compatible strings and configs for T113s and R329
- Fixed includes
- Fixed Kconfig description
- Removed duplicate probe error messages
- Used FIELD_PREP for bit setup
v1:
- Initial version
Maxim Kiselev (3):
iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
riscv: dts: allwinner: d1: Add GPADC node
.../iio/adc/allwinner,sun20i-d1-gpadc.yaml | 79 +++++ .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 10 + drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/sun20i-gpadc-iio.c | 296 ++++++++++++++++++ 5 files changed, 396 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml create mode 100644 drivers/iio/adc/sun20i-gpadc-iio.c
Signature · 1 line
2.39.2
PATCH v2 1/3 iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
From: Maxim Kiselev <[email protected]>
The General Purpose ADC (GPADC) can convert the external signal into a certain proportion of digital value, to realize the measurement of analog signal, which can be applied to power detection and key detection.
Theoretically, this ADC can support up to 16 channels. All SoCs below contain this GPADC IP. The only difference between them is the number of available channels:
T113 - 1 channel
D1 - 2 channels
R329 - 4 channels
T507 - 4 channels
--- drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/sun20i-gpadc-iio.c | 296 +++++++++++++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 drivers/iio/adc/sun20i-gpadc-iio.c
drivers/iio/adc/Kconfig +10 -0
@@ -1123,6 +1123,16 @@ config SUN4I_GPADC To compile this driver as a module, choose M here: the module will be called sun4i-gpadc-iio. +config SUN20I_GPADC + tristate "Support for the Allwinner SoCs GPADC" + depends on ARCH_SUNXI || COMPILE_TEST + help + Say yes here to build support for Allwinner (D1, T113, T507 and R329) + SoCs GPADC. This ADC provides up to 16 channels. + + To compile this driver as a module, choose M here: the module will be + called sun20i-gpadc-iio. + config TI_ADC081C tristate "Texas Instruments ADC081C/ADC101C/ADC121C family" depends on I2C
drivers/iio/adc/Makefile +1 -0
@@ -95,6 +95,7 @@ obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o obj-$(CONFIG_SPEAR_ADC) += spear_adc.o obj-$(CONFIG_SUN4I_GPADC) += sun4i-gpadc-iio.o +obj-$(CONFIG_SUN20I_GPADC) += sun20i-gpadc-iio.o obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o obj-$(CONFIG_STM32_ADC) += stm32-adc.o obj-$(CONFIG_STM32_DFSDM_CORE) += stm32-dfsdm-core.o
drivers/iio/adc/sun20i-gpadc-iio.c new file +296 -0
296 lines not on this page · raw message
Signature · 1 line
2.39.2
Re: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
Quoted 14 lines · Maksim Kiselev
From: Maxim Kiselev <[email protected]>
The General Purpose ADC (GPADC) can convert the external signal into a certain proportion of digital value, to realize the measurement of analog signal, which can be applied to power detection and key detection.
Theoretically, this ADC can support up to 16 channels. All SoCs below contain this GPADC IP. The only difference between them is the number of available channels:
T113 - 1 channel
D1 - 2 channels
R329 - 4 channels
T507 - 4 channels
...
drivers/iio/adc/sun20i-gpadc-iio.c line 55
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +4
-0+struct sun20i_gpadc_iio { +struct regmap *regmap; +struct completion completion; +struct mutex lock;
The locks should be explained (what are they for? what do they protect?).
Quoted 2 lines
(quoted diff) +2
-0+ int lastch; +};
...
drivers/iio/adc/sun20i-gpadc-iio.c line 63
Quoted 5 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +5
-0+static const struct regmap_config sun20i_gpadc_regmap_config = { + .reg_bits =32 , + .val_bits =32 , + .reg_stride =4 , + .fast_io = true,
I forgot if I asked about regmap lock do you need it?
Quoted 1 line
+};
...
drivers/iio/adc/sun20i-gpadc-iio.c line 92
Quoted 2 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +2
-0+if (!wait_for_completion_timeout (&info->completion, +msecs_to_jiffies (100 ))) {
Dunno if it's better to have this parameter to be defined with self-explanatory name.
Quoted 3 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +3
-0+ ret = -ETIMEDOUT ; + goto err; + }
...
Quoted 1 line
+err:
err_unlock:
Quoted 3 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +3
-0+mutex_unlock (&info->lock); + +return ret;
...
drivers/iio/adc/sun20i-gpadc-iio.c line 116
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +4
-0+switch (mask) { +case IIO_CHAN_INFO_RAW : + ret =sun20i_gpadc_adc_read (info, chan, val); +return ret;
return sun...;
Quoted 8 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +8
-0+case IIO_CHAN_INFO_SCALE : +/* value in mv = 1800mV / 4096 raw */ + *val =1800 ; + *val2 =12 ; +return IIO_VAL_FRACTIONAL_LOG2 ; +default : +return -EINVAL ; + }
...
drivers/iio/adc/sun20i-gpadc-iio.c line 167
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +4
-0+if (num_channels >SUN20I_GPADC_MAX_CHANNELS ) { +dev_err (dev,"num of channel children out of range" ); +return -EINVAL ; + }
Is it really critical error?
...
drivers/iio/adc/sun20i-gpadc-iio.c line 170
Quoted 2 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +2
-0+ channels =devm_kcalloc (dev, num_channels, +sizeof (*channels),GFP_KERNEL );
At least one more parameter can be located on the previous line.
Quoted 2 lines
(quoted diff) +2
-0+ if (!channels) + return -ENOMEM;
...
Quoted 1 line
+err_child_out:
err_put_child:
The goto labels should be self-explanatory of what to expect when goto.
Quoted 3 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +3
-0+fwnode_handle_put (node); + +return ret;
...
drivers/iio/adc/sun20i-gpadc-iio.c line 263
Quoted 3 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +3
-0+ ret =devm_request_irq (dev, irq, sun20i_gpadc_irq_handler, +0 ,dev_name (dev), info); +if (ret <0 )
Here...
Quoted 2 lines
(quoted diff) +2
-0+ return dev_err_probe(dev, ret, + "failed requesting irq %d\n", irq);
...
drivers/iio/adc/sun20i-gpadc-iio.c line 272
Quoted 2 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +2
-0+ ret =devm_iio_device_register (dev, indio_dev); +if (ret <0 )
...and here, do the positive returned values even possible?
Quoted 2 lines
(quoted diff) +2
-0+ return dev_err_probe(dev, ret, + "could not register the device\n");
...
drivers/iio/adc/sun20i-gpadc-iio.c line 280
Quoted 1 line
drivers/iio/adc/sun20i-gpadc-iio.c new file +1
-0+ { .compatible = "allwinner,sun20i-d1-gpadc", },
Inner comma is not needed.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerRe: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
Quoted 6 lines
drivers/iio/adc/sun20i-gpadc-iio.c line 92
Quoted 3 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +2
-0+if (!wait_for_completion_timeout (&info->completion, +msecs_to_jiffies (100 ))) {Dunno if it's better to have this parameter to be defined with self-explanatory name.
Probably a response to my earlier comment. I'd agree with a good name but GPADC_TIMEOUT which was the earlier naming is less use than a value and it's not obvious what that name should be.
A nice datasheet reference would be good to have though.
Quoted 6 lines
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +3
-0+ ret = -ETIMEDOUT ; + goto err; + }
Quoted 6 lines
drivers/iio/adc/sun20i-gpadc-iio.c line 167
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +4
-0+if (num_channels >SUN20I_GPADC_MAX_CHANNELS ) { +dev_err (dev,"num of channel children out of range" ); +return -EINVAL ; + }Is it really critical error?
Overflow of registers - so yes. I wondered this on v1 and went digging :) Now, there are no such devices known, so meh on whether check is useful.
Quoted 2 lines
...
Re: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
In reply to Andy Shevchenkoпт, 2 июн. 2023 г. в 17:49, Andy Shevchenko <[email protected]>:
...
Quoted 1 line
The locks should be explained (what are they for? what do they protect?).
I added the explanation comment in the next version.
...
Quoted 7 lines
drivers/iio/adc/sun20i-gpadc-iio.c line 63
Quoted 5 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +5
-0+static const struct regmap_config sun20i_gpadc_regmap_config = { + .reg_bits =32 , + .val_bits =32 , + .reg_stride =4 , + .fast_io = true,I forgot if I asked about regmap lock do you need it?
I think we could drop the regmap altogether. As Andy suggested in previous series.
...
Quoted 6 lines
drivers/iio/adc/sun20i-gpadc-iio.c line 167
Quoted 4 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +4
-0+if (num_channels >SUN20I_GPADC_MAX_CHANNELS ) { +dev_err (dev,"num of channel children out of range" ); +return -EINVAL ; + }Is it really critical error?
Yes, as Jonathan already noted, this may lead to out of range error.
вс, 4 июн. 2023 г. в 13:46, Jonathan Cameron <[email protected]>:
...
Quoted 4 lines
We try to make this name identify the chip in question.
If the driver name is sufficient for these platforms then fair enough. It should certainly be enough to distinguish this from other ADCs on the platform.
I believe the driver name should be enough. All listed SoCs have the same GPADC register
layout and differ only in the number of channels.
Jonathan Cameron
maintainerRe: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
In reply to Maksim KiselevQuoted 10 lines · On Fri, 2 Jun 2023 01:30:39 +0300 Maksim Kiselev <[email protected]> wrote:
From: Maxim Kiselev <[email protected]>
The General Purpose ADC (GPADC) can convert the external signal into a certain proportion of digital value, to realize the measurement of analog signal, which can be applied to power detection and key detection.
Theoretically, this ADC can support up to 16 channels. All SoCs below contain this GPADC IP. The only difference between them is the number of available channels:
Quoted 6 lines
T113 - 1 channel
D1 - 2 channels
R329 - 4 channels
T507 - 4 channelsSigned-off-by Maxim Kiselev <[email protected]>
Hi Maxim,
A few more minor comments from me. Looking good in general though.
Quoted 36 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +30
-0@@ -0,0 +1,296 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* GPADC driver for sunxi platforms (D1, T113-S3 and R329) +* Copyright (c) 2023 Maksim Kiselev <[email protected]> +*/ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/reset.h> + +#include <linux/iio/iio.h> + +#define SUN20I_GPADC_DRIVER_NAME "sun20i-gpadc" + +/* Register map definition */ +#define SUN20I_GPADC_SR 0x00 +#define SUN20I_GPADC_CTRL 0x04 +#define SUN20I_GPADC_CS_EN 0x08 +#define SUN20I_GPADC_FIFO_INTC 0x0c +#define SUN20I_GPADC_FIFO_INTS 0x10 +#define SUN20I_GPADC_FIFO_DATA 0X14 +#define SUN20I_GPADC_CB_DATA 0X18
drivers/iio/adc/sun20i-gpadc-iio.c line 40
Quoted 10 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +10
-0+#define SUN20I_GPADC_DATAL_INTC 0x20 +#define SUN20I_GPADC_DATAH_INTC 0x24 +#define SUN20I_GPADC_DATA_INTC 0x28 +#define SUN20I_GPADC_DATAL_INTS 0x30 +#define SUN20I_GPADC_DATAH_INTS 0x34 +#define SUN20I_GPADC_DATA_INTS 0x38 +#define SUN20I_GPADC_CH_CMP_DATA (x) (0x40 + (x) * 4) +#define SUN20I_GPADC_CH_DATA (x) (0x80 + (x) * 4) + +/* ADC bit shift */
Not sure what this comment means? I'd just drop it.
Quoted 9 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +6
-0+#define SUN20I_GPADC_CTRL_ADC_AUTOCALI_EN_MASK BIT(23) +#define SUN20I_GPADC_CTRL_WORK_MODE_MASK GENMASK(19, 18) +#define SUN20I_GPADC_CTRL_ADC_EN_MASK BIT(16) +#define SUN20I_GPADC_CS_EN_ADC_CH (x) BIT(x) +#define SUN20I_GPADC_DATA_INTC_CH_DATA_IRQ_EN (x) BIT(x) +static int sun20i_gpadc_read_raw(struct iio_dev *indio_dev,
drivers/iio/adc/sun20i-gpadc-iio.c line 116
Quoted 10 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +10
-0+ struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct sun20i_gpadc_iio *info = iio_priv(indio_dev); +int ret; + +switch (mask) { +case IIO_CHAN_INFO_RAW : + ret =sun20i_gpadc_adc_read (info, chan, val); +return ret;
return sun20i_gpadc_adc_read()
and drop the local variable ret as no longer used.
Quoted 29 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +28
-0+case IIO_CHAN_INFO_SCALE : +/* value in mv = 1800mV / 4096 raw */ + *val =1800 ; + *val2 =12 ; +return IIO_VAL_FRACTIONAL_LOG2 ; +default : +return -EINVAL ; + } +} +static int sun20i_gpadc_alloc_channels (struct iio_dev *indio_dev, +struct device *dev) +{ +unsigned int channel; +int num_channels, i, ret; +struct iio_chan_spec *channels; +struct fwnode_handle *node; + + num_channels =device_get_child_node_count (dev); +if (num_channels ==0 ) { +dev_err (dev,"no channel children" ); +return -ENODEV ; + } + +if (num_channels >SUN20I_GPADC_MAX_CHANNELS ) { +dev_err (dev,"num of channel children out of range" ); +return -EINVAL ; + } +
drivers/iio/adc/sun20i-gpadc-iio.c line 178
Quoted 10 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +10
-0+ channels =devm_kcalloc (dev, num_channels, +sizeof (*channels),GFP_KERNEL ); +if (!channels) +return -ENOMEM ; + + i =0 ; +device_for_each_child_node (dev, node) { + ret =fwnode_property_read_u32 (node,"reg" , &channel); +if (ret) + goto err_child_out;
You are fairly verbose on error messages elsewhere - which is somewhat of a personal choice, but if you are going to do that I'd expect to see on here as well.
Quoted 28 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +28
-0+ + channels[i].type =IIO_VOLTAGE ; + channels[i].indexed =1 ; + channels[i].channel = channel; + channels[i].info_mask_separate =BIT (IIO_CHAN_INFO_RAW ); + channels[i].info_mask_shared_by_type =BIT (IIO_CHAN_INFO_SCALE ); + + i++; + } + + indio_dev->channels = channels; + indio_dev->num_channels = num_channels; + +return 0 ; + +err_child_out: +fwnode_handle_put (node); + +return ret; +} + +static int sun20i_gpadc_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct iio_dev *indio_dev; +struct sun20i_gpadc_iio *info; +struct reset_control *rst; +void __iomem *base;
drivers/iio/adc/sun20i-gpadc-iio.c line 216
Quoted 10 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +10
-0+struct clk *clk; +int irq; +int ret; + + indio_dev =devm_iio_device_alloc (dev,sizeof (*info)); +if (!indio_dev) +return -ENOMEM ; + + info =iio_priv (indio_dev); + info->lastch =-1 ;
That naming briefly had me confused as my brain tried to figure it out as a typo :). Perhaps last_ch is clearer? Or just go with last_channel and be really clear.
drivers/iio/adc/sun20i-gpadc-iio.c line 226
Quoted 10 lines
drivers/iio/adc/sun20i-gpadc-iio.c new file +10
-0+ +mutex_init (&info->lock); +init_completion (&info->completion); + + ret =sun20i_gpadc_alloc_channels (indio_dev, dev); +if (ret) +return ret; + + indio_dev->info = &sun20i_gpadc_iio_info; + indio_dev->name =SUN20I_GPADC_DRIVER_NAME ;
We try to make this name identify the chip in question.
If the driver name is sufficient for these platforms then fair enough. It should certainly be enough to distinguish this from other ADCs on the platform.
PATCH v2 2/3 dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
In reply to Maksim KiselevFrom: Maxim Kiselev <[email protected]>
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
--- .../iio/adc/allwinner,sun20i-d1-gpadc.yaml | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +79 -0
@@ -0,0 +1,79 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/allwinner,sun20i-d1-gpadc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner D1 General Purpose ADC + +properties: + "#io-channel-cells": + const: 1 + + clocks: + maxItems: 1 + + compatible: + enum: + - allwinner,sun20i-d1-gpadc + + interrupts: + maxItems: 1 + + reg: + maxItems: 1 + + resets: + maxItems: 1 + +required: + - "#io-channel-cells" + - clocks + - compatible + - interrupts + - reg + - resets + +patternProperties: + "^channel@([0-15])$": + $ref: adc.yaml + type: object + description: | + Represents the internal channels of the ADC. + + properties: + reg: + description: | + The channel number. + Up to 16 channels, numbered from 0 to 15. + items: + minimum: 0 + maximum: 15 + + required: + - reg + + additionalProperties: false + +additionalProperties: false + +examples: + - | + gpadc: adc@2009000 { + compatible = "allwinner,sun20i-d1-gpadc"; + reg = <0x2009000 0x1000>; + clocks = <&ccu 80>; + resets = <&ccu 32>; + interrupts = <0 57 4>; + #io-channel-cells = <1>; + + channel@0 { + reg = <0>; + }; + + channel@1 { + reg = <1>; + }; + }; + +...
Signature · 1 line
2.39.2
Rob Herring
maintainerRe: [PATCH v2 2/3] dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
Quoted 6 lines · Maksim Kiselev
From: Maxim Kiselev <[email protected]>
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
Quoted 6 lines
Signed-off-by Maxim Kiselev <[email protected]>--- .../iio/adc/allwinner,sun20i-d1-gpadc.yaml | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml: 'maintainers' is a required property
hint: Metaschema for devicetree binding documentation
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:33.15-25: Warning (reg_format): /example-0/adc@2009000/channel@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:37.15-25: Warning (reg_format): /example-0/adc@2009000/channel@1:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:32.23-34.15: Warning (avoid_default_addr_size): /example-0/adc@2009000/channel@0: Relying on default #address-cells value Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:32.23-34.15: Warning (avoid_default_addr_size): /example-0/adc@2009000/channel@0: Relying on default #size-cells value Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:36.23-38.15: Warning (avoid_default_addr_size): /example-0/adc@2009000/channel@1: Relying on default #address-cells value Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dts:36.23-38.15: Warning (avoid_default_addr_size): /example-0/adc@2009000/channel@1: Relying on default #size-cells value Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
In reply to Maksim KiselevQuoted 5 lines · Maksim Kiselev
From: Maxim Kiselev <[email protected]>
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
Except that it wasn't tested...
Quoted 13 lines
Signed-off-by Maxim Kiselev <[email protected]>--- .../iio/adc/allwinner,sun20i-d1-gpadc.yaml | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yamlDocumentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +0
-0
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 1
Quoted 1 line
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +1
-0@@ -0,0 +1,79 @@ +# SPDX-License-Identifier: GPL-2.0
dual license
Please run scripts/checkpatch.pl and fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear.
Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +7
-0+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/allwinner,sun20i-d1-gpadc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner D1 General Purpose ADC +
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 18
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +10
-0+properties: + "#io-channel-cells": + const: 1 + + clocks: + maxItems: 1 + + compatible: + enum: + - allwinner,sun20i-d1-gpadc
compatible is first property
Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +7
-0+ + interrupts: + maxItems: 1 + + reg: + maxItems: 1 +
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 35
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +10
-0+ resets: + maxItems: 1 + +required: + - "#io-channel-cells" + - clocks + - compatible + - interrupts + - reg + - resets
required: block goes after all properties.
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 41
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +6
-0+ +patternProperties: + "^channel@([0-15])$": + $ref: adc.yaml + type: object + description: |
Do not need '|' unless you need to preserve formatting.
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 46
Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +5
-0+ Represents the internal channels of the ADC. + + properties: + reg: + description: |
Do not need '|' unless you need to preserve formatting.
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 48
Quoted 2 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +2
-0+ The channel number. + Up to 16 channels, numbered from 0 to 15.
Don't repeat constraints in free form text.
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 56
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +8
-0+ items: + minimum: 0 + maximum: 15 + + required: + - reg + + additionalProperties: false
Hm? So you do not allow anything from adc.yaml related? Are you sure this is your intention?
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 67
Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +11
-0+ +additionalProperties: false + +examples: + - | + gpadc: adc@2009000 { + compatible = "allwinner,sun20i-d1-gpadc"; + reg = <0x2009000 0x1000>; + clocks = <&ccu 80>; + resets = <&ccu 32>; + interrupts = <0 57 4>;
Use proper defines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 71
Quoted 4 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +4
-0+ #io-channel-cells = <1>; + + channel@0 { + reg = <0>;
Broken indentation.
Use 4 spaces for example indentation.
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml line 79
Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml new file +8
-0+ }; + + channel@1 { + reg = <1>; + }; + }; + +...
Best regards,
Krzysztof
Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
пт, 2 июн. 2023 г. в 11:38, Krzysztof Kozlowski
<[email protected]>:
Hi Krzysztof,
Quoted 9 lines · Maksim Kiselev
Quoted 5 lines
From: Maxim Kiselev <[email protected]>
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
Except that it wasn't tested...
Yes, you are right. I tested it only on the T113s board. And I will be glad if someone tests it on another SoC.
...
Quoted 3 lines
Please run
scripts/checkpatch.pland fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear.
I got a warning about required maintainer property. Should I do anything with this?
If yes, then who should be a maintainer?
...
Quoted 2 lines
Hm? So you do not allow anything from
adc.yamlrelated? Are you sure this is your intention?
I'm not sure about it. I looked at other ADC bindings and didn't find another driver with 'additionalProperties: true'
Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
Quoted 5 lines · Maxim Kiselev
пт, 2 июн. 2023 г. в 11:38, Krzysztof Kozlowski
<[email protected]>:Quoted 3 lines · Maksim Kiselev
Quoted 2 lines
From: Maxim Kiselev <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
Except that it wasn't tested...
Yes, you are right. I tested it only on the T113s board. And I will be glad if someone tests it on another SoC.
I would imagine Krzysztof meant testing the binding w/ dt_binding_check etc, rather than testing the ADC itself.
Quoted 6 lines
Quoted 3 lines
Please run
scripts/checkpatch.pland fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear.I got a warning about required maintainer property. Should I do anything with this?
Yes, you should!
Quoted 1 line
If yes, then who should be a maintainer?
You, preferably.
Quoted 5 lines
Quoted 2 lines
Hm? So you do not allow anything from
adc.yamlrelated? Are you sure this is your intention?I'm not sure about it. I looked at other ADC bindings and didn't find another driver with 'additionalProperties: true'
Try unevaluatedProperties: false instead.
Cheers,
Conor.
Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC
In reply to Maxim KiselevQuoted 7 lines · Maxim Kiselev
пт, 2 июн. 2023 г. в 11:38, Krzysztof Kozlowski
<[email protected]>:
Hi Krzysztof,Quoted 4 lines · Maksim Kiselev
Quoted 2 lines
From: Maxim Kiselev <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Allwinner's D1/T113s/R329/T507 SoCs have a new general purpose ADC. This ADC is the same for all of this SoCs. The only difference is the number of available channels.
Except that it wasn't tested...
Yes, you are right. I tested it only on the T113s board. And I will be glad if someone tests it on another SoC.
Please show me the commands how you tested bindings on T113s board. I really would like to see them...
Quoted 9 lines
...
Quoted 3 lines
Please run
scripts/checkpatch.pland fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear.I got a warning about required maintainer property. Should I do anything with this?
You should do something because sending broken code is not correct.
Quoted 1 line
If yes, then who should be a maintainer?
Someone responsible for this hardware.
Quoted 8 lines
...
Quoted 2 lines
Hm? So you do not allow anything from
adc.yamlrelated? Are you sure this is your intention?I'm not sure about it. I looked at other ADC bindings and didn't find another driver with 'additionalProperties: true'
I didn't write about additionalProperties:true, but about missing ref to adc.yaml.
Best regards,
Krzysztof
PATCH v2 3/3 riscv: dts: allwinner: d1: Add GPADC node
In reply to Maksim KiselevFrom: Maxim Kiselev <[email protected]>
This patch adds declaration of the general purpose ADC for D1 and T113s SoCs.
--- arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi +10 -0
@@ -138,6 +138,16 @@ ccu: clock-controller@2001000 { #reset-cells = <1>; }; + gpadc: adc@2009000 { + compatible = "allwinner,sun20i-d1-gpadc"; + reg = <0x2009000 0x1000>; + clocks = <&ccu CLK_BUS_GPADC>; + resets = <&ccu RST_BUS_GPADC>; + interrupts = <SOC_PERIPHERAL_IRQ(57) IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + #io-channel-cells = <1>; + }; + dmic: dmic@2031000 { compatible = "allwinner,sun20i-d1-dmic", "allwinner,sun50i-h6-dmic";
Signature · 1 line
2.39.2