diff mbox series

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

Message ID 20240118085856.70758-2-kimseer.paller@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Changes to admfm2000 driver | expand

Commit Message

Kim Seer Paller Jan. 18, 2024, 8:58 a.m. UTC
Dual microwave down converter module with input RF and LO frequency
ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
for each down conversion path.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
          Changed pin coords with friendly names. Removed Reviewed-by tag.
V4 -> V5: Added Reviewed-by tag.
V3 -> V4: Updated the description of the properties with multiple entries and
          defined the order.
V2 -> V3: Adjusted indentation to resolve wrong indentation warning. 
          Changed node name to converter. Updated the descriptions to clarify
          the properties.
V1 -> V2: Removed '|' after description. Specified the pins connected to
          the GPIOs. Added additionalProperties: false. Changed node name to gpio.
          Aligned < syntax with the previous syntax in the examples.

 .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 136 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml

Comments

Conor Dooley Jan. 18, 2024, 4:09 p.m. UTC | #1
Hey,

On Thu, Jan 18, 2024 at 04:58:55PM +0800, Kim Seer Paller wrote:
> Dual microwave down converter module with input RF and LO frequency
> ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
> 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
> for each down conversion path.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
> V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
>           Changed pin coords with friendly names. Removed Reviewed-by tag.
> V4 -> V5: Added Reviewed-by tag.
> V3 -> V4: Updated the description of the properties with multiple entries and
>           defined the order.
> V2 -> V3: Adjusted indentation to resolve wrong indentation warning. 
>           Changed node name to converter. Updated the descriptions to clarify
>           the properties.
> V1 -> V2: Removed '|' after description. Specified the pins connected to
>           the GPIOs. Added additionalProperties: false. Changed node name to gpio.
>           Aligned < syntax with the previous syntax in the examples.
> 
>  .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 136 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> new file mode 100644
> index 000000000000..6f2c91c38666
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADMFM2000 Dual Microwave Down Converter
> +
> +maintainers:
> +  - Kim Seer Paller <kimseer.paller@analog.com>
> +
> +description:
> +  Dual microwave down converter module with input RF and LO frequency ranges
> +  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
> +  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down
> +  conversion path.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,admfm2000
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +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"?

Cheers,
Conor.
Kim Seer Paller Jan. 19, 2024, 12:30 a.m. UTC | #2
> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Friday, January 19, 2024 12:10 AM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> Cc: linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; Lars-Peter
> Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Rob Herring <robh+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Crt Mori <cmo@melexis.com>; Linus Walleij
> <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> Subject: Re: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000
> 
> [External]
> 
> Hey,
> 
> On Thu, Jan 18, 2024 at 04:58:55PM +0800, Kim Seer Paller wrote:
> > Dual microwave down converter module with input RF and LO frequency
> > ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
> > 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
> > for each down conversion path.
> >
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---
> > V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
> >           Changed pin coords with friendly names. Removed Reviewed-by tag.
> > V4 -> V5: Added Reviewed-by tag.
> > V3 -> V4: Updated the description of the properties with multiple entries and
> >           defined the order.
> > V2 -> V3: Adjusted indentation to resolve wrong indentation warning.
> >           Changed node name to converter. Updated the descriptions to clarify
> >           the properties.
> > V1 -> V2: Removed '|' after description. Specified the pins connected to
> >           the GPIOs. Added additionalProperties: false. Changed node name to
> gpio.
> >           Aligned < syntax with the previous syntax in the examples.
> >
> >  .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 136 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> >
> > diff --git
> a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > new file mode 100644
> > index 000000000000..6f2c91c38666
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > @@ -0,0 +1,129 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2023 Analog Devices Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ADMFM2000 Dual Microwave Down Converter
> > +
> > +maintainers:
> > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > +
> > +description:
> > +  Dual microwave down converter module with input RF and LO frequency
> ranges
> > +  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
> > +  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down
> > +  conversion path.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,admfm2000
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +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?

Best regards,
Kim Seer Paller
Nuno Sá Jan. 19, 2024, 8:20 a.m. UTC | #3
Hi Kim,

