Add device tree bindings for the ADMV8818 Filter.
Signed-off-by: Antoniu Miclaus <[email protected]>
---
.../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
1 file changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
new file mode 100644
index 000000000000..d581e236dbdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/filter/adi,admv8818.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
+
+maintainers:
+ - Antoniu Miclaus <[email protected]>
+
+description: |
+ Fully monolithic microwave integrated circuit (MMIC) that
+ features a digitally selectable frequency of operation.
+ The device features four independently controlled high-pass
+ filters (HPFs) and four independently controlled low-pass filters
+ (LPFs) that span the 2 GHz to 18 GHz frequency range.
+
+ https://www.analog.com/en/products/admv8818.html
+
+properties:
+ compatible:
+ enum:
+ - adi,admv8818
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 10000000
+
+ clocks:
+ description:
+ Definition of the external clock.
+ minItems: 1
+
+ clock-names:
+ items:
+ - const: "rf_in"
+
+ clock-output-names:
+ maxItems: 1
+
+ adi,bw-hz:
+ description:
+ Allows the user to increase the Bandpass Filter (BPF) bandwidth
+ in Hz. Normally when invoked by the clk notifier, the driver
+ sets the HPF cutoff close below the frequency and the LPF cutoff
+ close above the frequency, and thus creating a BPF.
+ $ref: /schemas/types.yaml#/definitions/uint64
+
+ '#clock-cells':
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ admv8818@0 {
+ compatible = "adi,admv8818";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+ clocks = <&admv8818_rfin>;
+ clock-names = "rf_in";
+ adi,bw-hz = /bits/ 64 <600000000>;
+ };
+ };
+...
+
--
2.33.1
On Tue, 09 Nov 2021 14:31:26 +0200, Antoniu Miclaus wrote:
> Add device tree bindings for the ADMV8818 Filter.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: properties:adi,bw-hz: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml: ignoring, error in schema: properties: adi,bw-hz
warning: no schema found in file: ./Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
Documentation/devicetree/bindings/iio/filter/adi,admv8818.example.dt.yaml:0:0: /example-0/spi/admv8818@0: failed to match any schema with compatible: ['adi,admv8818']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1552959
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
--
Antoniu Micl?u?
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Wednesday, November 10, 2021 3:07 AM
> To: Miclaus, Antoniu <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
>
> [External]
>
> On Tue, 09 Nov 2021 14:31:26 +0200, Antoniu Miclaus wrote:
> > Add device tree bindings for the ADMV8818 Filter.
> >
> > Signed-off-by: Antoniu Miclaus <[email protected]>
> > ---
> > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
> > 1 file changed, 78 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-
> review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml:
> properties:adi,bw-hz: '$ref' should not be valid under {'const': '$ref'}
> hint: Standard unit suffix properties don't need a type $ref
> from schema $id:
> https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!tVywX0fQFR2vYZq-
> YTIJiJ0AXF2WapK4hXuLoFCYYlg19vxsZrLtIe7gWW7NfqCnlwuk$
> /builds/robherring/linux-dt-
> review/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml:
> ignoring, error in schema: properties: adi,bw-hz
> warning: no schema found in file:
> ./Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> Documentation/devicetree/bindings/iio/filter/adi,admv8818.example.dt.ya
> ml:0:0: /example-0/spi/admv8818@0: failed to match any schema with
> compatible: ['adi,admv8818']
>
> doc reference errors (make refcheckdocs):
>
> See
> https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1552959_
> _;!!A3Ni8CS0y2Y!tVywX0fQFR2vYZq-
> YTIJiJ0AXF2WapK4hXuLoFCYYlg19vxsZrLtIe7gWW7NfqkOdOfT$
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
Thanks!
I will resubmit this patch in v2 after the driver is reviewed.
Regards,
Antoniu
On Tue, 9 Nov 2021 14:31:26 +0200
Antoniu Miclaus <[email protected]> wrote:
> Add device tree bindings for the ADMV8818 Filter.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> new file mode 100644
> index 000000000000..d581e236dbdc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/filter/adi,admv8818.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
> +
> +maintainers:
> + - Antoniu Miclaus <[email protected]>
> +
> +description: |
> + Fully monolithic microwave integrated circuit (MMIC) that
> + features a digitally selectable frequency of operation.
> + The device features four independently controlled high-pass
> + filters (HPFs) and four independently controlled low-pass filters
> + (LPFs) that span the 2 GHz to 18 GHz frequency range.
> +
> + https://www.analog.com/en/products/admv8818.html
> +
> +properties:
> + compatible:
> + enum:
> + - adi,admv8818
> +
> + reg:
> + maxItems: 1
> +
> + spi-max-frequency:
> + maximum: 10000000
> +
> + clocks:
> + description:
> + Definition of the external clock.
> + minItems: 1
> +
> + clock-names:
> + items:
> + - const: "rf_in"
Is this what we'd normally think of as a clock signal? I'd not expect
a nice squarewave on that pin for example so this seems an odd way to
define it.
> +
> + clock-output-names:
> + maxItems: 1
> +
> + adi,bw-hz:
> + description:
> + Allows the user to increase the Bandpass Filter (BPF) bandwidth
> + in Hz. Normally when invoked by the clk notifier, the driver
> + sets the HPF cutoff close below the frequency and the LPF cutoff
> + close above the frequency, and thus creating a BPF.
I don't understand this item at all. Why do we need a control to
basically change how the other filter parameters are expressed?
> + $ref: /schemas/types.yaml#/definitions/uint64
> +
> + '#clock-cells':
> + const: 0
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + admv8818@0 {
> + compatible = "adi,admv8818";
> + reg = <0>;
> + spi-max-frequency = <10000000>;
> + clocks = <&admv8818_rfin>;
> + clock-names = "rf_in";
> + adi,bw-hz = /bits/ 64 <600000000>;
> + };
> + };
> +...
> +
Hello Jonathan,
--
Antoniu Micl?u?
> -----Original Message-----
> From: Jonathan Cameron <[email protected]>
> Sent: Friday, November 12, 2021 7:46 PM
> To: Miclaus, Antoniu <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
>
> [External]
>
> On Tue, 9 Nov 2021 14:31:26 +0200
> Antoniu Miclaus <[email protected]> wrote:
>
> > Add device tree bindings for the ADMV8818 Filter.
> >
> > Signed-off-by: Antoniu Miclaus <[email protected]>
> > ---
> > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
> > 1 file changed, 78 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> >
> > diff --git
> a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > new file mode 100644
> > index 000000000000..d581e236dbdc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> https://urldefense.com/v3/__http://devicetree.org/schemas/iio/filter/adi,a
> dmv8818.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3yE_r
> Ny91yIkDRTXFe54x-cHq_BtsyzDOedLohB5D$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3
> yE_rNy91yIkDRTXFe54x-cHq_BtsyzDOeYdHtx0a$
> > +
> > +title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
> > +
> > +maintainers:
> > + - Antoniu Miclaus <[email protected]>
> > +
> > +description: |
> > + Fully monolithic microwave integrated circuit (MMIC) that
> > + features a digitally selectable frequency of operation.
> > + The device features four independently controlled high-pass
> > + filters (HPFs) and four independently controlled low-pass filters
> > + (LPFs) that span the 2 GHz to 18 GHz frequency range.
> > +
> > + https://www.analog.com/en/products/admv8818.html
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,admv8818
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + spi-max-frequency:
> > + maximum: 10000000
> > +
> > + clocks:
> > + description:
> > + Definition of the external clock.
> > + minItems: 1
> > +
> > + clock-names:
> > + items:
> > + - const: "rf_in"
>
> Is this what we'd normally think of as a clock signal? I'd not expect
> a nice squarewave on that pin for example so this seems an odd way to
> define it.
>
The only actual use of this part, until now, was to filter the output of the following part:
https://www.analog.com/en/products/adf5610.html
This is the reason of using the clock framework in the driver. Moreover, the clock input is
optional inside the driver.
> > +
> > + clock-output-names:
> > + maxItems: 1
> > +
> > + adi,bw-hz:
> > + description:
> > + Allows the user to increase the Bandpass Filter (BPF) bandwidth
> > + in Hz. Normally when invoked by the clk notifier, the driver
> > + sets the HPF cutoff close below the frequency and the LPF cutoff
> > + close above the frequency, and thus creating a BPF.
>
> I don't understand this item at all. Why do we need a control to
> basically change how the other filter parameters are expressed?
>
Indeed, this property was requested by the users of the application in which this part was involved.
Same goes for the filter modes and the bandwidth in the ABI documentation.
If you think these attributes/properties are way too custom, I can drop them.
Let me know your thoughts.
> > + $ref: /schemas/types.yaml#/definitions/uint64
> > +
> > + '#clock-cells':
> > + const: 0
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + admv8818@0 {
> > + compatible = "adi,admv8818";
> > + reg = <0>;
> > + spi-max-frequency = <10000000>;
> > + clocks = <&admv8818_rfin>;
> > + clock-names = "rf_in";
> > + adi,bw-hz = /bits/ 64 <600000000>;
> > + };
> > + };
> > +...
> > +
On Tue, 16 Nov 2021 14:43:14 +0000
"Miclaus, Antoniu" <[email protected]> wrote:
> Hello Jonathan,
>
> --
> Antoniu Miclăuş
>
> > -----Original Message-----
> > From: Jonathan Cameron <[email protected]>
> > Sent: Friday, November 12, 2021 7:46 PM
> > To: Miclaus, Antoniu <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
> >
> > [External]
> >
> > On Tue, 9 Nov 2021 14:31:26 +0200
> > Antoniu Miclaus <[email protected]> wrote:
> >
> > > Add device tree bindings for the ADMV8818 Filter.
> > >
> > > Signed-off-by: Antoniu Miclaus <[email protected]>
> > > ---
> > > .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++
> > > 1 file changed, 78 insertions(+)
> > > create mode 100644
> > Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > >
> > > diff --git
> > a/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > > new file mode 100644
> > > index 000000000000..d581e236dbdc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml
> > > @@ -0,0 +1,78 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/filter/adi,a
> > dmv8818.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3yE_r
> > Ny91yIkDRTXFe54x-cHq_BtsyzDOedLohB5D$
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!qkKokhmcgS0YEIy3uC6OfOOF7Bq3
> > yE_rNy91yIkDRTXFe54x-cHq_BtsyzDOeYdHtx0a$
> > > +
> > > +title: ADMV8818 Digitally Tunable, High-Pass and Low-Pass Filter
> > > +
> > > +maintainers:
> > > + - Antoniu Miclaus <[email protected]>
> > > +
> > > +description: |
> > > + Fully monolithic microwave integrated circuit (MMIC) that
> > > + features a digitally selectable frequency of operation.
> > > + The device features four independently controlled high-pass
> > > + filters (HPFs) and four independently controlled low-pass filters
> > > + (LPFs) that span the 2 GHz to 18 GHz frequency range.
> > > +
> > > + https://www.analog.com/en/products/admv8818.html
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - adi,admv8818
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + spi-max-frequency:
> > > + maximum: 10000000
> > > +
> > > + clocks:
> > > + description:
> > > + Definition of the external clock.
> > > + minItems: 1
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: "rf_in"
> >
> > Is this what we'd normally think of as a clock signal? I'd not expect
> > a nice squarewave on that pin for example so this seems an odd way to
> > define it.
> >
> The only actual use of this part, until now, was to filter the output of the following part:
> https://www.analog.com/en/products/adf5610.html
> This is the reason of using the clock framework in the driver. Moreover, the clock input is
> optional inside the driver.
OK, so in theory that part is generating a sinusoid. I guess the clk framework works for
handling such devices, even if it's not typically what people expect from a clk.
> > > +
> > > + clock-output-names:
> > > + maxItems: 1
> > > +
> > > + adi,bw-hz:
> > > + description:
> > > + Allows the user to increase the Bandpass Filter (BPF) bandwidth
> > > + in Hz. Normally when invoked by the clk notifier, the driver
> > > + sets the HPF cutoff close below the frequency and the LPF cutoff
> > > + close above the frequency, and thus creating a BPF.
> >
> > I don't understand this item at all. Why do we need a control to
> > basically change how the other filter parameters are expressed?
> >
>
> Indeed, this property was requested by the users of the application in which this part was involved.
> Same goes for the filter modes and the bandwidth in the ABI documentation.
>
> If you think these attributes/properties are way too custom, I can drop them.
It's interesting. I guess the point here is people want a nice autonomous system to
keep the filter set appropriately for cleaning up a generated sine wave.
It could be argued that is a hardware related thing so makes sense in DT.
We are sort of 'emulating' a bandpass filter in the driver if we use this, but
I guess if that's the main use case then this is perhaps a reasonable decision.
>
> Let me know your thoughts.
> > > + $ref: /schemas/types.yaml#/definitions/uint64
> > > +
> > > + '#clock-cells':
> > > + const: 0
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - clocks
> > > + - clock-names
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + spi {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + admv8818@0 {
> > > + compatible = "adi,admv8818";
> > > + reg = <0>;
> > > + spi-max-frequency = <10000000>;
> > > + clocks = <&admv8818_rfin>;
> > > + clock-names = "rf_in";
> > > + adi,bw-hz = /bits/ 64 <600000000>;
> > > + };
> > > + };
> > > +...
> > > +
>