diff mbox series

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

Message ID 20240122090228.28363-1-kimseer.paller@analog.com (mailing list archive)
State Changes Requested
Headers show
Series [v7,1/2] dt-bindings: iio: frequency: add admfm2000 | expand

Commit Message

Kim Seer Paller Jan. 22, 2024, 9:02 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>
---
V6 -> V7: Changed RF path mode property to boolean.
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 | 124 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml


base-commit: 32f764943a21c1af01016bbcd43605220c076262

Comments

Conor Dooley Jan. 22, 2024, 9:44 a.m. UTC | #1 
On Mon, Jan 22, 2024 at 05:02:27PM +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>
> ---
> V6 -> V7: Changed RF path mode property to boolean.

In the process, the description went from attempting to explain what the
property did to the perfunctory "enable mixer mode". Can you please add
an adequate description of the property that covers what happens when
the property is omitted and what "mixer mode" actually does?

> 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 | 124 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 131 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..9e716f59d678
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 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,mixer-mode:
> +        description:
> +          Enable mixer mode.
> +        type: boolean
> +
> +      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

I cannot make sense of this table you have here, the double header row
you have going on is hard to follow. There's also no mention here of
what happens when both GPIOs are 0 or both GPIO are 1. Are these
configurations permitted?

Thanks,
Conor

> +        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
> +      - 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>;
> +        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,mixer-mode;
> +        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 8d1052fa6a69..1f7cd2e848de 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1267,6 +1267,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
> 
> base-commit: 32f764943a21c1af01016bbcd43605220c076262
> -- 
> 2.34.1
>
Kim Seer Paller Jan. 22, 2024, 11:06 a.m. UTC | #2 
> > +      adi,mixer-mode:
> > +        description:
> > +          Enable mixer mode.
> > +        type: boolean
> > +
> > +      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
> 
> I cannot make sense of this table you have here, the double header row you
> have going on is hard to follow. There's also no mention here of what happens
> when both GPIOs are 0 or both GPIO are 1. Are these configurations permitted?

I also feel that it's quite hard to understand without the gridlines. In the preliminary
datasheet, it's a switch control table, and I'm trying to replicate the way it is presented.

On the table, each channel has control pins (CTRL-A and CTRL-B). For a mixer mode
configuration on channel 1, CTRL-A GPIO should be 0, and CTRL-B GPIO should be 1.
It's not permitted when both GPIOs are 0 or both GPIOs are 1. The state for the GPIO
should strictly follow the truth table.

I'm considering making it much easier to understand by creating two separate tables for
each channel, just like the example below. Or is it preferred if it could be discussed in 
a sort of paragraph form?

SW-CH1   CTRL-A   CTRL-B    CH1 Status            CH2 Status
	   1              0              Direct IF mode     Mixer

SW-CH2   CTRL-A   CTRL-B    CH1 Status            CH2 Status
          	    0            1               Direct IF mode     Mixer

Best regards,
Kim
Conor Dooley Jan. 22, 2024, 5:35 p.m. UTC | #3 
On Mon, Jan 22, 2024 at 11:06:08AM +0000, Paller, Kim Seer wrote:
> > > +      adi,mixer-mode:
> > > +        description:
> > > +          Enable mixer mode.
> > > +        type: boolean
> > > +
> > > +      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
> > 
> > I cannot make sense of this table you have here, the double header row you
> > have going on is hard to follow. There's also no mention here of what happens
> > when both GPIOs are 0 or both GPIO are 1. Are these configurations permitted?
> 
> I also feel that it's quite hard to understand without the gridlines. In the preliminary
> datasheet, it's a switch control table, and I'm trying to replicate the way it is presented.
> 
> On the table, each channel has control pins (CTRL-A and CTRL-B). For a mixer mode
> configuration on channel 1, CTRL-A GPIO should be 0, and CTRL-B GPIO should be 1.
> It's not permitted when both GPIOs are 0 or both GPIOs are 1. The state for the GPIO
> should strictly follow the truth table.
> 
> I'm considering making it much easier to understand by creating two separate tables for
> each channel, just like the example below. Or is it preferred if it could be discussed in 
> a sort of paragraph form?
> 
> SW-CH1   CTRL-A   CTRL-B    CH1 Status            CH2 Status
> 	   1              0              Direct IF mode     Mixer
> 
> SW-CH2   CTRL-A   CTRL-B    CH1 Status            CH2 Status
>           	    0            1               Direct IF mode     Mixer

I think it would be sufficient to cut down the original table to
something like:
 CTRL-A   CTRL-B    CH1 Status        CH2 Status
 1        0         Direct IF mode    Mixer mode
 0        1         Mixer mode        Direct IF mode

And state that the configurations where A == B are not permitted.
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..9e716f59d678
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admfm2000.yaml
@@ -0,0 +1,124 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 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,mixer-mode:
+        description:
+          Enable mixer mode.
+        type: boolean
+
+      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
+      - 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>;
+        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,mixer-mode;
+        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 8d1052fa6a69..1f7cd2e848de 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1267,6 +1267,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