All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
To: Conor Dooley <conor@kernel.org>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Chen Wang" <unicorn_wang@outlook.com>,
	"Inochi Amaoto" <inochiama@outlook.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Miquèl Raynal" <miquel.raynal@bootlin.com>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo CV18XX SARADC binding
Date: Tue, 20 Aug 2024 18:21:07 +0200	[thread overview]
Message-ID: <89aabfbe-79bf-4da7-be44-b6cbd92b72a9@bootlin.com> (raw)
In-Reply-To: <20240812-unwary-mongrel-9f6758bf624c@spud>

Hello Conor,

On 8/12/24 5:53 PM, Conor Dooley wrote:
> On Mon, Aug 12, 2024 at 05:00:55PM +0200, Thomas Bonnefille wrote:
>> The Sophgo SARADC is a Successive Approximation ADC that can be found in
>> the Sophgo SoC.
>>
>> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> ---
>>   .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 85 ++++++++++++++++++++++
>>   1 file changed, 85 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> new file mode 100644
>> index 000000000000..846590808e5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> @@ -0,0 +1,85 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> 
> Filename matching the compatible please.
> 
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:
>> +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
>> +  Digital Converters
>> +
>> +maintainers:
>> +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> +
>> +description:
>> +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
>> +
>> +properties:
>> +  compatible:
>> +    const: sophgo,cv1800b-saradc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^channel@[0-3]+$":
>> +    $ref: adc.yaml
>> +
>> +    description: |
> 
> This | is not required.
> 
>> +      Represents the channels of the ADC.
>> +
>> +    properties:
>> +      reg:
>> +        description: |
>> +          The channel number. It can have up to 3 channels numbered from 0 to 2.
>> +        items:
>> +          - minimum: 0
>> +            maximum: 2
> 
> Is this sufficient to limit the number of channels to 3? Aren't you relying
> on the unique unit addresses warning in dtc to limit it, rather than
> actually limiting with min/maxItems?
> 
It seems like I can't use min/maxItems on this property. I think that it 
is using size-cells + address-cells to deduce that the number of items 
should be equal to 1.
Looking at the dtschema repository it seems to be the case in reg.yaml 
with address-cells/size-cells = 2/2, 1/1 and 2/1.
If I try to use maxItems here :

     properties:
       reg:
         maxItems: 1
         items:
           - minimum: 0
             maximum: 2

I get this strange error message from `make dt_binding_check`:

