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

[<Marius.Cristea@microchip.com>]

Hi David,

   Thanks for the suggestion.

//Marius

On Tue, 2025-06-10 at 11:35 -0500, David Lechner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> 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
>