RE: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000

From: Paller, Kim Seer
Date: Fri Jan 19 2024 - 04:17:17 EST


> > > > > +patternProperties:
> > > > > +  "^channel@[0-1]$":
> > > > > +    type: object
> > > > > +    description: Represents a channel of the device.
> > > > > +
> > > > > +    additionalProperties: false
> > > > > +
> > > > > +    properties:
> > > > > +      reg:
> > > > > +        description:
> > > > > +          The channel number.
> > > > > +        minimum: 0
> > > > > +        maximum: 1
> > > > > +
> > > > > +      adi,mode:
> > > > > +        description:
> > > > > +          RF path selected for the channel.
> > > > > +            0 - Direct IF mode
> > > > > +            1 - Mixer mode
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        enum: [0, 1]
> > > >
> > > > How come this is an enum, rather than a boolean property such as
> > > > "adi,mixer-mode"?
> > >
> > > I used an enum, perhaps because it was easier to implement. However,
> > > this could be changed if a boolean property might be more suitable in this
> case.
> > > Is that the preferred option?
> > >
> >
> > Hmmm, How is the enum easier than a boolean property :)? I guess the
> > device has a default mode. So, if it is Direct IF mode you have
> > 'adi,mixer-mode' to enable that mode and that's it. So the code is pretty
> much just:
> >
> > if (device_property_read_bool()) {
>
> device_property_present() is preferred I think.
>
> > /* enable mixer mode */
> > }
> >
> > Also remember that from a bindings point of view being easier to
> > implement or not in the driver does not matter much. Take for an
> > example, properties with well know units like 'microamp'. It would be
> > much easier to just have an enum with the device register values but
> > that's not how we should do it since it wouldn't be intuitive at all for people
> reading devicetrees.

Hi Conor, Nuno,

Thanks for the input, I'll take note of that.

Best,
Kim