From:   "Miclaus, Antoniu" <Antoniu.Miclaus@analog.com>
To:     Jonathan Cameron <jic23@kernel.org>
CC:     "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
Thread-Topic: [PATCH 3/4] dt-bindings:iio:filter: add admv8818 doc
Thread-Index: AQHX1WXROa8gBfces0OdmTnnZfx+/6wAL4SAgAYQw/A=
Date:   Tue, 16 Nov 2021 14:43:14 +0000
Message-ID: <CY4PR03MB339956D5EAA5D20C9D6579419B999@CY4PR03MB3399.namprd03.prod.outlook.com>
References: <20211109123127.96399-1-antoniu.miclaus@analog.com>
        <20211109123127.96399-4-antoniu.miclaus@analog.com>
 <20211112174601.3c1f6b4b@jic23-huawei>
In-Reply-To: <20211112174601.3c1f6b4b@jic23-huawei>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-2?Q?hZnAywYx3sWKs/lDpe3bqXdy/4EHjvQTMv877DOPv15lO2UBMQAo+s+jae?=
 =?iso-8859-2?Q?zbWGG6nE9Y8zSpsYl058s9tnscKBMemUVjVjpjfNVu8bOTFqbX0sEazjcr?=
 =?iso-8859-2?Q?ic1ExIhJK3aYhfSAouqij/w966TamlFY2hjiVrRuETNbUQzMTdQQSbbGXG?=
 =?iso-8859-2?Q?wZXTnDO96XpbBOQqW4pUOR9MG4ut3FL1fD0QjOVZ/636iEroMcOVgyJp7L?=
 =?iso-8859-2?Q?nFLkWEFqjSvTyxliPu+Q+vZV0RccRI2c1ui5B8ZE6wBSz/FN1+SIEaKlhA?=
 =?iso-8859-2?Q?Zlx47Dqbt46gXtiBmDfRUFDY1l4wF7rrUIzJKFqFAblg8wwSgzBTDu8Pn+?=
 =?iso-8859-2?Q?Q0z2QLy27N1+LTRcYRueSRxKSgBc5lNaEqcW1cvCDwUhV/vhTegRd1y/wo?=
 =?iso-8859-2?Q?XSC7RCrHxVyc11LulI+E3E9t8WecUvC51FZiwZS5RtBnYPcRWmmvfqD00X?=
 =?iso-8859-2?Q?0H7+PM/gqcCD2AYiVHRTV27/yehpYYLv02yPu2xIFZsItQ0PzRTo0Bc/ZV?=
 =?iso-8859-2?Q?v0BNasIVmFS6NQhKVnp64K41j/tF4sqC08/zcFoPJNQWZIzCqOUoxc/pzw?=
 =?iso-8859-2?Q?yumB0ayv1Tgscd8ZM/G5L+xVtwx1Tl32v9guFpXxJgoolb2dAJkCaUGLkx?=
 =?iso-8859-2?Q?VJRWYIQcQM5Wj0PaVFkwUlllPpxUTUGVRRyTq8w7LrjjkQzAHzGZrnwQaH?=
 =?iso-8859-2?Q?MYBvJGv8f2V9C7xZcAqHWmY87bDsJhnV6mFGtGXQQ61r23itMzQMU8MUFK?=
 =?iso-8859-2?Q?+JCynci0j9YExbPoiPbgVY3yVA8AxgAhAeFO2vMDqVU+LjG7xwnLHGjL4z?=
 =?iso-8859-2?Q?zeHX9Ql2T7C6Q/aSok2Tnj3ofn/46hEzEmQGXv4sFct3xjJXdqexaUEOVV?=
 =?iso-8859-2?Q?5AwvGjAulZK7rDez+CzGpjuopQzqvoC3o0BUElZyGsvtK/ijNQETSc7BJD?=
 =?iso-8859-2?Q?QouGvwquSjJyiv4Qxhs3Y7/FBcC753FfUMoD/hIQvWKnpB7gsTdtm3yXFe?=
 =?iso-8859-2?Q?l+C8XZAiZLWmvpogc9AHVG0jzI+Htu7STZeGDpQtVOIKxbnrRrN4cu/tp5?=
 =?iso-8859-2?Q?vFwtKfpIghZ3G9F2bSVsr8P7Yft1fM0G2LdWDJRkFslREY+aeKSmpExO+l?=
 =?iso-8859-2?Q?MA4U9V2PPCH4WafMEj6+kkzfPFT2X97xjjutCEvbdxZ04cQMmg05Lo+JGo?=
 =?iso-8859-2?Q?G9NoxU2EHvWMpSL9VIoaqpVbgqREYAdFFqdcC/+fTP2j5xgBLD7ikAFWVT?=
 =?iso-8859-2?Q?3NJX8a0QVL3WEax86GiOAb2D5hThTvpUlQn66/vMeiQQv1boNgdNKbKBeq?=
 =?iso-8859-2?Q?evX9XcUsBh+eOI1Y6LVgcp4hiShREQhQhJYu4vy7ub/hsWE5W0fefuwaiZ?=
 =?iso-8859-2?Q?XvQSgvcFKENClWEOi7Mq2mdBha3PrVE6h5vdIlJwSj/jCN6S+DiOJSfnIX?=
 =?iso-8859-2?Q?cg7YXPJPkVdjpZAfroY7JgQ1u2+M0fxq43HU+LgmVeGSPD08RyeiFsZsYq?=
 =?iso-8859-2?Q?68WAF95CrIpxZC03zcxSsQiotu61FZawFwh1uB3Af5epKouKDkUfAOrYAZ?=
 =?iso-8859-2?Q?iSs/IV/54uMo+b7T4SN6tsRtE81hfyn89wbDoeGbKN3ZJnoF/RQwh24Ukv?=
 =?iso-8859-2?Q?NFK34dr9gzgGWLwUOtN+K2NERHQ81oFXJKPVNLyXrEuGf6eHwq8VZQ7QSi?=
 =?iso-8859-2?Q?5j7ZUzwIH0trgkPu405gcPghLIpuZir9unIMSD8C6FHs2tVvPFakbMK6S2?=
 =?iso-8859-2?Q?Poog=3D=3D?=
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: analog.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CY4PR03MB3399.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8b440263-6bd6-4075-b1fb-08d9a90f6b75
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Nov 2021 14:43:14.1055
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1pZK4/bOOxr6QM9AveBQFEn6+EVhNxqfZHiZ4oHkQhlxnYS7kX3UyC6DzEV7uK9EyR6nstDBlAp9XwU6weKvuUxOZfrU1/uIUCQQUwOQLuY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR03MB2139
X-Proofpoint-GUID: CC-RWWJHY8S-x0d5f0g1_hHC0rTma3oO
X-Proofpoint-ORIG-GUID: CC-RWWJHY8S-x0d5f0g1_hHC0rTma3oO
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475
 definitions=2021-11-16_02,2021-11-16_01,2020-04-07_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015
 lowpriorityscore=0 bulkscore=0 adultscore=0 impostorscore=0 malwarescore=0
 mlxlogscore=999 priorityscore=1501 mlxscore=0 phishscore=0 spamscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2111160072
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Jonathan,

--
Antoniu Micl=E3u=BA

> -----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
>=20
> [External]
>=20
> On Tue, 9 Nov 2021 14:31:26 +0200
> Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
>=20
> > 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"
>=20
> 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.
>=20
The only actual use of this part, until now, was to filter the output of th=
e following part:
https://www.analog.com/en/products/adf5610.html
This is the reason of using the clock framework in the driver. Moreover, th=
e 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.
>=20
> I don't understand this item at all.  Why do we need a control to
> basically change how the other filter parameters are expressed?
>=20

Indeed, this property was requested by the users of the application in whic=
h 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 the=
m.

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 =3D <1>;
> > +      #size-cells =3D <0>;
> > +      admv8818@0 {
> > +        compatible =3D "adi,admv8818";
> > +        reg =3D <0>;
> > +        spi-max-frequency =3D <10000000>;
> > +        clocks =3D <&admv8818_rfin>;
> > +        clock-names =3D "rf_in";
> > +        adi,bw-hz =3D /bits/ 64 <600000000>;
> > +      };
> > +    };
> > +...
> > +