Re: [PATCH v3 1/2] dt-bindings: iio: adc: adding support for PAC194X

[David Lechner <dlechner@baylibre.com>]

On 6/10/25 11:04 AM, Marius.Cristea@microchip.com wrote:
> On Tue, 2025-06-10 at 10:22 -0500, David Lechner wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> On 6/10/25 9:46 AM, Marius.Cristea@microchip.com wrote:
>>> Hi David,
>>>
>>>    Thank you for the feedback. Please see my comments below...
>>>

...

>>>>
>>>> Using Jonathan's suggestion from v2 to just have a single
>>>> property
>>>> with 3 different
>>>> ranges to chose from seems simpler that this. It would only
>>>> require
>>>> one property
>>>> and would be self-documenting. The description could be shortened
>>>> to
>>>> just a couple
>>>> of lines.
>>>
>>> I was thinking to add the range for this property, but it looks
>>> (for me
>>> at least) more complicated from the checking point of view. The
>>> driver
>>> is supporting two family of devices that has, each, 3 different
>>> voltage
>>> range as an input.
>>>
>>
>> Usually, having a consistent binding for the same thing among similar
>> devices is more important than how easy it is to implement in the
>> driver.
>>
>> Since this seems to be a common pattern, we could probably justify an
>> iio_property_match_ranges() helper function that would simplify the
>> implementation in drivers that would need to use such a property.
>> Then
>> in each driver it would just be a matter of making a static const
>> array
>> lookup table of ranges for each device and calling the helper
>> function.
> 
> Sorry for not explaining very well. I have implemented the range into
> the driver and I was working well, but I had issues defining the range
> into the device binding and the checker was failing. That was the
> reason that I've dropped the range from the binding. Also I had some
> issues enforcing a certain "available" ranges for a particular part
> into the binding.

What did you try?

The usual way is to define all possibilities and then limit it by compatible.
I think something like this should work:

patternProperties:
  "^channel@[1-4]$":
    properties:
      microchip,input-range-microvolt:
       items:
          - enum: [-32000000, -16000000, -9000000, -4500000, 0]
          - enum: [4500000, 9000000, 16000000, 32000000]


allOf:
  - if:
      properties:
        compatible:
          pattern: "^microchip,pac194"
     then:
       patternProperties:
        "^channel@[1-4]$":
          properties:
            microchip,input-range-microvolt:
              oneOf:
                - items:
                    - const: 0
                    - const: 9000000
                - items:
                    - const: -9000000
                    - const: 9000000
                - items:
                    - const: -4500000
                    - const: 4500000
              default:
                items:
                  - const: 0
                  - const: 9000000
  - if:
      properties:
        compatible:
          pattern: "^microchip,pac195"
     then:
       patternProperties:
        "^channel@[1-4]$":
          properties:
            microchip,input-range-microvolt:
              oneOf:
                - items:
                    - const: 0
                    - const: 32000000
                - items:
                    - const: -32000000
                    - const: 32000000
                - items:
                    - const: -16000000
                    - const: 16000000
              default:
                items:
                  - const: 0
                  - const: 32000000