DTEX 
Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.example.dts
/home/thomas/linux/Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.yaml: 
patternProperties:^channel@[0-2]+$:properties:reg: {'maxItems': 1, 
'items': [{'minimum': 0, 'maximum': 2}]} should not be valid under 
{'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/thomas/linux/Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.yaml: 
patternProperties:^channel@[0-2]+$:properties:reg: 'anyOf' conditional 
failed, one must be fixed:
	'items' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no 
constraints defined for the values.
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 
'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'items' is not one of ['description', 'deprecated', 'const', 'enum', 
'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of 
minItems/maxItems/items
	hint: cell array properties must define how many entries and what the 
entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#

Isn't it okay to just use minimum and maximum and rely on 
address-cells/size-cells for the number of items allowed ?

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
To: Conor Dooley <conor@kernel.org>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Chen Wang" <unicorn_wang@outlook.com>,
	"Inochi Amaoto" <inochiama@outlook.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Miquèl Raynal" <miquel.raynal@bootlin.com>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo CV18XX SARADC binding
Date: Tue, 20 Aug 2024 18:21:07 +0200	[thread overview]
Message-ID: <89aabfbe-79bf-4da7-be44-b6cbd92b72a9@bootlin.com> (raw)
In-Reply-To: <20240812-unwary-mongrel-9f6758bf624c@spud>

Hello Conor,

On 8/12/24 5:53 PM, Conor Dooley wrote:
> On Mon, Aug 12, 2024 at 05:00:55PM +0200, Thomas Bonnefille wrote:
>> The Sophgo SARADC is a Successive Approximation ADC that can be found in
>> the Sophgo SoC.
>>
>> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> ---
>>   .../bindings/iio/adc/sophgo,cv18xx-saradc.yaml     | 85 ++++++++++++++++++++++
>>   1 file changed, 85 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> new file mode 100644
>> index 000000000000..846590808e5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/sophgo,cv18xx-saradc.yaml
>> @@ -0,0 +1,85 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/sophgo,cv18xx-saradc.yaml#
> 
> Filename matching the compatible please.
> 
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:
>> +  Sophgo CV18XX SoC series 3 channels Successive Approximation Analog to
>> +  Digital Converters
>> +
>> +maintainers:
>> +  - Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>> +
>> +description:
>> +  Datasheet at https://github.com/sophgo/sophgo-doc/releases
>> +
>> +properties:
>> +  compatible:
>> +    const: sophgo,cv1800b-saradc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^channel@[0-3]+$":
>> +    $ref: adc.yaml
>> +
>> +    description: |
> 
> This | is not required.
> 
>> +      Represents the channels of the ADC.
>> +
>> +    properties:
>> +      reg:
>> +        description: |
>> +          The channel number. It can have up to 3 channels numbered from 0 to 2.
>> +        items:
>> +          - minimum: 0
>> +            maximum: 2
> 
> Is this sufficient to limit the number of channels to 3? Aren't you relying
> on the unique unit addresses warning in dtc to limit it, rather than
> actually limiting with min/maxItems?
> 
It seems like I can't use min/maxItems on this property. I think that it 
is using size-cells + address-cells to deduce that the number of items 
should be equal to 1.
Looking at the dtschema repository it seems to be the case in reg.yaml 
with address-cells/size-cells = 2/2, 1/1 and 2/1.
If I try to use maxItems here :

     properties:
       reg:
         maxItems: 1
         items:
           - minimum: 0
             maximum: 2

I get this strange error message from `make dt_binding_check`:

DTEX 
Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.example.dts
/home/thomas/linux/Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.yaml: 
patternProperties:^channel@[0-2]+$:properties:reg: {'maxItems': 1, 
'items': [{'minimum': 0, 'maximum': 2}]} should not be valid under 
{'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/home/thomas/linux/Documentation/devicetree/bindings/iio/adc/sophgo,cv1800b-saradc.yaml: 
patternProperties:^channel@[0-2]+$:properties:reg: 'anyOf' conditional 
failed, one must be fixed:
	'items' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no 
constraints defined for the values.
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 
'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'items' is not one of ['description', 'deprecated', 'const', 'enum', 
'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 is less than the minimum of 2
		hint: Arrays must be described with a combination of 
minItems/maxItems/items
	hint: cell array properties must define how many entries and what the 
entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#

Isn't it okay to just use minimum and maximum and rely on 
address-cells/size-cells for the number of items allowed ?

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2024-08-20 16:21 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-12 15:00 [PATCH v4 0/3] Add SARADC support on Sophgo CV18XX series Thomas Bonnefille
2024-08-12 15:00 ` Thomas Bonnefille
2024-08-12 15:00 ` [PATCH v4 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo CV18XX SARADC binding Thomas Bonnefille
2024-08-12 15:00   ` Thomas Bonnefille
2024-08-12 15:53   ` Conor Dooley
2024-08-12 15:53     ` Conor Dooley
2024-08-20 16:21     ` Thomas Bonnefille [this message]
2024-08-20 16:21       ` Thomas Bonnefille
2024-08-20 16:38       ` Conor Dooley
2024-08-20 16:38         ` Conor Dooley
2024-08-21  7:41         ` Miquel Raynal
2024-08-21  7:41           ` Miquel Raynal
2024-08-21 15:29           ` Conor Dooley
2024-08-21 15:29             ` Conor Dooley
2024-08-22  8:52             ` Miquel Raynal
2024-08-22  8:52               ` Miquel Raynal
2024-08-13  9:50   ` Krzysztof Kozlowski
2024-08-13  9:50     ` Krzysztof Kozlowski
2024-08-12 15:00 ` [PATCH v4 2/3] iio: adc: sophgo-saradc: Add driver for Sophgo CV18XX series SARADC Thomas Bonnefille
2024-08-12 15:00   ` Thomas Bonnefille
2024-08-13  1:39   ` Chen Wang
2024-08-13  1:39     ` Chen Wang
2024-08-17 13:05   ` Jonathan Cameron
2024-08-17 13:05     ` Jonathan Cameron
2024-08-12 15:00 ` [PATCH v4 3/3] riscv: dts: sophgo: Add SARADC description for Sophgo CV18XX Thomas Bonnefille
2024-08-12 15:00   ` Thomas Bonnefille
2024-08-13  1:45   ` Chen Wang
2024-08-13  1:45     ` Chen Wang
2024-08-13  1:50     ` Inochi Amaoto
2024-08-13  1:50       ` Inochi Amaoto
2024-08-13 23:32       ` Chen Wang
2024-08-13 23:32         ` Chen Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=89aabfbe-79bf-4da7-be44-b6cbd92b72a9@bootlin.com \
    --to=thomas.bonnefille@bootlin.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=inochiama@outlook.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh@kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=unicorn_wang@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.