RE: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver

[Balas, Eliza]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Wednesday, July 26, 2023 21:35
> To: Balas, Eliza <Eliza.Balas@xxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Derek Kiernan <derek.kiernan@xxxxxxx>; Dragan Cvetic <dragan.cvetic@xxxxxxx>; Arnd Bergmann
> <arnd@xxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
> 
> [External]
> 
> On 26/07/2023 09:11, Eliza Balas wrote:
> > Add device tree documentation for the AXI TDD Core.
> > The generic TDD controller is in essence a waveform generator capable
> > of addressing RF applications which require Time Division Duplexing,
> > as well as controlling other modules of general applications through
> > its dedicated 32 channel outputs.
> >
> > The reason of creating the generic TDD controller was to reduce the
> > naming confusion around the existing repurposed TDD core built for
> > AD9361, as well as expanding its number of output channels for systems
> > which require more than six controlling signals.
> 
> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on
> the directory your patch is touching.
> 
> Subject: drop driver. Bindings are for hardware, not drivers... unless driver is here a hardware term?

It is not a hardware term in this case, I will make the changes.

> >
> > Signed-off-by: Eliza Balas <eliza.balas@xxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/misc/adi,axi-tdd.yaml | 51 +++++++++++++++++++
> >  MAINTAINERS                                   |  7 +++
> >  2 files changed, 58 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > new file mode 100644
> > index 000000000000..1894c1c34d4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> 
> Why is this in misc? No suitable directory?

I chose misc because I don't know where it should fit, I did not find a suitable
subsystem to include this driver because this is a driver for an FPGA IP core.
Do you have an idea where I should put it?

> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
> > +2023 Analog Devices Inc.
> > +%YAML 1.2
> > +---
> > +$id:
> > +https://urldefense.com/v3/__http://devicetree.org/schemas/misc/adi,ax
> > +i-tdd.yaml*__;Iw!!A3Ni8CS0y2Y!-aEgpt9-vXre1oL3ApPlyd6fzMCVcInaEiG59qJ
> > +dJJKb-sqn2VGEeQhf551gXH5KpujlBOGDfyTzaC-yEqHGJfOfc0dq-w$
> > +$schema:
> > +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
> > +aml*__;Iw!!A3Ni8CS0y2Y!-aEgpt9-vXre1oL3ApPlyd6fzMCVcInaEiG59qJdJJKb-s
> > +qn2VGEeQhf551gXH5KpujlBOGDfyTzaC-yEqHGJfMiy2YBUg$
> > +
> > +title: Analog Devices AXI TDD Core
> > +
> > +maintainers:
> > +  - Eliza Balas <eliza.balas@xxxxxxxxxx>
> > +
> > +description: |
> > +  Bindings for the new generic TDD CORE, which is part of the Analog Devices hdl reference designs.
> 
> Drop boiler plate, so "Bindings for the new generic". Instead, describe the hardware.
> 
> > +  For more information see the wiki:
> > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> 
> Not enough. Describe it more. Here.
> 
> Also, wrap according to Linux style, so at 80.
> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,axi-tdd-2.00.a
> 
> Versioned blocks... https://wiki.analog.com/resources/fpga/docs/axi_tdd
> says nothing about 2.00.a
> 
> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: System clock
> > +      - description: TDD Core clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: s_axi_aclk
> > +      - const: intf_clk
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    tdd: tdd@84a00000 {
> 
> Drop label, not used.
> 
> > +            compatible = "adi,axi-tdd-2.00.a";
> 
> Use 4 spaces for example indentation.
> 
> > +            reg = <0x84a00000 0x10000>;
> > +            clocks = <&zynqmp_clk_PL0_REF>, <&zynqmp_clk_PL1_REF>;
> > +            clock-names = "s_axi_aclk", "intf_clk";
> > +    };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > aee340630eca..280e66ccdd56 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1360,6 +1360,13 @@ F:	Documentation/devicetree/bindings/hwmon/adi,max31760.yaml
> >  F:	Documentation/hwmon/max31760.rst
> >  F:	drivers/hwmon/max31760.c
> >
> > +ANALOG DEVICES INC GENERIC TDD ENGINE DRIVER
> 
> I am pretty sure G is before M. It's difficult, I know.
> 
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c192ac7357683f78c2e6d
> 6e75adfcc29deb8c4ae__;!!A3Ni8CS0y2Y!-aEgpt9-vXre1oL3ApPlyd6fzMCVcInaEiG59qJdJJKb-sqn2VGEeQhf551gXH5KpujlBOGDfyTzaC-
> yEqHGJfPtKJuszA$
> 
> 
> Best regards,
> Krzysztof

Thank you,
Eliza