| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20211121122451.569e41f5@jic23-huawei>
Date: Sun, 21 Nov 2021 12:24:51 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>
Cc: "robh+dt@...nel.org" <robh+dt@...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>
Subject: Re: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
On Tue, 16 Nov 2021 14:43:14 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@...log.com> wrote:
> Hello Jonathan,
>
> --
> Antoniu Miclăuş
>
> > -----Original Message-----
> > From: Jonathan Cameron <jic23@...nel.org>
> > Sent: Friday, November 12, 2021 7:46 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@...log.com>
> > Cc: robh+dt@...nel.org; linux-iio@...r.kernel.org;
> > devicetree@...r.kernel.org; linux-kernel@...r.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@...log.com> wrote:
> >
> > > Add device tree bindings for the ADMV8818 Filter.
> > >
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.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@...log.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>;
> > > + };
> > > + };
> > > +...
> > > +
>
Powered by blists - more mailing lists