2023-09-28 09:58:25

by Balas, Eliza

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

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.

Signed-off-by: Eliza Balas <[email protected]>
---
.../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
MAINTAINERS | 7 ++
2 files changed, 72 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..8938da801b95
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2023 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/adi,axi-tdd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AXI TDD Core
+
+maintainers:
+ - Eliza Balas <[email protected]>
+
+description: |
+ The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
+ multiple frames of data through multiple buffers.
+ The TDD IP core is part of the Analog Devices hdl reference designs and has
+ the following features:
+ * Up to 32 independent output channels
+ * Start/stop time values per channel
+ * Enable and polarity bit values per channel
+ * 32 bit-max internal reference counter
+ * Initial startup delay before waveform generation
+ * Configurable frame length and number of frames per burst
+ * 3 sources of synchronization: external, internal and software generated
+ For more information see the wiki:
+ https://wiki.analog.com/resources/fpga/docs/axi_tdd
+
+properties:
+ compatible:
+ enum:
+ - adi,axi-tdd-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@84a00000 {
+ compatible = "adi,axi-tdd-2.00.a";
+ 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 bf0f54c24f81..c5cc69c83c39 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1345,6 +1345,13 @@ S: Supported
W: https://ez.analog.com/linux-software-drivers
F: drivers/dma/dma-axi-dmac.c

+ANALOG DEVICES INC GENERIC TDD ENGINE DRIVER
+M: Eliza Balas <[email protected]>
+S: Supported
+W: http://wiki.analog.com/resources/fpga/docs/axi_tdd
+W: http://ez.analog.com/linux-software-drivers/
+F: Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
+
ANALOG DEVICES INC IIO DRIVERS
M: Lars-Peter Clausen <[email protected]>
M: Michael Hennerich <[email protected]>
--
2.25.1


2023-10-02 16:54:51

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
>
> Signed-off-by: Eliza Balas <[email protected]>
> ---
> .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> MAINTAINERS | 7 ++
> 2 files changed, 72 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..8938da801b95
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/adi,axi-tdd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AXI TDD Core
> +
> +maintainers:
> + - Eliza Balas <[email protected]>
> +
> +description: |
> + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> + multiple frames of data through multiple buffers.
> + The TDD IP core is part of the Analog Devices hdl reference designs and has
> + the following features:
> + * Up to 32 independent output channels
> + * Start/stop time values per channel
> + * Enable and polarity bit values per channel
> + * 32 bit-max internal reference counter
> + * Initial startup delay before waveform generation
> + * Configurable frame length and number of frames per burst
> + * 3 sources of synchronization: external, internal and software generated
> + For more information see the wiki:
> + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> +
> +properties:
> + compatible:
> + enum:
> + - adi,axi-tdd-2.00.a

Where does this version number come from? I looked at the above link and
see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
whether there's some per IP version.

If you want to use version numbers, please document the versioning
scheme. For example, see
Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.

Rob

2023-10-02 19:33:34

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

On Mon, Oct 02, 2023 at 04:46:26PM +0000, Balas, Eliza wrote:
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: Monday, October 2, 2023 19:33
> > To: Balas, Eliza <[email protected]>
> > Cc: Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Derek Kiernan
> > <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> > <[email protected]>; [email protected]; [email protected]
> > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> >
> > [External]
> >
> > On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
> > >
> > > Signed-off-by: Eliza Balas <[email protected]>
> > > ---
> > > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> > > MAINTAINERS | 7 ++
> > > 2 files changed, 72 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..8938da801b95
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > @@ -0,0 +1,65 @@
> > > +# 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,axi-
> > tdd.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_TR9yjTKw$
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_RK8aQ9xw$
> > > +
> > > +title: Analog Devices AXI TDD Core
> > > +
> > > +maintainers:
> > > + - Eliza Balas <[email protected]>
> > > +
> > > +description: |
> > > + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> > > + multiple frames of data through multiple buffers.
> > > + The TDD IP core is part of the Analog Devices hdl reference designs and has
> > > + the following features:
> > > + * Up to 32 independent output channels
> > > + * Start/stop time values per channel
> > > + * Enable and polarity bit values per channel
> > > + * 32 bit-max internal reference counter
> > > + * Initial startup delay before waveform generation
> > > + * Configurable frame length and number of frames per burst
> > > + * 3 sources of synchronization: external, internal and software generated
> > > + For more information see the wiki:
> > > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - adi,axi-tdd-2.00.a
> >
> > Where does this version number come from? I looked at the above link and
> > see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
> > whether there's some per IP version.
> >
> > If you want to use version numbers, please document the versioning
> > scheme. For example, see
> > Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.
> >
> > Rob
>
> The version refers to the IP version. The version of the IP is also
> specified in its VERSION register (there is a drop down to expand the
> register map on the wiki page) which is verified by the driver during
> probe. "2021_R2" refers to the compatible tool version used for
> creating the FPGAIP Core.

If you have version registers in these IPs, what benefit does version
numbers in the compatible string bring?
Rather than using the version numbers to validate what the DT gave you,
which not the kernel's job IMO, why not just use the information from
the register to determine the version?

Cheers,
Conor.


Attachments:
(No filename) (4.80 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-02 20:21:39

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

On Mon, Oct 02, 2023 at 07:48:42PM +0000, Balas, Eliza wrote:
> > -----Original Message-----
> > From: Conor Dooley <[email protected]>
> > Sent: Monday, October 2, 2023 22:21
> > To: Balas, Eliza <[email protected]>
> > Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>;
> > Derek Kiernan <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> > Kroah-Hartman <[email protected]>; [email protected]; [email protected]
> > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> >
> > [External]
> >
> > On Mon, Oct 02, 2023 at 04:46:26PM +0000, Balas, Eliza wrote:
> > > > -----Original Message-----
> > > > From: Rob Herring <[email protected]>
> > > > Sent: Monday, October 2, 2023 19:33
> > > > To: Balas, Eliza <[email protected]>
> > > > Cc: Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Derek Kiernan
> > > > <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> > > > <[email protected]>; [email protected]; [email protected]
> > > > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> > > >
> > > > [External]
> > > >
> > > > On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
> > > > >
> > > > > Signed-off-by: Eliza Balas <[email protected]>
> > > > > ---
> > > > > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> > > > > MAINTAINERS | 7 ++
> > > > > 2 files changed, 72 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..8938da801b95
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > > @@ -0,0 +1,65 @@
> > > > > +# 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,axi-
> > > > tdd.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_TR9yjTKw$
> > > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > > >
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_RK8aQ9xw$
> > > > > +
> > > > > +title: Analog Devices AXI TDD Core
> > > > > +
> > > > > +maintainers:
> > > > > + - Eliza Balas <[email protected]>
> > > > > +
> > > > > +description: |
> > > > > + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> > > > > + multiple frames of data through multiple buffers.
> > > > > + The TDD IP core is part of the Analog Devices hdl reference designs and has
> > > > > + the following features:
> > > > > + * Up to 32 independent output channels
> > > > > + * Start/stop time values per channel
> > > > > + * Enable and polarity bit values per channel
> > > > > + * 32 bit-max internal reference counter
> > > > > + * Initial startup delay before waveform generation
> > > > > + * Configurable frame length and number of frames per burst
> > > > > + * 3 sources of synchronization: external, internal and software generated
> > > > > + For more information see the wiki:
> > > > > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + enum:
> > > > > + - adi,axi-tdd-2.00.a
> > > >
> > > > Where does this version number come from? I looked at the above link and
> > > > see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
> > > > whether there's some per IP version.
> > > >
> > > > If you want to use version numbers, please document the versioning
> > > > scheme. For example, see
> > > > Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.
> > > >
> > > > Rob
> > >
> > > The version refers to the IP version. The version of the IP is also
> > > specified in its VERSION register (there is a drop down to expand the
> > > register map on the wiki page) which is verified by the driver during
> > > probe. "2021_R2" refers to the compatible tool version used for
> > > creating the FPGAIP Core.
> >
> > If you have version registers in these IPs, what benefit does version
> > numbers in the compatible string bring?
> > Rather than using the version numbers to validate what the DT gave you,
> > which not the kernel's job IMO, why not just use the information from
> > the register to determine the version?
> >
> > Cheers,
> > Conor.
>
> As the description of this patch says, we want to resolve the naming confusion around the existing repurposed TDD core (https://wiki.analog.com/resources/eval/user-guides/ad-pzsdr2400tdd-eb/reference_hdl#tdd_controller)
> built for AD9361 and this TDD Engine IP core (https://wiki.analog.com/resources/fpga/docs/axi_tdd) which is a similar core, with more output channels and some extra features. The version numbers in the compatible string are used to differentiate between the two IPs.

Firstly, please fix your mail client to wrap text at a sane width :)
Secondly, where is the binding for that TDD ad9361 specific core that
calling this generic one "adi,axi-tdd" would conflict with?
Grepping the bindings directory of the kernel tree for "adi.*tdd" returns
nothing. If there is an ad9361 specific tdd, I would expect it to have a
compatible like "adi,ad9361-tdd".


Attachments:
(No filename) (6.75 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-02 22:10:43

by Balas, Eliza

[permalink] [raw]
Subject: RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Monday, October 2, 2023 19:33
> To: Balas, Eliza <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Derek Kiernan
> <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
>
> [External]
>
> On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
> >
> > Signed-off-by: Eliza Balas <[email protected]>
> > ---
> > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> > MAINTAINERS | 7 ++
> > 2 files changed, 72 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..8938da801b95
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > @@ -0,0 +1,65 @@
> > +# 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,axi-
> tdd.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_TR9yjTKw$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_RK8aQ9xw$
> > +
> > +title: Analog Devices AXI TDD Core
> > +
> > +maintainers:
> > + - Eliza Balas <[email protected]>
> > +
> > +description: |
> > + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> > + multiple frames of data through multiple buffers.
> > + The TDD IP core is part of the Analog Devices hdl reference designs and has
> > + the following features:
> > + * Up to 32 independent output channels
> > + * Start/stop time values per channel
> > + * Enable and polarity bit values per channel
> > + * 32 bit-max internal reference counter
> > + * Initial startup delay before waveform generation
> > + * Configurable frame length and number of frames per burst
> > + * 3 sources of synchronization: external, internal and software generated
> > + For more information see the wiki:
> > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,axi-tdd-2.00.a
>
> Where does this version number come from? I looked at the above link and
> see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
> whether there's some per IP version.
>
> If you want to use version numbers, please document the versioning
> scheme. For example, see
> Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.
>
> Rob

The version refers to the IP version. The version of the IP is also specified in its VERSION register (there is a drop down to expand the register map on the wiki page) which is verified by the driver during probe.
"2021_R2" refers to the compatible tool version used for creating the FPGA IP Core.

I will document the versioning scheme.

Thank you,
Eliza

2023-10-02 22:10:43

by Balas, Eliza

[permalink] [raw]
Subject: RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Monday, October 2, 2023 22:21
> To: Balas, Eliza <[email protected]>
> Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>;
> Derek Kiernan <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> Kroah-Hartman <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
>
> [External]
>
> On Mon, Oct 02, 2023 at 04:46:26PM +0000, Balas, Eliza wrote:
> > > -----Original Message-----
> > > From: Rob Herring <[email protected]>
> > > Sent: Monday, October 2, 2023 19:33
> > > To: Balas, Eliza <[email protected]>
> > > Cc: Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Derek Kiernan
> > > <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-Hartman
> > > <[email protected]>; [email protected]; [email protected]
> > > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> > >
> > > [External]
> > >
> > > On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
> > > >
> > > > Signed-off-by: Eliza Balas <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> > > > MAINTAINERS | 7 ++
> > > > 2 files changed, 72 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..8938da801b95
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > @@ -0,0 +1,65 @@
> > > > +# 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,axi-
> > > tdd.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_TR9yjTKw$
> > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > >
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_RK8aQ9xw$
> > > > +
> > > > +title: Analog Devices AXI TDD Core
> > > > +
> > > > +maintainers:
> > > > + - Eliza Balas <[email protected]>
> > > > +
> > > > +description: |
> > > > + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> > > > + multiple frames of data through multiple buffers.
> > > > + The TDD IP core is part of the Analog Devices hdl reference designs and has
> > > > + the following features:
> > > > + * Up to 32 independent output channels
> > > > + * Start/stop time values per channel
> > > > + * Enable and polarity bit values per channel
> > > > + * 32 bit-max internal reference counter
> > > > + * Initial startup delay before waveform generation
> > > > + * Configurable frame length and number of frames per burst
> > > > + * 3 sources of synchronization: external, internal and software generated
> > > > + For more information see the wiki:
> > > > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - adi,axi-tdd-2.00.a
> > >
> > > Where does this version number come from? I looked at the above link and
> > > see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
> > > whether there's some per IP version.
> > >
> > > If you want to use version numbers, please document the versioning
> > > scheme. For example, see
> > > Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.
> > >
> > > Rob
> >
> > The version refers to the IP version. The version of the IP is also
> > specified in its VERSION register (there is a drop down to expand the
> > register map on the wiki page) which is verified by the driver during
> > probe. "2021_R2" refers to the compatible tool version used for
> > creating the FPGAIP Core.
>
> If you have version registers in these IPs, what benefit does version
> numbers in the compatible string bring?
> Rather than using the version numbers to validate what the DT gave you,
> which not the kernel's job IMO, why not just use the information from
> the register to determine the version?
>
> Cheers,
> Conor.

As the description of this patch says, we want to resolve the naming confusion around the existing repurposed TDD core (https://wiki.analog.com/resources/eval/user-guides/ad-pzsdr2400tdd-eb/reference_hdl#tdd_controller)
built for AD9361 and this TDD Engine IP core (https://wiki.analog.com/resources/fpga/docs/axi_tdd) which is a similar core, with more output channels and some extra features. The version numbers in the compatible string are used to differentiate between the two IPs.

Best regards,
Eliza

2023-10-02 22:50:39

by Balas, Eliza

[permalink] [raw]
Subject: RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine



> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Monday, October 2, 2023 23:07
> To: Balas, Eliza <[email protected]>
> Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>;
> Derek Kiernan <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> Kroah-Hartman <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
>
> [External]
>
> On Mon, Oct 02, 2023 at 07:48:42PM +0000, Balas, Eliza wrote:
> > > -----Original Message-----
> > > From: Conor Dooley <[email protected]>
> > > Sent: Monday, October 2, 2023 22:21
> > > To: Balas, Eliza <[email protected]>
> > > Cc: Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>;
> > > Derek Kiernan <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg
> > > Kroah-Hartman <[email protected]>; [email protected]; [email protected]
> > > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> > >
> > > [External]
> > >
> > > On Mon, Oct 02, 2023 at 04:46:26PM +0000, Balas, Eliza wrote:
> > > > > -----Original Message-----
> > > > > From: Rob Herring <[email protected]>
> > > > > Sent: Monday, October 2, 2023 19:33
> > > > > To: Balas, Eliza <[email protected]>
> > > > > Cc: Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Derek Kiernan
> > > > > <[email protected]>; Dragan Cvetic <[email protected]>; Arnd Bergmann <[email protected]>; Greg Kroah-
> Hartman
> > > > > <[email protected]>; [email protected]; [email protected]
> > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
> > > > >
> > > > > [External]
> > > > >
> > > > > On Thu, Sep 28, 2023 at 12:28:03PM +0300, 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.
> > > > > >
> > > > > > Signed-off-by: Eliza Balas <[email protected]>
> > > > > > ---
> > > > > > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 +++++++++++++++++++
> > > > > > MAINTAINERS | 7 ++
> > > > > > 2 files changed, 72 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..8938da801b95
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > > > @@ -0,0 +1,65 @@
> > > > > > +# 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,axi-
> > > > > tdd.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_TR9yjTKw$
> > > > > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-
> > > > >
> > >
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!5Cxus2huppjhkiJZLWTpJEgA0IXdLZx4t0fS9J1yt0xgjp9g3Y1N5PbZ7pAcIkKU4WPbS_RK8aQ9xw$
> > > > > > +
> > > > > > +title: Analog Devices AXI TDD Core
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Eliza Balas <[email protected]>
> > > > > > +
> > > > > > +description: |
> > > > > > + The TDD controller is 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. It solves the synchronization issue when transmitting and receiving
> > > > > > + multiple frames of data through multiple buffers.
> > > > > > + The TDD IP core is part of the Analog Devices hdl reference designs and has
> > > > > > + the following features:
> > > > > > + * Up to 32 independent output channels
> > > > > > + * Start/stop time values per channel
> > > > > > + * Enable and polarity bit values per channel
> > > > > > + * 32 bit-max internal reference counter
> > > > > > + * Initial startup delay before waveform generation
> > > > > > + * Configurable frame length and number of frames per burst
> > > > > > + * 3 sources of synchronization: external, internal and software generated
> > > > > > + For more information see the wiki:
> > > > > > + https://wiki.analog.com/resources/fpga/docs/axi_tdd
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - adi,axi-tdd-2.00.a
> > > > >
> > > > > Where does this version number come from? I looked at the above link and
> > > > > see versions such as '2021_R2', '2019_r2', etc. I didn't dig deeper
> > > > > whether there's some per IP version.
> > > > >
> > > > > If you want to use version numbers, please document the versioning
> > > > > scheme. For example, see
> > > > > Documentation/devicetree/bindings/sifive/sifive-blocks-ip-versioning.txt.
> > > > >
> > > > > Rob
> > > >
> > > > The version refers to the IP version. The version of the IP is also
> > > > specified in its VERSION register (there is a drop down to expand the
> > > > register map on the wiki page) which is verified by the driver during
> > > > probe. "2021_R2" refers to the compatible tool version used for
> > > > creating the FPGAIP Core.
> > >
> > > If you have version registers in these IPs, what benefit does version
> > > numbers in the compatible string bring?
> > > Rather than using the version numbers to validate what the DT gave you,
> > > which not the kernel's job IMO, why not just use the information from
> > > the register to determine the version?
> > >
> > > Cheers,
> > > Conor.
> >
> > As the description of this patch says, we want to resolve the naming confusion around the existing repurposed TDD core
> (https://wiki.analog.com/resources/eval/user-guides/ad-pzsdr2400tdd-eb/reference_hdl#tdd_controller)
> > built for AD9361 and this TDD Engine IP core (https://wiki.analog.com/resources/fpga/docs/axi_tdd) which is a similar core, with
> more output channels and some extra features. The version numbers in the compatible string are used to differentiate between the
> two IPs.
>
> Firstly, please fix your mail client to wrap text at a sane width :)
> Secondly, where is the binding for that TDD ad9361 specific core that
> calling this generic one "adi,axi-tdd" would conflict with?
> Grepping the bindings directory of the kernel tree for "adi.*tdd" returns
> nothing. If there is an ad9361 specific tdd, I would expect it to have a
> compatible like "adi,ad9361-tdd".

We didn't upstream the ad9361 tdd driver, but we are using it
in our internal kernel. If this is an issue, I will change the
compatible string to "adi,axi-tdd".

2023-10-03 08:58:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine

On 02/10/2023 22:23, Balas, Eliza wrote:
>>>> If you have version registers in these IPs, what benefit does version
>>>> numbers in the compatible string bring?
>>>> Rather than using the version numbers to validate what the DT gave you,
>>>> which not the kernel's job IMO, why not just use the information from
>>>> the register to determine the version?
>>>>
>>>> Cheers,
>>>> Conor.
>>>
>>> As the description of this patch says, we want to resolve the naming confusion around the existing repurposed TDD core
>> (https://wiki.analog.com/resources/eval/user-guides/ad-pzsdr2400tdd-eb/reference_hdl#tdd_controller)
>>> built for AD9361 and this TDD Engine IP core (https://wiki.analog.com/resources/fpga/docs/axi_tdd) which is a similar core, with
>> more output channels and some extra features. The version numbers in the compatible string are used to differentiate between the
>> two IPs.
>>
>> Firstly, please fix your mail client to wrap text at a sane width :)
>> Secondly, where is the binding for that TDD ad9361 specific core that
>> calling this generic one "adi,axi-tdd" would conflict with?
>> Grepping the bindings directory of the kernel tree for "adi.*tdd" returns
>> nothing. If there is an ad9361 specific tdd, I would expect it to have a
>> compatible like "adi,ad9361-tdd".
>
> We didn't upstream the ad9361 tdd driver, but we are using it
> in our internal kernel. If this is an issue, I will change the
> compatible string to "adi,axi-tdd".
>

Unfortunately, we do not care much about your internal kernel and
whatever you have there should rarely be an argument in upstream
discussions.

Best regards,
Krzysztof