Hello Jonathan, Adding @Thahirally, Murtaza, maybe he can help us with some more details hardware-wise. Unfortunately, the current mail format does not allow me to highlight the points of interest in the conversation. > -----Original Message----- > From: Jonathan Cameron <jic23@xxxxxxxxxx> > Sent: Saturday, July 17, 2021 5:22 PM > To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx> > Cc: linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; Rob Herring <robh@xxxxxxxxxx> > Subject: Re: [PATCH v6 2/2] dt-bindings: iio: frequency: add adrf6780 doc > > [External] > > On Fri, 16 Jul 2021 14:42:10 +0300 > Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote: > > > Add device tree bindings for the ADRF6780 Upconverter. > > > > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > > oops. Would have ideally gotten to taking a close look at this before v6 :( > Sorry about that! I don't suppose we have any other reviewers whose > knowledge of > this sort of hardware is fresher and deeper than mine? I'd like more eyes on > the next version of this if possible! > > Jonathan > > > --- > > no changes in v6 > > .../bindings/iio/frequency/adi,adrf6780.yaml | 119 ++++++++++++++++++ > > 1 file changed, 119 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/iio/frequency/adi,adrf6780.yaml > > > > diff --git > a/Documentation/devicetree/bindings/iio/frequency/adi,adrf6780.yaml > b/Documentation/devicetree/bindings/iio/frequency/adi,adrf6780.yaml > > new file mode 100644 > > index 000000000000..65cb3bee4aca > > --- /dev/null > > +++ > b/Documentation/devicetree/bindings/iio/frequency/adi,adrf6780.yaml > > @@ -0,0 +1,119 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/frequency > /adi,adrf6780.yaml*__;Iw!!A3Ni8CS0y2Y!tq2rTJBBpvfHI71YPxIn96552nFJvLYy > U6rbHeP_e5sxgwDvLPHhZ_9NMjP_lD2kj1lJ$ > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta- > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!tq2rTJBBpvfHI71YPxIn96552nFJvLY > yU6rbHeP_e5sxgwDvLPHhZ_9NMjP_lJHL0noG$ > > + > > +title: ADRF6780 Microwave Upconverter > > + > > +maintainers: > > + - Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > > + > > +description: | > > + Wideband, microwave upconverter optimized for point to point > microwave > > + radio designs operating in the 5.9 GHz to 23.6 GHz frequency range. > > + > > + https://www.analog.com/en/products/adrf6780.html > > + > > +properties: > > + compatible: > > + enum: > > + - adi,adrf6780 > > + > > + reg: > > + maxItems: 1 > > + > > + spi-max-frequency: > > + maximum: 1000000 > > + > > + clocks: > > + description: > > + Definition of the external clock. > > + minItems: 1 > > + > > + clock-names: > > + items: > > + - const: lo_in > > + > > + clock-output-names: > > + maxItems: 1 > > + > > + adi,vga-buff-en: > > + description: > > + VGA Buffer Enable. > > Ideally I'd like any acronyms spelt out in the descriptions. > (I assume this is variable gain amplifier?) The fun question from looking at > this in the datasheet is where is it in the functional diagram? I'm not actually > sure where it might be. Perhaps in the VVA, so on the VAAT input? > > If you have a convenient path to point out to your datasheet people that it is > undocumented, that might be good to clean up :) > My guess is this is needed if the precision reference on the example circuit > needs > a high impedance input, but I'm only guessing. > > > > + type: boolean > > + > > + adi,lo-buff-en: > > + description: > > + LO Buffer Enable. > > What is LO and why might it need a buffer? (or is it always a good idea to turn > this on > when using the device?) > > > + type: boolean > > + > > + adi,if-mode-en: > > + description: > > + IF Mode Enable. > IF stands for what? Intermediate Frequency... > > > + type: boolean > > + > > + adi,iq-mode-en: > > + description: > > + IQ Mode Enable. > > + type: boolean > > I struggled to figure this out from the datasheet, but is there a circumstance > under which > if and iq mode might both be enabled? Nothing stops you setting the > registers that way, but > it seems to be documented as one or the other... > > For that matter, why probably want to describe this as "baseband IQ mode" > given datasheet > refers to as baseband in most places other than the register definition. > > > + > > + adi,lo-x2-en: > > + description: > > + LO x2 Enable. > > + type: boolean > > + > > + adi,lo-ppf-en: > > + description: > > + LO x1 Enable. > > This is curious. I agree that the register write documents it as x1 enable, but it > seems > to be be enabling polyphase filters - what exactly their relationship is to x1 is > not that > clear to me. Perhaps one to query with the hardware people if possible! > > > + type: boolean > > + > > + adi,lo-en: > > + description: > > + LO Enable. > > + type: boolean > > Would you ever have this off whilst running? It's possible I'm missing > something, but > I think you need that frequency path to be enabled to get anything at all to > happen. > > > + > > + adi,uc-bias-en: > > + description: > > + UC Bias Enable. > > + type: boolean > > This being completely undocumented apart from the register setting, I have > 0 idea what > it actually is. Any chance we can get some more details? > > > + > > + adi,lo-sideband: > > + description: > > + Switch to the Other LO Sideband. > > Switch what to the other LO sideband? > > > + type: boolean > > + > > + adi,vdet-out-en: > > So my reading of the datasheet on this one didn't lead me to a completely > clear answer. > Does turning this one effectively stop you using the internal ADC to measure > the power > whilst exposing the signal on an external pin? > > > + description: > > + VDET Output Select Enable. > > + type: boolean > > + > > + '#clock-cells': > > + const: 0 > > + > > +dependencies: > > + adi,lo-x2-en: [ "adi,lo-en" ] > > + adi,lo-ppf-en: [ "adi,lo-en" ] > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + spi { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + adrf6780@0 { > > + compatible = "adi,adrf6780"; > > + reg = <0>; > > + spi-max-frequency = <1000000>; > > + clocks = <&adrf6780_lo>; > > + clock-names = "lo_in"; > > + }; > > + }; > > +...