On Fri, 2024-01-19 at 00:30 +0000, Paller, Kim Seer wrote:
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Friday, January 19, 2024 12:10 AM
> > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > Cc: linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; Lars-Peter
> > Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Rob Herring <robh+dt@kernel.org>;
> > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > <conor+dt@kernel.org>; Crt Mori <cmo@melexis.com>; Linus Walleij
> > <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> > Subject: Re: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000
> > 
> > [External]
> > 
> > Hey,
> > 
> > On Thu, Jan 18, 2024 at 04:58:55PM +0800, Kim Seer Paller wrote:
> > > Dual microwave down converter module with input RF and LO frequency
> > > ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
> > > 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
> > > for each down conversion path.
> > > 
> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > ---
> > > V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
> > >           Changed pin coords with friendly names. Removed Reviewed-by tag.
> > > V4 -> V5: Added Reviewed-by tag.
> > > V3 -> V4: Updated the description of the properties with multiple entries and
> > >           defined the order.
> > > V2 -> V3: Adjusted indentation to resolve wrong indentation warning.
> > >           Changed node name to converter. Updated the descriptions to clarify
> > >           the properties.
> > > V1 -> V2: Removed '|' after description. Specified the pins connected to
> > >           the GPIOs. Added additionalProperties: false. Changed node name to
> > gpio.
> > >           Aligned < syntax with the previous syntax in the examples.
> > > 
> > >  .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
> > >  MAINTAINERS                                   |   7 +
> > >  2 files changed, 136 insertions(+)
> > >  create mode 100644
> > Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > 
> > > diff --git
> > a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > new file mode 100644
> > > index 000000000000..6f2c91c38666
> > > --- /dev/null
> > > +++
> > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > @@ -0,0 +1,129 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright 2023 Analog Devices Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ADMFM2000 Dual Microwave Down Converter
> > > +
> > > +maintainers:
> > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > +
> > > +description:
> > > +  Dual microwave down converter module with input RF and LO frequency
> > ranges
> > > +  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
> > > +  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down
> > > +  conversion path.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,admfm2000
> > > +
> > > +  '#address-cells':
> > > +    const: 1
> > > +
> > > +  '#size-cells':
> > > +    const: 0
> > > +
> > > +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()) {
	/* 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.

- Nuno Sá
Conor Dooley Jan. 19, 2024, 8:31 a.m. UTC | #4
On Fri, Jan 19, 2024 at 09:20:01AM +0100, Nuno Sá wrote:
> Hi Kim,
> 
> On Fri, 2024-01-19 at 00:30 +0000, Paller, Kim Seer wrote:
> > > -----Original Message-----
> > > From: Conor Dooley <conor@kernel.org>
> > > Sent: Friday, January 19, 2024 12:10 AM
> > > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > > Cc: linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; Lars-Peter
> > > Clausen <lars@metafoo.de>; Hennerich, Michael
> > > <Michael.Hennerich@analog.com>; Rob Herring <robh+dt@kernel.org>;
> > > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > > <conor+dt@kernel.org>; Crt Mori <cmo@melexis.com>; Linus Walleij
> > > <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> > > Subject: Re: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000
> > > 
> > > [External]
> > > 
> > > Hey,
> > > 
> > > On Thu, Jan 18, 2024 at 04:58:55PM +0800, Kim Seer Paller wrote:
> > > > Dual microwave down converter module with input RF and LO frequency
> > > > ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
> > > > 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
> > > > for each down conversion path.
> > > > 
> > > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > > ---
> > > > V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
> > > >           Changed pin coords with friendly names. Removed Reviewed-by tag.
> > > > V4 -> V5: Added Reviewed-by tag.
> > > > V3 -> V4: Updated the description of the properties with multiple entries and
> > > >           defined the order.
> > > > V2 -> V3: Adjusted indentation to resolve wrong indentation warning.
> > > >           Changed node name to converter. Updated the descriptions to clarify
> > > >           the properties.
> > > > V1 -> V2: Removed '|' after description. Specified the pins connected to
> > > >           the GPIOs. Added additionalProperties: false. Changed node name to
> > > gpio.
> > > >           Aligned < syntax with the previous syntax in the examples.
> > > > 
> > > >  .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
> > > >  MAINTAINERS                                   |   7 +
> > > >  2 files changed, 136 insertions(+)
> > > >  create mode 100644
> > > Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > 
> > > > diff --git
> > > a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > new file mode 100644
> > > > index 000000000000..6f2c91c38666
> > > > --- /dev/null
> > > > +++
> > > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > @@ -0,0 +1,129 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright 2023 Analog Devices Inc.
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: ADMFM2000 Dual Microwave Down Converter
> > > > +
> > > > +maintainers:
> > > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > > +
> > > > +description:
> > > > +  Dual microwave down converter module with input RF and LO frequency
> > > ranges
> > > > +  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
> > > > +  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down
> > > > +  conversion path.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - adi,admfm2000
> > > > +
> > > > +  '#address-cells':
> > > > +    const: 1
> > > > +
> > > > +  '#size-cells':
> > > > +    const: 0
> > > > +
> > > > +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.
> 
> - Nuno Sá
>
Kim Seer Paller Jan. 19, 2024, 9:16 a.m. UTC | #5
> > > > > +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
Nuno Sá Jan. 19, 2024, 9:21 a.m. UTC | #6
On Fri, 2024-01-19 at 08:31 +0000, Conor Dooley wrote:
> On Fri, Jan 19, 2024 at 09:20:01AM +0100, Nuno Sá wrote:
> > Hi Kim,
> > 
> > On Fri, 2024-01-19 at 00:30 +0000, Paller, Kim Seer wrote:
> > > > -----Original Message-----
> > > > From: Conor Dooley <conor@kernel.org>
> > > > Sent: Friday, January 19, 2024 12:10 AM
> > > > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > > > Cc: linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; Jonathan Cameron <jic23@kernel.org>; Lars-Peter
> > > > Clausen <lars@metafoo.de>; Hennerich, Michael
> > > > <Michael.Hennerich@analog.com>; Rob Herring <robh+dt@kernel.org>;
> > > > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > > > <conor+dt@kernel.org>; Crt Mori <cmo@melexis.com>; Linus Walleij
> > > > <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> > > > Subject: Re: [PATCH v6 1/2] dt-bindings: iio: frequency: add admfm2000
> > > > 
> > > > [External]
> > > > 
> > > > Hey,
> > > > 
> > > > On Thu, Jan 18, 2024 at 04:58:55PM +0800, Kim Seer Paller wrote:
> > > > > Dual microwave down converter module with input RF and LO frequency
> > > > > ranges from 0.5 to 32 GHz and an output IF frequency range from 0.1 to
> > > > > 8 GHz. It consists of a LNA, mixer, IF filter, DSA, and IF amplifier
> > > > > for each down conversion path.
> > > > > 
> > > > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > > > ---
> > > > > V5 -> V6: Moved array of switch and attenuation GPIOs to the channel node.
> > > > >           Changed pin coords with friendly names. Removed Reviewed-by tag.
> > > > > V4 -> V5: Added Reviewed-by tag.
> > > > > V3 -> V4: Updated the description of the properties with multiple entries
> > > > > and
> > > > >           defined the order.
> > > > > V2 -> V3: Adjusted indentation to resolve wrong indentation warning.
> > > > >           Changed node name to converter. Updated the descriptions to
> > > > > clarify
> > > > >           the properties.
> > > > > V1 -> V2: Removed '|' after description. Specified the pins connected to
> > > > >           the GPIOs. Added additionalProperties: false. Changed node name
> > > > > to
> > > > gpio.
> > > > >           Aligned < syntax with the previous syntax in the examples.
> > > > > 
> > > > >  .../bindings/iio/frequency/adi,admfm2000.yaml | 129 ++++++++++++++++++
> > > > >  MAINTAINERS                                   |   7 +
> > > > >  2 files changed, 136 insertions(+)
> > > > >  create mode 100644
> > > > Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > > 
> > > > > diff --git
> > > > a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..6f2c91c38666
> > > > > --- /dev/null
> > > > > +++
> > > > b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> > > > > @@ -0,0 +1,129 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +# Copyright 2023 Analog Devices Inc.
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: ADMFM2000 Dual Microwave Down Converter
> > > > > +
> > > > > +maintainers:
> > > > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > > > +
> > > > > +description:
> > > > > +  Dual microwave down converter module with input RF and LO frequency
> > > > ranges
> > > > > +  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
> > > > > +  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each
> > > > > down
> > > > > +  conversion path.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - adi,admfm2000
> > > > > +
> > > > > +  '#address-cells':
> > > > > +    const: 1
> > > > > +
> > > > > +  '#size-cells':
> > > > > +    const: 0
> > > > > +
> > > > > +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.
> 

Hmm, don't want to start an argument but I'm not sure either :). I would argue that
device_property_read_bool() has more users (according to git grep - and if I did not
mess the grep) and it pretty much wraps device_property_present(). So, if there was
no value in it's "meaning" we would/should stop using it and eventually drop it...
Anyways, not really a big deal.

- Nuno Sá
> >
Conor Dooley Jan. 19, 2024, 4:54 p.m. UTC | #7
> > > 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.
> > 
> 
> Hmm, don't want to start an argument but I'm not sure either :). I would argue that
> device_property_read_bool() has more users (according to git grep - and if I did not
> mess the grep) and it pretty much wraps device_property_present(). So, if there was
> no value in it's "meaning" we would/should stop using it and eventually drop it...
> Anyways, not really a big deal.

If there's an actually boolean property that can have a true/false
value, but testing for presence alone device_property_present() is
the more accurate function to use.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
new file mode 100644
index 000000000000..6f2c91c38666
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
@@ -0,0 +1,129 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2023 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/frequency/adi,admfm2000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADMFM2000 Dual Microwave Down Converter
+
+maintainers:
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description:
+  Dual microwave down converter module with input RF and LO frequency ranges
+  from 0.5 to 32 GHz and an output IF frequency range from 0.1 to 8 GHz.
+  It consists of a LNA, mixer, IF filter, DSA, and IF amplifier for each down
+  conversion path.
+
+properties:
+  compatible:
+    enum:
+      - adi,admfm2000
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+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]
+
+      switch-gpios:
+        description: |
+          GPIOs to select the RF path for the channel.
+          SW-CH1   CTRL-A   CTRL-B
+          SW-CH2   CTRL-A   CTRL-B    CH1 Status        CH2 Status
+                   1        0         Direct IF mode    Mixer mode
+                   0        1         Mixer mode        Direct IF mode
+
+        items:
+          - description: SW-CH-CTRL-A GPIO
+          - description: SW-CH-CTRL-B GPIO
+
+      attenuation-gpios:
+        description: |
+          Choice of attenuation:
+          DSA-V4  DSA-V3  DSA-V2  DSA-V1  DSA-V0
+          1       1       1       1       1        0 dB
+          1       1       1       1       0        -1 dB
+          1       1       1       0       1        -2 dB
+          1       1       0       1       1        -4 dB
+          1       0       1       1       1        -8 dB
+          0       1       1       1       1        -16 dB
+          0       0       0       0       0        -31 dB
+
+        items:
+          - description: DSA-V0 GPIO
+          - description: DSA-V1 GPIO
+          - description: DSA-V2 GPIO
+          - description: DSA-V3 GPIO
+          - description: DSA-V4 GPIO
+
+    required:
+      - reg
+      - adi,mode
+      - switch-gpios
+      - attenuation-gpios
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    converter {
+      compatible = "adi,admfm2000";
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      channel@0 {
+        reg = <0>;
+        adi,mode = <1>;
+        switch-gpios = <&gpio 1 GPIO_ACTIVE_LOW>,
+                       <&gpio 2 GPIO_ACTIVE_HIGH>;
+
+        attenuation-gpios = <&gpio 17 GPIO_ACTIVE_LOW>,
+                            <&gpio 22 GPIO_ACTIVE_LOW>,
+                            <&gpio 23 GPIO_ACTIVE_LOW>,
+                            <&gpio 24 GPIO_ACTIVE_LOW>,
+                            <&gpio 25 GPIO_ACTIVE_LOW>;
+      };
+
+      channel@1 {
+        reg = <1>;
+        adi,mode = <1>;
+        switch-gpios = <&gpio 3 GPIO_ACTIVE_LOW>,
+                       <&gpio 4 GPIO_ACTIVE_HIGH>;
+
+        attenuation-gpios = <&gpio 0 GPIO_ACTIVE_LOW>,
+                            <&gpio 5 GPIO_ACTIVE_LOW>,
+                            <&gpio 6 GPIO_ACTIVE_LOW>,
+                            <&gpio 16 GPIO_ACTIVE_LOW>,
+                            <&gpio 26 GPIO_ACTIVE_LOW>;
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 2d94c72c3742..3a86f9d6cb98 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1260,6 +1260,13 @@  W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/hwmon/adi,adm1177.yaml
 F:	drivers/hwmon/adm1177.c
 
+ANALOG DEVICES INC ADMFM2000 DRIVER
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
+
 ANALOG DEVICES INC ADMV1013 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
 L:	linux-iio@vger.kernel.org