[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PH0PR03MB7141990D32CE5544397384F9F9702@PH0PR03MB7141.namprd03.prod.outlook.com>
Date: Fri, 19 Jan 2024 09:16:44 +0000
From: "Paller, Kim Seer" <KimSeer.Paller@...log.com>
To: Conor Dooley <conor.dooley@...rochip.com>,
Nuno Sá
<noname.nuno@...il.com>
CC: Conor Dooley <conor@...nel.org>,
"linux-iio@...r.kernel.org"
<linux-iio@...r.kernel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
"Hennerich, Michael"
<Michael.Hennerich@...log.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof
Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley
<conor+dt@...nel.org>, Crt Mori <cmo@...exis.com>,
Linus Walleij
<linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>
Subject: RE: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000
> > > > > +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
Powered by blists - more mailing lists