| Message ID | 20211109123127.96399-4-antoniu.miclaus@analog.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for ADMV8818 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
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 <antoniu.miclaus@analog.com> > --- > .../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 <robh@kernel.org> > Sent: Wednesday, November 10, 2021 3:07 AM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: jic23@kernel.org; linux-iio@vger.kernel.org; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; robh+dt@kernel.org > 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 <antoniu.miclaus@analog.com> > > --- > > .../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 <antoniu.miclaus@analog.com> wrote: > Add device tree bindings for the ADMV8818 Filter. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > --- > .../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 <antoniu.miclaus@analog.com> > + > +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 <jic23@kernel.org> > Sent: Friday, November 12, 2021 7:46 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc > > [External] > > On Tue, 9 Nov 2021 14:31:26 +0200 > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > > Add device tree bindings for the ADMV8818 Filter. > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > --- > > .../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 <antoniu.miclaus@analog.com> > > + > > +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" <Antoniu.Miclaus@analog.com> wrote: > Hello Jonathan, > > -- > Antoniu Miclăuş > > > -----Original Message----- > > From: Jonathan Cameron <jic23@kernel.org> > > Sent: Friday, November 12, 2021 7:46 PM > > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com> > > Cc: robh+dt@kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc > > > > [External] > > > > On Tue, 9 Nov 2021 14:31:26 +0200 > > Antoniu Miclaus <antoniu.miclaus@analog.com> wrote: > > > > > Add device tree bindings for the ADMV8818 Filter. > > > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > > --- > > > .../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 <antoniu.miclaus@analog.com> > > > + > > > +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>; > > > + }; > > > + }; > > > +... > > > + >
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 <antoniu.miclaus@analog.com> + +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>; + }; + }; +... +
Add device tree bindings for the ADMV8818 Filter. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- .../bindings/iio/filter/adi,admv8818.yaml | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/filter/adi,admv8818.yaml