* [PATCH v2 0/2] Add support for ADI TDD Engine
@ 2023-09-28 9:28 Eliza Balas
2023-09-28 9:28 ` [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine Eliza Balas
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Eliza Balas @ 2023-09-28 9:28 UTC (permalink / raw)
Cc: Eliza Balas, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
linux-kernel, devicetree
V1 -> V2:
* remove label in examples in bindings file
* add detailed description of the hardware in bindings file
* remove adi_axi_tdd_clk_disable function
* remove devm_add_action_or_reset, devm_clk_get, clk_prepare_enable
and use instead devm_clk_get_enabled
Eliza Balas (2):
dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine
drivers: misc: adi-axi-tdd: Add TDD engine
.../sysfs-bus-platform-drivers-adi-axi-tdd | 158 ++++
.../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 ++
MAINTAINERS | 9 +
drivers/misc/Kconfig | 10 +
drivers/misc/Makefile | 1 +
drivers/misc/adi-axi-tdd.c | 780 ++++++++++++++++++
6 files changed, 1023 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd
create mode 100644 Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
create mode 100644 drivers/misc/adi-axi-tdd.c
--
2.25.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-09-28 9:28 [PATCH v2 0/2] Add support for ADI TDD Engine Eliza Balas @ 2023-09-28 9:28 ` Eliza Balas 2023-10-02 16:32 ` Rob Herring 2023-09-28 9:28 ` [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add " Eliza Balas 2023-09-28 9:38 ` [PATCH v2 0/2] Add support for ADI TDD Engine Andreas Herrmann 2 siblings, 1 reply; 20+ messages in thread From: Eliza Balas @ 2023-09-28 9:28 UTC (permalink / raw) Cc: Eliza Balas, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, devicetree 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 <eliza.balas@analog.com> --- .../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 <eliza.balas@analog.com> + +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 <eliza.balas@analog.com> +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 <lars@metafoo.de> M: Michael Hennerich <Michael.Hennerich@analog.com> -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-09-28 9:28 ` [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine Eliza Balas @ 2023-10-02 16:32 ` Rob Herring 2023-10-02 16:46 ` Balas, Eliza 0 siblings, 1 reply; 20+ messages in thread From: Rob Herring @ 2023-10-02 16:32 UTC (permalink / raw) To: Eliza Balas Cc: Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, devicetree 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 <eliza.balas@analog.com> > --- > .../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 <eliza.balas@analog.com> > + > +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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 16:32 ` Rob Herring @ 2023-10-02 16:46 ` Balas, Eliza 2023-10-02 19:21 ` Conor Dooley 0 siblings, 1 reply; 20+ messages in thread From: Balas, Eliza @ 2023-10-02 16:46 UTC (permalink / raw) To: Rob Herring Cc: Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Monday, October 2, 2023 19:33 > To: Balas, Eliza <Eliza.Balas@analog.com> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Derek Kiernan > <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > 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 <eliza.balas@analog.com> > > --- > > .../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 <eliza.balas@analog.com> > > + > > +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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 16:46 ` Balas, Eliza @ 2023-10-02 19:21 ` Conor Dooley 2023-10-02 19:48 ` Balas, Eliza 0 siblings, 1 reply; 20+ messages in thread From: Conor Dooley @ 2023-10-02 19:21 UTC (permalink / raw) To: Balas, Eliza Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 4917 bytes --] On Mon, Oct 02, 2023 at 04:46:26PM +0000, Balas, Eliza wrote: > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Monday, October 2, 2023 19:33 > > To: Balas, Eliza <Eliza.Balas@analog.com> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Derek Kiernan > > <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman > > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > 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 <eliza.balas@analog.com> > > > --- > > > .../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 <eliza.balas@analog.com> > > > + > > > +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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 19:21 ` Conor Dooley @ 2023-10-02 19:48 ` Balas, Eliza 2023-10-02 20:07 ` Conor Dooley 0 siblings, 1 reply; 20+ messages in thread From: Balas, Eliza @ 2023-10-02 19:48 UTC (permalink / raw) To: Conor Dooley Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org > -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Monday, October 2, 2023 22:21 > To: Balas, Eliza <Eliza.Balas@analog.com> > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Derek Kiernan <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg > Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > 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 <robh@kernel.org> > > > Sent: Monday, October 2, 2023 19:33 > > > To: Balas, Eliza <Eliza.Balas@analog.com> > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Derek Kiernan > > > <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman > > > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > 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 <eliza.balas@analog.com> > > > > --- > > > > .../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 <eliza.balas@analog.com> > > > > + > > > > +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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 19:48 ` Balas, Eliza @ 2023-10-02 20:07 ` Conor Dooley 2023-10-02 20:23 ` Balas, Eliza 0 siblings, 1 reply; 20+ messages in thread From: Conor Dooley @ 2023-10-02 20:07 UTC (permalink / raw) To: Balas, Eliza Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 6915 bytes --] On Mon, Oct 02, 2023 at 07:48:42PM +0000, Balas, Eliza wrote: > > -----Original Message----- > > From: Conor Dooley <conor@kernel.org> > > Sent: Monday, October 2, 2023 22:21 > > To: Balas, Eliza <Eliza.Balas@analog.com> > > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > > Derek Kiernan <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg > > Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > 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 <robh@kernel.org> > > > > Sent: Monday, October 2, 2023 19:33 > > > > To: Balas, Eliza <Eliza.Balas@analog.com> > > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Derek Kiernan > > > > <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman > > > > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > > 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 <eliza.balas@analog.com> > > > > > --- > > > > > .../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 <eliza.balas@analog.com> > > > > > + > > > > > +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". [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 20:07 ` Conor Dooley @ 2023-10-02 20:23 ` Balas, Eliza 2023-10-03 8:54 ` Krzysztof Kozlowski 0 siblings, 1 reply; 20+ messages in thread From: Balas, Eliza @ 2023-10-02 20:23 UTC (permalink / raw) To: Conor Dooley Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org > -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Monday, October 2, 2023 23:07 > To: Balas, Eliza <Eliza.Balas@analog.com> > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Derek Kiernan <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg > Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > 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 <conor@kernel.org> > > > Sent: Monday, October 2, 2023 22:21 > > > To: Balas, Eliza <Eliza.Balas@analog.com> > > > Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org>; > > > Derek Kiernan <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg > > > Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > 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 <robh@kernel.org> > > > > > Sent: Monday, October 2, 2023 19:33 > > > > > To: Balas, Eliza <Eliza.Balas@analog.com> > > > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Derek Kiernan > > > > > <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann <arnd@arndb.de>; Greg Kroah- > Hartman > > > > > <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > > > 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 <eliza.balas@analog.com> > > > > > > --- > > > > > > .../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 <eliza.balas@analog.com> > > > > > > + > > > > > > +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". ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine 2023-10-02 20:23 ` Balas, Eliza @ 2023-10-03 8:54 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2023-10-03 8:54 UTC (permalink / raw) To: Balas, Eliza, Conor Dooley Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 9:28 [PATCH v2 0/2] Add support for ADI TDD Engine Eliza Balas 2023-09-28 9:28 ` [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine Eliza Balas @ 2023-09-28 9:28 ` Eliza Balas 2023-09-28 10:07 ` Arnd Bergmann 2023-09-28 12:23 ` Greg Kroah-Hartman 2023-09-28 9:38 ` [PATCH v2 0/2] Add support for ADI TDD Engine Andreas Herrmann 2 siblings, 2 replies; 20+ messages in thread From: Eliza Balas @ 2023-09-28 9:28 UTC (permalink / raw) Cc: Eliza Balas, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, devicetree This patch introduces the driver for the new ADI TDD engine HDL. 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 <eliza.balas@analog.com> --- .../sysfs-bus-platform-drivers-adi-axi-tdd | 158 ++++ MAINTAINERS | 2 + drivers/misc/Kconfig | 10 + drivers/misc/Makefile | 1 + drivers/misc/adi-axi-tdd.c | 780 ++++++++++++++++++ 5 files changed, 951 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd create mode 100644 drivers/misc/adi-axi-tdd.c diff --git a/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd b/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd new file mode 100644 index 000000000000..acbb3dcf10f1 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd @@ -0,0 +1,158 @@ +What: /sys/bus/platform/drivers/adi-axi-tdd/*/burst_count +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the number of TDD frames per burst. + If set to 0x0 and the TDD module is enabled, then the controller + operates in TDD mode as long as the ENABLE property is set. + If set to a non-zero value, the controller + operates for the set number of frames and then stops. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/core_id +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Displays the value of the ID configuration parameter. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/enable +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to enable or disable the TDD module. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/frame_length_ms +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the length of the transmission frame, + defined in milliseconds. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/frame_length_raw +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the length of the transmission frame, + defined in clock cycles of the input clock. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/internal_sync_period_ms +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the period of the internal sync generator, + defined in milliseconds. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/internal_sync_period_raw +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the period of the internal sync generator, + defined in clock cycles of the input clock. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/magic +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Displays the code name of the TDD module for identification. + The value is 0x5444444E ('T', 'D', 'D', 'N'). + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_enable +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to enable or disable the output channel CH<n>. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_off_ms +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the offset from the beginning of a new + frame when channel CH<n> is reset, defined in milliseconds. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_off_raw +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the offset from the beginning of a new + frame when channel CH<n> is reset, defined in clock cycles + of the input clock. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_on_ms +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the offset from the beginning of a new + frame when channel CH<n> is set, defined in milliseconds. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_on_raw +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the offset from the beginning of a new + frame when channel CH<n> is set, defined in clock cycles + of the input clock. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/out_channel<n>_polarity +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the polarity of the output channel CH<n>. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/scratch +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to write and read the scratch register for + debugging purposes. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/startup_delay_ms +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the initial delay before the first frame, + defined in milliseconds. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/startup_delay_raw +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to set the initial delay before the first frame, + defined in clock cycles of the input clock. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/state +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Displays the current state of the peripheral FSM, used for + debugging purposes. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/sync_external +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to enable or disable the external sync trigger. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/sync_internal +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to enable or disable the internal sync trigger. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/sync_reset +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to reset the internal counter when receiving a + sync event. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/sync_soft +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Allows the user to trigger one software sync pulse. + +What: /sys/bus/platform/drivers/adi-axi-tdd/*/version +Date: November 2023 +KernelVersion: 6.7 +Contact: Eliza Balas <eliza.balas@analog.com> +Description: Displays the version of the peripheral. Follows semantic + versioning. Current version is 2.00.61. diff --git a/MAINTAINERS b/MAINTAINERS index c5cc69c83c39..9862704cec7a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1350,7 +1350,9 @@ M: Eliza Balas <eliza.balas@analog.com> S: Supported W: http://wiki.analog.com/resources/fpga/docs/axi_tdd W: http://ez.analog.com/linux-software-drivers/ +F: Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd F: Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml +F: drivers/misc/adi-axi-tdd.c ANALOG DEVICES INC IIO DRIVERS M: Lars-Peter Clausen <lars@metafoo.de> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index cadd4a820c03..45074a93fa55 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -50,6 +50,16 @@ config AD525X_DPOT_SPI To compile this driver as a module, choose M here: the module will be called ad525x_dpot-spi. +config ADI_AXI_TDD + tristate "Analog Devices TDD Engine support" + depends on HAS_IOMEM + select REGMAP_MMIO + help + The ADI AXI TDD core is the reworked and generic TDD engine which + was developed for general use in Analog Devices HDL projects. Unlike + the previous TDD engine, this core can only be used standalone mode, + and is not embedded into other devices. + config DUMMY_IRQ tristate "Dummy IRQ handler" help diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index f2a4d1ff65d4..d97afe1eeba5 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_IBMVMC) += ibmvmc.o obj-$(CONFIG_AD525X_DPOT) += ad525x_dpot.o obj-$(CONFIG_AD525X_DPOT_I2C) += ad525x_dpot-i2c.o obj-$(CONFIG_AD525X_DPOT_SPI) += ad525x_dpot-spi.o +obj-$(CONFIG_ADI_AXI_TDD) += adi-axi-tdd.o obj-$(CONFIG_ATMEL_SSC) += atmel-ssc.o obj-$(CONFIG_DUMMY_IRQ) += dummy-irq.o obj-$(CONFIG_ICS932S401) += ics932s401.o diff --git a/drivers/misc/adi-axi-tdd.c b/drivers/misc/adi-axi-tdd.c new file mode 100644 index 000000000000..764e65086e83 --- /dev/null +++ b/drivers/misc/adi-axi-tdd.c @@ -0,0 +1,780 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) +/* + * TDD HDL CORE driver + * + * Copyright 2023 Analog Devices Inc. + * + */ +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/clk.h> +#include <linux/device.h> +#include <linux/fpga/adi-axi-common.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/notifier.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/sysfs.h> + +/* Register Map */ +#define ADI_REG_TDD_VERSION 0x0000 +#define ADI_REG_TDD_PERIPHERAL_ID 0x0004 +#define ADI_REG_TDD_SCRATCH 0x0008 +#define ADI_REG_TDD_IDENTIFICATION 0x000c +#define ADI_REG_TDD_INTERFACE_DESCRIPTION 0x0010 +#define ADI_REG_TDD_DEFAULT_POLARITY 0x0014 +#define ADI_REG_TDD_CONTROL 0x0040 +#define ADI_REG_TDD_CHANNEL_ENABLE 0x0044 +#define ADI_REG_TDD_CHANNEL_POLARITY 0x0048 +#define ADI_REG_TDD_BURST_COUNT 0x004c +#define ADI_REG_TDD_STARTUP_DELAY 0x0050 +#define ADI_REG_TDD_FRAME_LENGTH 0x0054 +#define ADI_REG_TDD_SYNC_COUNTER_LOW 0x0058 +#define ADI_REG_TDD_SYNC_COUNTER_HIGH 0x005c +#define ADI_REG_TDD_STATUS 0x0060 +#define ADI_REG_TDD_CHANNEL_BASE 0x0080 + +/* Identification Register */ +#define ADI_TDD_MAGIC 0x5444444E + +/* Interface Description Register */ +#define ADI_TDD_SYNC_COUNT_WIDTH GENMASK(30, 24) +#define ADI_TDD_BURST_COUNT_WIDTH GENMASK(21, 16) +#define ADI_TDD_REG_WIDTH GENMASK(13, 8) +#define ADI_TDD_SYNC_EXTERNAL_CDC BIT(7) +#define ADI_TDD_SYNC_EXTERNAL BIT(6) +#define ADI_TDD_SYNC_INTERNAL BIT(5) +#define ADI_TDD_CHANNEL_COUNT GENMASK(4, 0) + +/* Control Register */ +#define ADI_TDD_SYNC_SOFT BIT(4) +#define ADI_TDD_SYNC_EXT BIT(3) +#define ADI_TDD_SYNC_INT BIT(2) +#define ADI_TDD_SYNC_RST BIT(1) +#define ADI_TDD_ENABLE BIT(0) + +/* Channel Definitions */ +#define ADI_TDD_CHANNEL_OFFSET 0x0008 +#define ADI_TDD_CHANNEL_ON 0x0000 +#define ADI_TDD_CHANNEL_OFF 0x0004 + +#define ADI_REG_TDD_CHANNEL(c, o) \ + (ADI_REG_TDD_CHANNEL_BASE + ADI_TDD_CHANNEL_OFFSET * (c) + (o)) + +enum adi_axi_tdd_attribute_id { + ADI_TDD_ATTR_VERSION, + ADI_TDD_ATTR_CORE_ID, + ADI_TDD_ATTR_SCRATCH, + ADI_TDD_ATTR_MAGIC, + + ADI_TDD_ATTR_SYNC_SOFT, + ADI_TDD_ATTR_SYNC_EXT, + ADI_TDD_ATTR_SYNC_INT, + ADI_TDD_ATTR_SYNC_RST, + ADI_TDD_ATTR_ENABLE, + + ADI_TDD_ATTR_BURST_COUNT, + ADI_TDD_ATTR_STARTUP_DELAY_RAW, + ADI_TDD_ATTR_STARTUP_DELAY_MS, + ADI_TDD_ATTR_FRAME_LENGTH_RAW, + ADI_TDD_ATTR_FRAME_LENGTH_MS, + ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW, + ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS, + + ADI_TDD_ATTR_STATE, + + ADI_TDD_ATTR_CHANNEL_ENABLE, + ADI_TDD_ATTR_CHANNEL_POLARITY, + ADI_TDD_ATTR_CHANNEL_ON_RAW, + ADI_TDD_ATTR_CHANNEL_OFF_RAW, + ADI_TDD_ATTR_CHANNEL_ON_MS, + ADI_TDD_ATTR_CHANNEL_OFF_MS, +}; + +struct adi_axi_tdd_clk { + struct notifier_block nb; + unsigned long rate; + struct clk *clk; + +}; + +struct adi_axi_tdd_attribute { + enum adi_axi_tdd_attribute_id id; + struct device_attribute attr; + u32 channel; + u8 name[32]; +}; + +#define to_tdd_attribute(x) container_of(x, struct adi_axi_tdd_attribute, attr) + +/** + * struct adi_axi_tdd_state - Driver state information for the TDD CORE + * @clk: Interface clock definition. Used to translate ms into cycle counts + * @base: Device register base address in memory + * @regs: Device memory-mapped region regmap + * @sync_count_width: Bit width of the internal synchronization counter, <= 64 + * @sync_count_mask: Bit mask of sync counter + * @burst_count_width: Bit width of the burst counter, <= 32 + * @burst_count_mask: Bit mask of the burst counter + * @reg_width: Timing register bit width, <= 32 + * @reg_mask: Timing register bit mask + * @sync_external_cdc: Whether the external sync input is synchronized into the main clock domain + * @sync_external: Whether external synchronization support was enabled at synthesis time + * @sync_internal: Whether internal synchronization support was enabled at synthesis time + * @channel_count: Available channel count + * @enabled: Whether the TDD engine is currently enabled. + * Note: Most configuration registers cannot be changed while the TDD core is enabled. + */ +struct adi_axi_tdd_state { + struct adi_axi_tdd_clk clk; + void __iomem *base; + struct regmap *regs; + u32 sync_count_width; + u64 sync_count_mask; + u32 burst_count_width; + u32 burst_count_mask; + u32 reg_width; + u32 reg_mask; + bool sync_external_cdc; + bool sync_external; + bool sync_internal; + u32 channel_count; + bool enabled; +}; + +static const struct regmap_config adi_axi_tdd_regmap_cfg = { + .name = "adi-axi-tdd", + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, +}; + +static int adi_axi_tdd_format_ms(struct adi_axi_tdd_state *st, u64 x, char *buf) +{ + u32 vals[2]; + u64 t_ns; + + t_ns = div_u64(x * 1000000000ULL, READ_ONCE(st->clk.rate)); + vals[0] = div_u64_rem(t_ns, 1000000, &vals[1]); + + return sysfs_emit(buf, "%d.%06u\n", vals[0], vals[1]); +} + +static ssize_t adi_axi_tdd_show(struct device *dev, + struct device_attribute *dev_attr, char *buf) +{ + const struct adi_axi_tdd_attribute *attr = to_tdd_attribute(dev_attr); + struct adi_axi_tdd_state *st = dev_get_drvdata(dev); + u32 channel = attr->channel; + bool ms = false; + u64 data64; + u32 data; + int ret; + + switch (attr->id) { + case ADI_TDD_ATTR_VERSION: + ret = regmap_read(st->regs, ADI_AXI_REG_VERSION, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d.%.2d.%c\n", + ADI_AXI_PCORE_VER_MAJOR(data), + ADI_AXI_PCORE_VER_MINOR(data), + ADI_AXI_PCORE_VER_PATCH(data)); + case ADI_TDD_ATTR_CORE_ID: + ret = regmap_read(st->regs, ADI_REG_TDD_PERIPHERAL_ID, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_SCRATCH: + ret = regmap_read(st->regs, ADI_REG_TDD_SCRATCH, &data); + if (ret) + return ret; + return sysfs_emit(buf, "0x%08x\n", data); + case ADI_TDD_ATTR_MAGIC: + ret = regmap_read(st->regs, ADI_REG_TDD_IDENTIFICATION, &data); + if (ret) + return ret; + return sysfs_emit(buf, "0x%08x\n", data); + case ADI_TDD_ATTR_SYNC_EXT: + ret = regmap_read(st->regs, ADI_REG_TDD_CONTROL, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", !!(data & ADI_TDD_SYNC_EXT)); + case ADI_TDD_ATTR_SYNC_INT: + ret = regmap_read(st->regs, ADI_REG_TDD_CONTROL, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", !!(data & ADI_TDD_SYNC_INT)); + case ADI_TDD_ATTR_SYNC_RST: + ret = regmap_read(st->regs, ADI_REG_TDD_CONTROL, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", !!(data & ADI_TDD_SYNC_RST)); + case ADI_TDD_ATTR_ENABLE: + ret = regmap_read(st->regs, ADI_REG_TDD_CONTROL, &data); + if (ret) + return ret; + st->enabled = !!(data & ADI_TDD_ENABLE); + return sysfs_emit(buf, "%d\n", st->enabled); + case ADI_TDD_ATTR_BURST_COUNT: + ret = regmap_read(st->regs, ADI_REG_TDD_BURST_COUNT, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_STARTUP_DELAY_RAW: + ret = regmap_read(st->regs, ADI_REG_TDD_STARTUP_DELAY, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_STARTUP_DELAY_MS: + ret = regmap_read(st->regs, ADI_REG_TDD_STARTUP_DELAY, &data); + if (ret) + return ret; + return adi_axi_tdd_format_ms(st, data, buf); + case ADI_TDD_ATTR_FRAME_LENGTH_RAW: + ret = regmap_read(st->regs, ADI_REG_TDD_FRAME_LENGTH, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_FRAME_LENGTH_MS: + ret = regmap_read(st->regs, ADI_REG_TDD_FRAME_LENGTH, &data); + if (ret) + return ret; + return adi_axi_tdd_format_ms(st, data, buf); + case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW: + ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW, + &data64, 2); + if (ret) + return ret; + return sysfs_emit(buf, "%llu\n", data64); + case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS: + ret = regmap_bulk_read(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW, + &data64, 2); + if (ret) + return ret; + return adi_axi_tdd_format_ms(st, data64, buf); + case ADI_TDD_ATTR_STATE: + ret = regmap_read(st->regs, ADI_REG_TDD_STATUS, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_CHANNEL_ENABLE: + ret = regmap_read(st->regs, ADI_REG_TDD_CHANNEL_ENABLE, &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", !!(BIT(channel) & data)); + case ADI_TDD_ATTR_CHANNEL_POLARITY: + ret = regmap_read(st->regs, ADI_REG_TDD_CHANNEL_POLARITY, + &data); + if (ret) + return ret; + return sysfs_emit(buf, "%d\n", !!(BIT(channel) & data)); + case ADI_TDD_ATTR_CHANNEL_ON_MS: + ms = true; + fallthrough; + case ADI_TDD_ATTR_CHANNEL_ON_RAW: + ret = regmap_read(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_ON), + &data); + if (ret) + return ret; + if (ms) + return adi_axi_tdd_format_ms(st, data, buf); + return sysfs_emit(buf, "%u\n", data); + case ADI_TDD_ATTR_CHANNEL_OFF_MS: + ms = true; + fallthrough; + case ADI_TDD_ATTR_CHANNEL_OFF_RAW: + ret = regmap_read(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_OFF), + &data); + if (ret) + return ret; + if (ms) + return adi_axi_tdd_format_ms(st, data, buf); + return sysfs_emit(buf, "%u\n", data); + default: + return -EINVAL; + } +} + +static int adi_axi_tdd_parse_ms(struct adi_axi_tdd_state *st, + const char *buf, + u64 *res) +{ + u64 clk_rate = READ_ONCE(st->clk.rate); + char *orig_str, *modf_str, *int_part, frac_part[7]; + long ival, frac; + int ret; + + orig_str = kstrdup(buf, GFP_KERNEL); + int_part = strsep(&orig_str, "."); + ret = kstrtol(int_part, 10, &ival); + if (ret || ival < 0) + return -EINVAL; + modf_str = strsep(&orig_str, "."); + if (modf_str) { + snprintf(frac_part, 7, "%s00000", modf_str); + ret = kstrtol(frac_part, 10, &frac); + if (ret) + return -EINVAL; + } else { + frac = 0; + } + + *res = DIV_ROUND_CLOSEST_ULL((u64)ival * clk_rate, 1000) + + DIV_ROUND_CLOSEST_ULL((u64)frac * clk_rate, 1000000000); + + kfree(orig_str); + + return ret; +} + +static int adi_axi_tdd_write_regs(const struct adi_axi_tdd_attribute *attr, + struct adi_axi_tdd_state *st, + const char *buf) +{ + u32 channel = attr->channel; + u64 data64; + u32 data; + int ret; + + switch (attr->id) { + case ADI_TDD_ATTR_SCRATCH: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, ADI_REG_TDD_SCRATCH, data); + case ADI_TDD_ATTR_SYNC_SOFT: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, ADI_REG_TDD_CONTROL, + ADI_TDD_SYNC_SOFT, + ADI_TDD_SYNC_SOFT * !!data, + NULL, false, false); + case ADI_TDD_ATTR_SYNC_EXT: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, ADI_REG_TDD_CONTROL, + ADI_TDD_SYNC_EXT, + ADI_TDD_SYNC_EXT * !!data, + NULL, false, false); + case ADI_TDD_ATTR_SYNC_INT: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, ADI_REG_TDD_CONTROL, + ADI_TDD_SYNC_INT, + ADI_TDD_SYNC_INT * !!data, + NULL, false, false); + case ADI_TDD_ATTR_SYNC_RST: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, ADI_REG_TDD_CONTROL, + ADI_TDD_SYNC_RST, + ADI_TDD_SYNC_RST * !!data, + NULL, false, false); + case ADI_TDD_ATTR_ENABLE: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, ADI_REG_TDD_CONTROL, + ADI_TDD_ENABLE, + ADI_TDD_ENABLE * !!data, + NULL, false, false); + case ADI_TDD_ATTR_BURST_COUNT: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, ADI_REG_TDD_BURST_COUNT, data); + case ADI_TDD_ATTR_STARTUP_DELAY_RAW: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, ADI_REG_TDD_STARTUP_DELAY, data); + case ADI_TDD_ATTR_STARTUP_DELAY_MS: + ret = adi_axi_tdd_parse_ms(st, buf, &data64); + if (ret) + return ret; + if (FIELD_GET(GENMASK_ULL(63, 32), data64)) + return -EINVAL; + return regmap_write(st->regs, ADI_REG_TDD_STARTUP_DELAY, + (u32)data64); + case ADI_TDD_ATTR_FRAME_LENGTH_RAW: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, ADI_REG_TDD_FRAME_LENGTH, data); + case ADI_TDD_ATTR_FRAME_LENGTH_MS: + ret = adi_axi_tdd_parse_ms(st, buf, &data64); + if (ret) + return ret; + if (FIELD_GET(GENMASK_ULL(63, 32), data64)) + return -EINVAL; + return regmap_write(st->regs, ADI_REG_TDD_FRAME_LENGTH, + (u32)data64); + case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW: + ret = kstrtou64(buf, 0, &data64); + if (ret) + return ret; + return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW, + &data64, 2); + case ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS: + ret = adi_axi_tdd_parse_ms(st, buf, &data64); + if (ret) + return ret; + return regmap_bulk_write(st->regs, ADI_REG_TDD_SYNC_COUNTER_LOW, + &data64, 2); + case ADI_TDD_ATTR_CHANNEL_ENABLE: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, + ADI_REG_TDD_CHANNEL_ENABLE, + BIT(channel), + BIT(channel) * !!data, + NULL, false, false); + case ADI_TDD_ATTR_CHANNEL_POLARITY: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_update_bits_base(st->regs, + ADI_REG_TDD_CHANNEL_POLARITY, + BIT(channel), + BIT(channel) * !!data, + NULL, false, false); + case ADI_TDD_ATTR_CHANNEL_ON_RAW: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_ON), + data); + case ADI_TDD_ATTR_CHANNEL_ON_MS: + ret = adi_axi_tdd_parse_ms(st, buf, &data64); + if (ret) + return ret; + if (FIELD_GET(GENMASK_ULL(63, 32), data64)) + return -EINVAL; + return regmap_write(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_ON), + (u32)data64); + case ADI_TDD_ATTR_CHANNEL_OFF_RAW: + ret = kstrtou32(buf, 0, &data); + if (ret) + return ret; + return regmap_write(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_OFF), + data); + case ADI_TDD_ATTR_CHANNEL_OFF_MS: + ret = adi_axi_tdd_parse_ms(st, buf, &data64); + if (ret) + return ret; + if (FIELD_GET(GENMASK_ULL(63, 32), data64)) + return -EINVAL; + return regmap_write(st->regs, + ADI_REG_TDD_CHANNEL(channel, + ADI_TDD_CHANNEL_OFF), + (u32)data64); + default: + return -EINVAL; + } +} + +static ssize_t adi_axi_tdd_store(struct device *dev, + struct device_attribute *dev_attr, + const char *buf, size_t count) +{ + const struct adi_axi_tdd_attribute *attr = to_tdd_attribute(dev_attr); + struct adi_axi_tdd_state *st = dev_get_drvdata(dev); + + return adi_axi_tdd_write_regs(attr, st, buf) ?: count; +} + +static int adi_axi_tdd_init_synthesis_parameters(struct adi_axi_tdd_state *st) +{ + u32 interface_config; + int ret; + + ret = regmap_read(st->regs, ADI_REG_TDD_INTERFACE_DESCRIPTION, + &interface_config); + if (ret) + return ret; + + st->sync_count_width = FIELD_GET(ADI_TDD_SYNC_COUNT_WIDTH, + interface_config); + st->burst_count_width = FIELD_GET(ADI_TDD_BURST_COUNT_WIDTH, + interface_config); + st->reg_width = FIELD_GET(ADI_TDD_REG_WIDTH, interface_config); + st->sync_external_cdc = ADI_TDD_SYNC_EXTERNAL_CDC & interface_config; + st->sync_external = ADI_TDD_SYNC_EXTERNAL & interface_config; + st->sync_internal = ADI_TDD_SYNC_INTERNAL & interface_config; + st->channel_count = FIELD_GET(ADI_TDD_CHANNEL_COUNT, + interface_config) + 1; + + if (!st->burst_count_width || !st->reg_width) + return -EINVAL; + + st->sync_count_mask = GENMASK_ULL(st->sync_count_width - 1, 0); + st->burst_count_mask = GENMASK_ULL(st->burst_count_width - 1, 0); + st->reg_mask = GENMASK_ULL(st->reg_width - 1, 0); + + return ret; +} + +#define __TDD_ATTR(_name, _id, _channel, _mode) \ + { \ + .attr = __ATTR(_name, _mode, adi_axi_tdd_show, \ + adi_axi_tdd_store), \ + .id = _id, \ + .channel = _channel \ + } + +#define TDD_ATTR(_name, _id, _mode) \ + struct adi_axi_tdd_attribute dev_attr_##_name = \ + __TDD_ATTR(_name, _id, 0, _mode) \ + +static const TDD_ATTR(version, ADI_TDD_ATTR_VERSION, 0444); +static const TDD_ATTR(core_id, ADI_TDD_ATTR_CORE_ID, 0444); +static const TDD_ATTR(scratch, ADI_TDD_ATTR_SCRATCH, 0644); +static const TDD_ATTR(magic, ADI_TDD_ATTR_MAGIC, 0444); + +static const TDD_ATTR(sync_soft, ADI_TDD_ATTR_SYNC_SOFT, 0200); +static const TDD_ATTR(sync_external, ADI_TDD_ATTR_SYNC_EXT, 0644); +static const TDD_ATTR(sync_internal, ADI_TDD_ATTR_SYNC_INT, 0644); +static const TDD_ATTR(sync_reset, ADI_TDD_ATTR_SYNC_RST, 0644); +static const TDD_ATTR(enable, ADI_TDD_ATTR_ENABLE, 0644); + +static const TDD_ATTR(burst_count, ADI_TDD_ATTR_BURST_COUNT, 0644); +static const TDD_ATTR(startup_delay_raw, ADI_TDD_ATTR_STARTUP_DELAY_RAW, 0644); +static const TDD_ATTR(startup_delay_ms, ADI_TDD_ATTR_STARTUP_DELAY_MS, 0644); +static const TDD_ATTR(frame_length_raw, ADI_TDD_ATTR_FRAME_LENGTH_RAW, 0644); +static const TDD_ATTR(frame_length_ms, ADI_TDD_ATTR_FRAME_LENGTH_MS, 0644); +static const TDD_ATTR(internal_sync_period_raw, + ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_RAW, 0644); +static const TDD_ATTR(internal_sync_period_ms, + ADI_TDD_ATTR_INTERNAL_SYNC_PERIOD_MS, 0644); + +static const TDD_ATTR(state, ADI_TDD_ATTR_STATE, 0444); + +static const struct attribute *adi_axi_tdd_base_attributes[] = { + &dev_attr_version.attr.attr, + &dev_attr_core_id.attr.attr, + &dev_attr_scratch.attr.attr, + &dev_attr_magic.attr.attr, + &dev_attr_sync_soft.attr.attr, + &dev_attr_sync_external.attr.attr, + &dev_attr_sync_internal.attr.attr, + &dev_attr_sync_reset.attr.attr, + &dev_attr_enable.attr.attr, + &dev_attr_burst_count.attr.attr, + &dev_attr_startup_delay_raw.attr.attr, + &dev_attr_startup_delay_ms.attr.attr, + &dev_attr_frame_length_raw.attr.attr, + &dev_attr_frame_length_ms.attr.attr, + &dev_attr_internal_sync_period_raw.attr.attr, + &dev_attr_internal_sync_period_ms.attr.attr, + &dev_attr_state.attr.attr, + /* NOT TERMINATED */ +}; + +static const char * const channel_names[] = { + "out_channel%u_enable", "out_channel%u_polarity", + "out_channel%u_on_raw", "out_channel%u_off_raw", + "out_channel%u_on_ms", "out_channel%u_off_ms" +}; + +static const enum adi_axi_tdd_attribute_id channel_ids[] = { + ADI_TDD_ATTR_CHANNEL_ENABLE, ADI_TDD_ATTR_CHANNEL_POLARITY, + ADI_TDD_ATTR_CHANNEL_ON_RAW, ADI_TDD_ATTR_CHANNEL_OFF_RAW, + ADI_TDD_ATTR_CHANNEL_ON_MS, ADI_TDD_ATTR_CHANNEL_OFF_MS +}; + +static int adi_axi_tdd_init_sysfs(struct platform_device *pdev, + struct adi_axi_tdd_state *st) +{ + size_t base_attr_count = ARRAY_SIZE(adi_axi_tdd_base_attributes); + size_t attribute_count = base_attr_count + 6 * st->channel_count + 1; + struct adi_axi_tdd_attribute *channel_attributes; + struct adi_axi_tdd_attribute *channel_iter; + struct attribute_group *attr_group; + struct attribute **tdd_attrs; + u32 i, j; + + channel_attributes = devm_kcalloc(&pdev->dev, 6 * st->channel_count, + sizeof(*channel_attributes), + GFP_KERNEL); + if (!channel_attributes) + return -ENOMEM; + + tdd_attrs = devm_kcalloc(&pdev->dev, attribute_count, + sizeof(*tdd_attrs), GFP_KERNEL); + if (!tdd_attrs) + return -ENOMEM; + + memcpy(tdd_attrs, adi_axi_tdd_base_attributes, + sizeof(adi_axi_tdd_base_attributes)); + + channel_iter = channel_attributes; + + for (i = 0; i < st->channel_count; i++) { + for (j = 0; j < ARRAY_SIZE(channel_names); j++) { + snprintf(channel_iter->name, + sizeof(channel_iter->name), channel_names[j], + i); + channel_iter->id = channel_ids[j]; + channel_iter->channel = i; + channel_iter->attr.attr.name = channel_iter->name; + channel_iter->attr.attr.mode = 0644; + channel_iter->attr.show = adi_axi_tdd_show; + channel_iter->attr.store = adi_axi_tdd_store; + + tdd_attrs[base_attr_count + 6 * i + j] = + &channel_iter->attr.attr; + channel_iter++; + } + } + + attr_group = devm_kzalloc(&pdev->dev, sizeof(attr_group), GFP_KERNEL); + if (!attr_group) + return -ENOMEM; + + attr_group->attrs = tdd_attrs; + + return devm_device_add_group(&pdev->dev, attr_group); +} + +static int adi_axi_tdd_rate_change(struct notifier_block *nb, + unsigned long flags, void *data) +{ + struct adi_axi_tdd_clk *clk = + container_of(nb, struct adi_axi_tdd_clk, nb); + struct clk_notifier_data *cnd = data; + + /* cache the new rate */ + WRITE_ONCE(clk->rate, cnd->new_rate); + + return NOTIFY_OK; +} + +static void adi_axi_tdd_clk_notifier_unreg(void *data) +{ + struct adi_axi_tdd_clk *clk = data; + + clk_notifier_unregister(clk->clk, &clk->nb); +} + +static int adi_axi_tdd_probe(struct platform_device *pdev) +{ + unsigned int expected_version, version, data; + struct adi_axi_tdd_state *st; + struct clk *aclk; + int ret; + + st = devm_kzalloc(&pdev->dev, sizeof(*st), GFP_KERNEL); + if (!st) + return -ENOMEM; + + st->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(st->base)) + return PTR_ERR(st->base); + + platform_set_drvdata(pdev, st); + + aclk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk"); + if (IS_ERR(aclk)) + return PTR_ERR(aclk); + + st->clk.clk = devm_clk_get_enabled(&pdev->dev, "intf_clk"); + if (IS_ERR(st->clk.clk)) + return PTR_ERR(st->clk.clk); + + st->clk.rate = clk_get_rate(st->clk.clk); + st->clk.nb.notifier_call = adi_axi_tdd_rate_change; + ret = clk_notifier_register(st->clk.clk, &st->clk.nb); + if (ret) + return ret; + + ret = devm_add_action_or_reset(&pdev->dev, + adi_axi_tdd_clk_notifier_unreg, + st->clk.clk); + if (ret) + return ret; + + st->regs = devm_regmap_init_mmio(&pdev->dev, st->base, + &adi_axi_tdd_regmap_cfg); + if (IS_ERR(st->regs)) + return PTR_ERR(st->regs); + + ret = regmap_read(st->regs, ADI_AXI_REG_VERSION, &version); + if (ret) + return ret; + + expected_version = ADI_AXI_PCORE_VER(2, 0, 'a'); + + if (ADI_AXI_PCORE_VER_MAJOR(version) != + ADI_AXI_PCORE_VER_MAJOR(expected_version)) + return dev_err_probe(&pdev->dev, + -ENODEV, + "Major version mismatch between PCORE and driver. Driver expected %d.%.2d.%c, PCORE reported %d.%.2d.%c\n", + ADI_AXI_PCORE_VER_MAJOR(expected_version), + ADI_AXI_PCORE_VER_MINOR(expected_version), + ADI_AXI_PCORE_VER_PATCH(expected_version), + ADI_AXI_PCORE_VER_MAJOR(version), + ADI_AXI_PCORE_VER_MINOR(version), + ADI_AXI_PCORE_VER_PATCH(version)); + + ret = adi_axi_tdd_init_synthesis_parameters(st); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "Failed to load synthesis parameters, make sure the device is configured correctly.\n"); + + ret = regmap_read(st->regs, ADI_REG_TDD_CONTROL, &data); + if (ret) + return ret; + + st->enabled = data & ADI_TDD_ENABLE; + + ret = adi_axi_tdd_init_sysfs(pdev, st); + if (ret) + return dev_err_probe(&pdev->dev, ret, "Failed to init sysfs, aborting ...\n"); + + dev_dbg(&pdev->dev, "Probed Analog Devices AXI TDD (%d.%.2d.%c)", + ADI_AXI_PCORE_VER_MAJOR(version), + ADI_AXI_PCORE_VER_MINOR(version), + ADI_AXI_PCORE_VER_PATCH(version)); + + return 0; +} + +/* Match table for of_platform binding */ +static const struct of_device_id adi_axi_tdd_of_match[] = { + { .compatible = "adi,axi-tdd-2.00.a" }, + { } +}; +MODULE_DEVICE_TABLE(of, adi_axi_tdd_of_match); + +static struct platform_driver adi_axi_tdd_driver = { + .driver = { + .name = "adi-axi-tdd", + .of_match_table = adi_axi_tdd_of_match, + }, + .probe = adi_axi_tdd_probe, +}; +module_platform_driver(adi_axi_tdd_driver); + +MODULE_AUTHOR("Eliza Balas <eliza.balas@analog.com>"); +MODULE_AUTHOR("David Winter <david.winter@analog.com>"); +MODULE_DESCRIPTION("Analog Devices TDD HDL CORE driver"); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 9:28 ` [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add " Eliza Balas @ 2023-09-28 10:07 ` Arnd Bergmann 2023-09-28 10:54 ` Balas, Eliza 2023-09-28 11:52 ` Nuno Sá 2023-09-28 12:23 ` Greg Kroah-Hartman 1 sibling, 2 replies; 20+ messages in thread From: Arnd Bergmann @ 2023-09-28 10:07 UTC (permalink / raw) To: Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel, devicetree On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > This patch introduces the driver for the new ADI TDD engine HDL. > 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 <eliza.balas@analog.com> Thanks for your submission, I've had a first look at the driver and the implementation of the interface you have chosen looks all good to me, so I have no detailed comments on that. It would however help to explain the ideas you had for the user-space interface design and summarize them in the changelog text. You have chosen a low-level interface that wraps the individual device registers and gives user space direct control over them. The risk here is to lock yourself into the first design, giving you less flexibility for future extensions, so it would help to understand what the usage model is here. One risk is that there may be an in-kernel user in the future when the TDD engine interacts with another device, so you need a driver level interface, which would in turn break if any user pokes the registers directly. Another possible problem I see is that an application written for this driver would be incompatible with similar hardware that has the same functionality but a different register-level interface, or even a minor revision of the device that ends up breaking one of the assumptions about the hardware design. In both cases, the likely answer is to have a higher-level interface of some sort, but the downside of that would be that it is much harder to come up with a good interface that covers all possible use cases. Another question is whether you could fit into some existing subsystem instead of creating a single-driver interface. drivers/iio/ might be a good choice, as it already handles both in-kernel and userspace users, and provides a common abstraction for multiple classes of devices that (without any domain knowledge in my case) look similar enough that this could be added there. Arnd ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 10:07 ` Arnd Bergmann @ 2023-09-28 10:54 ` Balas, Eliza 2023-09-28 12:30 ` Nuno Sá 2023-09-28 14:19 ` Arnd Bergmann 2023-09-28 11:52 ` Nuno Sá 1 sibling, 2 replies; 20+ messages in thread From: Balas, Eliza @ 2023-09-28 10:54 UTC (permalink / raw) To: Arnd Bergmann Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org > -----Original Message----- > From: Arnd Bergmann <arnd@arndb.de> > Sent: Thursday, September 28, 2023 13:07 > To: Balas, Eliza <Eliza.Balas@analog.com> > Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > [External] > > On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > > This patch introduces the driver for the new ADI TDD engine HDL. > > 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 <eliza.balas@analog.com> > > Thanks for your submission, I've had a first look at the driver > and the implementation of the interface you have chosen looks > all good to me, so I have no detailed comments on that. > > It would however help to explain the ideas you had for the > user-space interface design and summarize them in the changelog > text. > > You have chosen a low-level interface that wraps the individual > device registers and gives user space direct control over them. > The risk here is to lock yourself into the first design, > giving you less flexibility for future extensions, so it would > help to understand what the usage model is here. > > One risk is that there may be an in-kernel user in the future > when the TDD engine interacts with another device, so you > need a driver level interface, which would in turn break > if any user pokes the registers directly. > > Another possible problem I see is that an application written > for this driver would be incompatible with similar hardware > that has the same functionality but a different register-level > interface, or even a minor revision of the device that ends up > breaking one of the assumptions about the hardware design. > > In both cases, the likely answer is to have a higher-level > interface of some sort, but the downside of that would be > that it is much harder to come up with a good interface that > covers all possible use cases. > > Another question is whether you could fit into some > existing subsystem instead of creating a single-driver > interface. drivers/iio/ might be a good choice, as > it already handles both in-kernel and userspace users, > and provides a common abstraction for multiple classes > of devices that (without any domain knowledge in my case) > look similar enough that this could be added there. > > Arnd Hello Arnd, We are using this driver with an iio-fake device https://github.com/analogdevicesinc/linux/blob/master/Documentation/devicetree/bindings/iio/jesd204/adi%2Ciio-fakedev.yaml so we can take advantage of the iio user-space interface. We talked in the previous v1 patch emails about adding this driver to an existing subsystem, and I raised the question if we should add it to the iio subsystem, but the driver is not registered into the IIO device tree, and does not rely on IIO kernel APIs, so I concluded that misc is a better choice. What do you think? Thank you, Eliza ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 10:54 ` Balas, Eliza @ 2023-09-28 12:30 ` Nuno Sá 2023-09-28 14:19 ` Arnd Bergmann 1 sibling, 0 replies; 20+ messages in thread From: Nuno Sá @ 2023-09-28 12:30 UTC (permalink / raw) To: Balas, Eliza, Arnd Bergmann Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org On Thu, 2023-09-28 at 10:54 +0000, Balas, Eliza wrote: > > > > -----Original Message----- > > From: Arnd Bergmann <arnd@arndb.de> > > Sent: Thursday, September 28, 2023 13:07 > > To: Balas, Eliza <Eliza.Balas@analog.com> > > Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > > <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg Kroah- > > Hartman <gregkh@linuxfoundation.org>; > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > > > [External] > > > > On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > > > This patch introduces the driver for the new ADI TDD engine HDL. > > > 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 <eliza.balas@analog.com> > > > > Thanks for your submission, I've had a first look at the driver > > and the implementation of the interface you have chosen looks > > all good to me, so I have no detailed comments on that. > > > > It would however help to explain the ideas you had for the > > user-space interface design and summarize them in the changelog > > text. > > > > You have chosen a low-level interface that wraps the individual > > device registers and gives user space direct control over them. > > The risk here is to lock yourself into the first design, > > giving you less flexibility for future extensions, so it would > > help to understand what the usage model is here. > > > > One risk is that there may be an in-kernel user in the future > > when the TDD engine interacts with another device, so you > > need a driver level interface, which would in turn break > > if any user pokes the registers directly. > > > > Another possible problem I see is that an application written > > for this driver would be incompatible with similar hardware > > that has the same functionality but a different register-level > > interface, or even a minor revision of the device that ends up > > breaking one of the assumptions about the hardware design. > > > > In both cases, the likely answer is to have a higher-level > > interface of some sort, but the downside of that would be > > that it is much harder to come up with a good interface that > > covers all possible use cases. > > > > Another question is whether you could fit into some > > existing subsystem instead of creating a single-driver > > interface. drivers/iio/ might be a good choice, as > > it already handles both in-kernel and userspace users, > > and provides a common abstraction for multiple classes > > of devices that (without any domain knowledge in my case) > > look similar enough that this could be added there. > > > > Arnd > > Hello Arnd, > > We are using this driver with an iio-fake device > https://github.com/analogdevicesinc/linux/blob/master/Documentation/devicetree/bindings/iio/jesd204/adi%2Ciio-fakedev.yaml > so we can take advantage of the iio user-space interface. > Ehehehh, iio-fake device is an hack we have to make use of libiio capability to access IIO devices trough USB, network and serial. I might be wrong, but I think that's pretty much the reason for that driver. In the past we used to treat non IIO devices as IIO solely for that reason so it's better do it just once (with that fake device) - Nuno Sá ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 10:54 ` Balas, Eliza 2023-09-28 12:30 ` Nuno Sá @ 2023-09-28 14:19 ` Arnd Bergmann 2023-09-28 16:35 ` Nuno Sá 1 sibling, 1 reply; 20+ messages in thread From: Arnd Bergmann @ 2023-09-28 14:19 UTC (permalink / raw) To: Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org On Thu, Sep 28, 2023, at 06:54, Balas, Eliza wrote: >> <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; >> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org >> Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine >> >> [External] >> >> On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: >> > This patch introduces the driver for the new ADI TDD engine HDL. >> > 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 <eliza.balas@analog.com> >> >> Thanks for your submission, I've had a first look at the driver >> and the implementation of the interface you have chosen looks >> all good to me, so I have no detailed comments on that. >> >> It would however help to explain the ideas you had for the >> user-space interface design and summarize them in the changelog >> text. >> >> You have chosen a low-level interface that wraps the individual >> device registers and gives user space direct control over them. >> The risk here is to lock yourself into the first design, >> giving you less flexibility for future extensions, so it would >> help to understand what the usage model is here. >> >> One risk is that there may be an in-kernel user in the future >> when the TDD engine interacts with another device, so you >> need a driver level interface, which would in turn break >> if any user pokes the registers directly. >> >> Another possible problem I see is that an application written >> for this driver would be incompatible with similar hardware >> that has the same functionality but a different register-level >> interface, or even a minor revision of the device that ends up >> breaking one of the assumptions about the hardware design. >> >> In both cases, the likely answer is to have a higher-level >> interface of some sort, but the downside of that would be >> that it is much harder to come up with a good interface that >> covers all possible use cases. >> >> Another question is whether you could fit into some >> existing subsystem instead of creating a single-driver >> interface. drivers/iio/ might be a good choice, as >> it already handles both in-kernel and userspace users, >> and provides a common abstraction for multiple classes >> of devices that (without any domain knowledge in my case) >> look similar enough that this could be added there. >> > > We are using this driver with an iio-fake device > https://github.com/analogdevicesinc/linux/blob/master/Documentation/devicetree/bindings/iio/jesd204/adi%2Ciio-fakedev.yaml > so we can take advantage of the iio user-space interface. I don't understand how that works yet: Do you mean that there is user-space application that uses the tdd sysfs interface to export an IIO device back into the kernel, or do you mean there is a regular IIO device in with a kernel driver that is used as the back-end for the tdd device, or something else? > We talked in the previous v1 patch emails about adding this driver to > an existing subsystem, and I raised the question if we should add it to > the iio subsystem, but the driver is not registered into the IIO device > tree, and does not rely on IIO kernel APIs, so I concluded that misc is > a better choice. > What do you think? My feeling is that if you can make it fit into IIO, then this is likely the better choice, unless you can guarantee that this is a one-off driver with a single hardware implementation and a single userspace. If you need the flexibility later to do more things, the risk is that you end up duplicating a lot of functionality that already exists in IIO. This would of course mean using the interfaces provided by the IIO core, with the addition of a tdd device type rather than just having a standalone driver with just the sysfs interface you have here. Arnd ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 14:19 ` Arnd Bergmann @ 2023-09-28 16:35 ` Nuno Sá 2023-10-02 16:02 ` Balas, Eliza 0 siblings, 1 reply; 20+ messages in thread From: Nuno Sá @ 2023-09-28 16:35 UTC (permalink / raw) To: Arnd Bergmann, Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org On Thu, 2023-09-28 at 10:19 -0400, Arnd Bergmann wrote: > On Thu, Sep 28, 2023, at 06:54, Balas, Eliza wrote: > > > <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg > > > Kroah-Hartman <gregkh@linuxfoundation.org>; > > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > > > > > [External] > > > > > > On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > > > > This patch introduces the driver for the new ADI TDD engine HDL. > > > > 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 <eliza.balas@analog.com> > > > > > > Thanks for your submission, I've had a first look at the driver > > > and the implementation of the interface you have chosen looks > > > all good to me, so I have no detailed comments on that. > > > > > > It would however help to explain the ideas you had for the > > > user-space interface design and summarize them in the changelog > > > text. > > > > > > You have chosen a low-level interface that wraps the individual > > > device registers and gives user space direct control over them. > > > The risk here is to lock yourself into the first design, > > > giving you less flexibility for future extensions, so it would > > > help to understand what the usage model is here. > > > > > > One risk is that there may be an in-kernel user in the future > > > when the TDD engine interacts with another device, so you > > > need a driver level interface, which would in turn break > > > if any user pokes the registers directly. > > > > > > Another possible problem I see is that an application written > > > for this driver would be incompatible with similar hardware > > > that has the same functionality but a different register-level > > > interface, or even a minor revision of the device that ends up > > > breaking one of the assumptions about the hardware design. > > > > > > In both cases, the likely answer is to have a higher-level > > > interface of some sort, but the downside of that would be > > > that it is much harder to come up with a good interface that > > > covers all possible use cases. > > > > > > Another question is whether you could fit into some > > > existing subsystem instead of creating a single-driver > > > interface. drivers/iio/ might be a good choice, as > > > it already handles both in-kernel and userspace users, > > > and provides a common abstraction for multiple classes > > > of devices that (without any domain knowledge in my case) > > > look similar enough that this could be added there. > > > > > > > We are using this driver with an iio-fake device > > https://github.com/analogdevicesinc/linux/blob/master/Documentation/devicetree/bindings/iio/jesd204/adi%2Ciio-fakedev.yaml > > > > so we can take advantage of the iio user-space interface. > > I don't understand how that works yet: Do you mean that there > is user-space application that uses the tdd sysfs interface to > export an IIO device back into the kernel, or do you mean there > is a regular IIO device in with a kernel driver that is used > as the back-end for the tdd device, or something else? > Well, I never used this myself but the iio-fakedev is an out of tree driver that receives a phandle to a device and a string list of sysfs attributes of that same device. It then symlinks those to an IIO fake device so they seem like IIO device attributes. As a said, this is __very__, __very__ hackish and the solely reason it's being done (I believe), is to use libiio on that fakedev so you can access these kind of devices (like this TDD core) remotely through the network for example (or USB). In the past, we would put drivers that are not IIO in IIO for the same reason. So, at least now, it's a one time ugly hack :sweat_smile: but then we can put drivers in their right places. Not saying this justifies this fakedev but it is what it is :). Well, maybe this one is not really in here but the IIO maintainer was also not to keen to have in there. So I'm not really sure where else it can go. > - Nuno Sá ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 16:35 ` Nuno Sá @ 2023-10-02 16:02 ` Balas, Eliza 0 siblings, 0 replies; 20+ messages in thread From: Balas, Eliza @ 2023-10-02 16:02 UTC (permalink / raw) To: Arnd Bergmann Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Nuno Sá > -----Original Message----- > From: Nuno Sá <noname.nuno@gmail.com> > Sent: Thursday, September 28, 2023 19:35 > To: Arnd Bergmann <arnd@arndb.de>; Balas, Eliza <Eliza.Balas@analog.com> > Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > [External] > > On Thu, 2023-09-28 at 10:19 -0400, Arnd Bergmann wrote: > > On Thu, Sep 28, 2023, at 06:54, Balas, Eliza wrote: > > > > <conor+dt@kernel.org>; derek.kiernan@amd.com; dragan.cvetic@amd.com; Greg > > > > Kroah-Hartman <gregkh@linuxfoundation.org>; > > > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > > > > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > > > > > > > [External] > > > > > > > > On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > > > > > This patch introduces the driver for the new ADI TDD engine HDL. > > > > > 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 <eliza.balas@analog.com> > > > > > > > > Thanks for your submission, I've had a first look at the driver > > > > and the implementation of the interface you have chosen looks > > > > all good to me, so I have no detailed comments on that. > > > > > > > > It would however help to explain the ideas you had for the > > > > user-space interface design and summarize them in the changelog > > > > text. > > > > > > > > You have chosen a low-level interface that wraps the individual > > > > device registers and gives user space direct control over them. > > > > The risk here is to lock yourself into the first design, > > > > giving you less flexibility for future extensions, so it would > > > > help to understand what the usage model is here. > > > > > > > > One risk is that there may be an in-kernel user in the future > > > > when the TDD engine interacts with another device, so you > > > > need a driver level interface, which would in turn break > > > > if any user pokes the registers directly. > > > > > > > > Another possible problem I see is that an application written > > > > for this driver would be incompatible with similar hardware > > > > that has the same functionality but a different register-level > > > > interface, or even a minor revision of the device that ends up > > > > breaking one of the assumptions about the hardware design. > > > > > > > > In both cases, the likely answer is to have a higher-level > > > > interface of some sort, but the downside of that would be > > > > that it is much harder to come up with a good interface that > > > > covers all possible use cases. > > > > > > > > Another question is whether you could fit into some > > > > existing subsystem instead of creating a single-driver > > > > interface. drivers/iio/ might be a good choice, as > > > > it already handles both in-kernel and userspace users, > > > > and provides a common abstraction for multiple classes > > > > of devices that (without any domain knowledge in my case) > > > > look similar enough that this could be added there. > > > > > > > > > > We are using this driver with an iio-fake device > > > > https://urldefense.com/v3/__https://github.com/analogdevicesinc/linux/blob/master/Documentation/devicetree/bindings/iio/jesd2 > 04/adi*2Ciio-fakedev.yaml__;JQ!!A3Ni8CS0y2Y!8Km0TXYcgKkusJTPoYw6V09_KZFbj09ZfbrUpnuVRZoKKOECMJQ- > 8tGXAYWsjrQ50IbGnsYXzqx8-GjR0BkqRQ$ > > > > > > so we can take advantage of the iio user-space interface. > > > > I don't understand how that works yet: Do you mean that there > > is user-space application that uses the tdd sysfs interface to > > export an IIO device back into the kernel, or do you mean there > > is a regular IIO device in with a kernel driver that is used > > as the back-end for the tdd device, or something else? > > > > Well, I never used this myself but the iio-fakedev is an out of tree driver that > receives a phandle to a device and a string list of sysfs attributes of that same > device. It then symlinks those to an IIO fake device so they seem like IIO device > attributes. As a said, this is __very__, __very__ hackish and the solely reason it's > being done (I believe), is to use libiio on that fakedev so you can access these kind > of devices (like this TDD core) remotely through the network for example (or USB). In > the past, we would put drivers that are not IIO in IIO for the same reason. So, at > least now, it's a one time ugly hack :sweat_smile: but then we can put drivers in > their right places. Not saying this justifies this fakedev but it is what it is :). > > Well, maybe this one is not really in here but the IIO maintainer was also not to > keen to have in there. So I'm not really sure where else it can go. > > > > - Nuno Sá As Nuno explained in the previous email, we are using the iio-fake device as a workaround for using the driver with the libiio API to interface it with some user-space client applications. The device itself contains attributes which are not part of the IIO. We do not want to confuse this device with an IIO device, so I believe that the driver should reside in the misc subsystem. This driver is created for a specific FPGA Core and the intent was to have a low-level interface so that the user can access the registers directly, in an easy way, without using a complex interface. If there will be other future revisions of the TDD FPGA Core, we will keep the register space compatible, so we don't break the current functionality of the driver. I will make the changes related to the dual-license and change it to single license for the driver. If that is okay with you, I will keep the driver in the misc subsystem. I thank you all for taking the time to review this driver and for helping me figure out where this driver fits best. Eliza ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 10:07 ` Arnd Bergmann 2023-09-28 10:54 ` Balas, Eliza @ 2023-09-28 11:52 ` Nuno Sá 1 sibling, 0 replies; 20+ messages in thread From: Nuno Sá @ 2023-09-28 11:52 UTC (permalink / raw) To: Arnd Bergmann, Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, derek.kiernan@amd.com, dragan.cvetic@amd.com, Greg Kroah-Hartman, linux-kernel, devicetree, Jonathan Cameron Hi Arnd, On Thu, 2023-09-28 at 12:07 +0200, Arnd Bergmann wrote: > On Thu, Sep 28, 2023, at 11:28, Eliza Balas wrote: > > This patch introduces the driver for the new ADI TDD engine HDL. > > 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 <eliza.balas@analog.com> > > Thanks for your submission, I've had a first look at the driver > and the implementation of the interface you have chosen looks > all good to me, so I have no detailed comments on that. > > It would however help to explain the ideas you had for the > user-space interface design and summarize them in the changelog > text. > > You have chosen a low-level interface that wraps the individual > device registers and gives user space direct control over them. > The risk here is to lock yourself into the first design, > giving you less flexibility for future extensions, so it would > help to understand what the usage model is here. > > One risk is that there may be an in-kernel user in the future > when the TDD engine interacts with another device, so you > need a driver level interface, which would in turn break > if any user pokes the registers directly. > > Another possible problem I see is that an application written > for this driver would be incompatible with similar hardware > that has the same functionality but a different register-level > interface, or even a minor revision of the device that ends up > breaking one of the assumptions about the hardware design. > > In both cases, the likely answer is to have a higher-level > interface of some sort, but the downside of that would be > that it is much harder to come up with a good interface that > covers all possible use cases. > > Another question is whether you could fit into some > existing subsystem instead of creating a single-driver > interface. drivers/iio/ might be a good choice, as > it already handles both in-kernel and userspace users, > and provides a common abstraction for multiple classes > of devices that (without any domain knowledge in my case) > look similar enough that this could be added there. > Yeah, the same question was done in v1 [1]: For some reason (I guess a simple mistake :)) Jonathan only replied to me: Here is it's response: " > We do have tons of specific attributes (non IIO ones) for this device. The ones > resembling IIO is likely because it feels familiar to us in ADI. Anyways, I have my > doubts this fits (at least as IIO stands right now) but maybe Jonathan thinks > otherwise. From a superficial look, it's a stretch but we have stretched IIO in the past (things like very high frequency synthesizers) In some ways this looks like a complex PWM device though it would probably be hard to force it into that framework. This is weird enough that I'd be surprised to see it fit well anywhere so misc might be the best option. Jonathan > > +cc Jonathan... " That said, I do agree that some of the interface should likely not be part of the "main" ABI. Like anything that states "debugging purposes" in the ABI file should probably go to debugfs. I'm also not sure about those dual "*_ms" vs "*_raw" files. Ideally, we would only provide the non "raw" ones. At least in the beginning... +cc Jonathan again so he can confirm that I'm not putting words in his mouth :) [1]: https://lore.kernel.org/all/b2379aadad95d68ca9605bb9566ce6a70121a409.camel@gmail.com/ - Nuno Sá ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 9:28 ` [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add " Eliza Balas 2023-09-28 10:07 ` Arnd Bergmann @ 2023-09-28 12:23 ` Greg Kroah-Hartman 2023-09-28 14:05 ` Balas, Eliza 1 sibling, 1 reply; 20+ messages in thread From: Greg Kroah-Hartman @ 2023-09-28 12:23 UTC (permalink / raw) To: Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, linux-kernel, devicetree On Thu, Sep 28, 2023 at 12:28:04PM +0300, Eliza Balas wrote: > --- /dev/null > +++ b/drivers/misc/adi-axi-tdd.c > @@ -0,0 +1,780 @@ > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) License comment, why is this dual licensed? You are calling gpl-only-marked functions in this driver so attempting to say it is also BSD is quite odd, how are you going to resolve that? Has a lawyer agreed with this licensing? Please get a lawyer to sign off on your next contribution of this with a dual license to ensure that they agree and that you all fully understand the legal issues and complexity of attempting to have dual-licensed Linux kernel code (hint, it's not as simple as you might think...) And document in the changelog _why_ you want this to be dual licensed so that we can all review that as well. thanks, greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine 2023-09-28 12:23 ` Greg Kroah-Hartman @ 2023-09-28 14:05 ` Balas, Eliza 0 siblings, 0 replies; 20+ messages in thread From: Balas, Eliza @ 2023-09-28 14:05 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org > -----Original Message----- > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Sent: Thursday, September 28, 2023 15:23 > To: Balas, Eliza <Eliza.Balas@analog.com> > Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley > <conor+dt@kernel.org>; Derek Kiernan <derek.kiernan@amd.com>; Dragan Cvetic <dragan.cvetic@amd.com>; Arnd Bergmann > <arnd@arndb.de>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add TDD engine > > [External] > > On Thu, Sep 28, 2023 at 12:28:04PM +0300, Eliza Balas wrote: > > --- /dev/null > > +++ b/drivers/misc/adi-axi-tdd.c > > @@ -0,0 +1,780 @@ > > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) > > License comment, why is this dual licensed? You are calling > gpl-only-marked functions in this driver so attempting to say it is also > BSD is quite odd, how are you going to resolve that? > > Has a lawyer agreed with this licensing? > > Please get a lawyer to sign off on your next contribution of this with a > dual license to ensure that they agree and that you all fully understand > the legal issues and complexity of attempting to have dual-licensed > Linux kernel code (hint, it's not as simple as you might think...) > > And document in the changelog _why_ you want this to be dual licensed so > that we can all review that as well. > > thanks, > > greg k-h Hello Greg, Thank you for the review. I will make the changes to use GPL-2.0 license, and not dual license. Best regards, Eliza ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/2] Add support for ADI TDD Engine 2023-09-28 9:28 [PATCH v2 0/2] Add support for ADI TDD Engine Eliza Balas 2023-09-28 9:28 ` [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine Eliza Balas 2023-09-28 9:28 ` [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add " Eliza Balas @ 2023-09-28 9:38 ` Andreas Herrmann 2 siblings, 0 replies; 20+ messages in thread From: Andreas Herrmann @ 2023-09-28 9:38 UTC (permalink / raw) To: Eliza Balas Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, devicetree Hi, You've used an odd "To:" field in your E-Mail. You might have to fix this in future mails. Have fun. On Thu, Sep 28, 2023 at 12:28:02PM +0300, Eliza Balas wrote: > V1 -> V2: > * remove label in examples in bindings file > * add detailed description of the hardware in bindings file > * remove adi_axi_tdd_clk_disable function > * remove devm_add_action_or_reset, devm_clk_get, clk_prepare_enable > and use instead devm_clk_get_enabled > > Eliza Balas (2): > dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine > drivers: misc: adi-axi-tdd: Add TDD engine > > .../sysfs-bus-platform-drivers-adi-axi-tdd | 158 ++++ > .../devicetree/bindings/misc/adi,axi-tdd.yaml | 65 ++ > MAINTAINERS | 9 + > drivers/misc/Kconfig | 10 + > drivers/misc/Makefile | 1 + > drivers/misc/adi-axi-tdd.c | 780 ++++++++++++++++++ > 6 files changed, 1023 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-platform-drivers-adi-axi-tdd > create mode 100644 Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml > create mode 100644 drivers/misc/adi-axi-tdd.c > > -- > 2.25.1 > -- Regards, Andreas SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nürnberg, Germany GF: Ivo Totev, Andrew McDonald, Werner Knoblich (HRB 36809, AG Nürnberg) ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-10-03 8:54 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-28 9:28 [PATCH v2 0/2] Add support for ADI TDD Engine Eliza Balas 2023-09-28 9:28 ` [PATCH v2 1/2] dt-bindings: misc: adi,axi-tdd: Add device-tree binding for TDD engine Eliza Balas 2023-10-02 16:32 ` Rob Herring 2023-10-02 16:46 ` Balas, Eliza 2023-10-02 19:21 ` Conor Dooley 2023-10-02 19:48 ` Balas, Eliza 2023-10-02 20:07 ` Conor Dooley 2023-10-02 20:23 ` Balas, Eliza 2023-10-03 8:54 ` Krzysztof Kozlowski 2023-09-28 9:28 ` [PATCH v2 2/2] drivers: misc: adi-axi-tdd: Add " Eliza Balas 2023-09-28 10:07 ` Arnd Bergmann 2023-09-28 10:54 ` Balas, Eliza 2023-09-28 12:30 ` Nuno Sá 2023-09-28 14:19 ` Arnd Bergmann 2023-09-28 16:35 ` Nuno Sá 2023-10-02 16:02 ` Balas, Eliza 2023-09-28 11:52 ` Nuno Sá 2023-09-28 12:23 ` Greg Kroah-Hartman 2023-09-28 14:05 ` Balas, Eliza 2023-09-28 9:38 ` [PATCH v2 0/2] Add support for ADI TDD Engine Andreas Herrmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).