All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
@ 2023-07-26  7:11 Eliza Balas
  2023-07-26  7:11 ` [PATCH 2/2] drivers: misc: adi-axi-tdd: " Eliza Balas
  2023-07-26 18:35 ` [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: " Krzysztof Kozlowski
  0 siblings, 2 replies; 9+ messages in thread
From: Eliza Balas @ 2023-07-26  7:11 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 | 51 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml

diff --git a/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
new file mode 100644
index 000000000000..1894c1c34d4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
@@ -0,0 +1,51 @@
+# 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: |
+  Bindings for the new generic TDD CORE, which is part of the Analog Devices hdl reference designs.
+  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: 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 aee340630eca..280e66ccdd56 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1360,6 +1360,13 @@ F:	Documentation/devicetree/bindings/hwmon/adi,max31760.yaml
 F:	Documentation/hwmon/max31760.rst
 F:	drivers/hwmon/max31760.c
 
+ANALOG DEVICES INC GENERIC TDD ENGINE DRIVER
+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
+
 ANALOGBITS PLL LIBRARIES
 M:	Paul Walmsley <paul.walmsley@sifive.com>
 S:	Supported
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drivers: misc: adi-axi-tdd: Add new TDD engine driver
  2023-07-26  7:11 [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver Eliza Balas
@ 2023-07-26  7:11 ` Eliza Balas
  2023-07-26 18:39   ` Krzysztof Kozlowski
  2023-07-26 18:35 ` [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: " Krzysztof Kozlowski
  1 sibling, 1 reply; 9+ messages in thread
From: Eliza Balas @ 2023-07-26  7:11 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                    | 753 ++++++++++++++++++
 5 files changed, 924 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..eb5f3db7d0cb
--- /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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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:           July 2023
+KernelVersion:  6.5
+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 280e66ccdd56..618479069503 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1365,7 +1365,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
 
 ANALOGBITS PLL LIBRARIES
 M:	Paul Walmsley <paul.walmsley@sifive.com>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 75e427f124b2..683d184f4eb7 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 "support Analog Devices TDD"
+	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..8d79b3fd573c
--- /dev/null
+++ b/drivers/misc/adi-axi-tdd.c
@@ -0,0 +1,753 @@
+// 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 void adi_axi_tdd_clk_disable(void *clk)
+{
+	clk_disable_unprepare(clk);
+}
+
+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);
+
+	ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, aclk);
+	if (ret)
+		return ret;
+
+	st->clk.clk = devm_clk_get(&pdev->dev, "intf_clk");
+	if (IS_ERR(st->clk.clk))
+		return PTR_ERR(st->clk.clk);
+
+	ret = clk_prepare_enable(st->clk.clk);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, st->clk.clk);
+	if (ret)
+		return ret;
+
+	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] 9+ messages in thread

* Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
  2023-07-26  7:11 [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver Eliza Balas
  2023-07-26  7:11 ` [PATCH 2/2] drivers: misc: adi-axi-tdd: " Eliza Balas
@ 2023-07-26 18:35 ` Krzysztof Kozlowski
  2023-07-27  9:05   ` Balas, Eliza
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-26 18:35 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

On 26/07/2023 09:11, Eliza Balas wrote:
> Add device tree documentation for the AXI TDD Core.
> The generic TDD controller is in essence a waveform generator
> capable of addressing RF applications which require Time Division
> Duplexing, as well as controlling other modules of general
> applications through its dedicated 32 channel outputs.
> 
> The reason of creating the generic TDD controller was to reduce
> the naming confusion around the existing repurposed TDD core
> built for AD9361, as well as expanding its number of output
> channels for systems which require more than six controlling signals.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

Subject: drop driver. Bindings are for hardware, not drivers... unless
driver is here a hardware term?

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

Why is this in misc? No suitable directory?

> @@ -0,0 +1,51 @@
> +# 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: |
> +  Bindings for the new generic TDD CORE, which is part of the Analog Devices hdl reference designs.

Drop boiler plate, so "Bindings for the new generic". Instead, describe
the hardware.

> +  For more information see the wiki: https://wiki.analog.com/resources/fpga/docs/axi_tdd

Not enough. Describe it more. Here.

Also, wrap according to Linux style, so at 80.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,axi-tdd-2.00.a

Versioned blocks... https://wiki.analog.com/resources/fpga/docs/axi_tdd
says nothing about 2.00.a


> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: System clock
> +      - description: TDD Core clock
> +
> +  clock-names:
> +    items:
> +      - const: s_axi_aclk
> +      - const: intf_clk
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    tdd: tdd@84a00000 {

Drop label, not used.

> +            compatible = "adi,axi-tdd-2.00.a";

Use 4 spaces for example indentation.

> +            reg = <0x84a00000 0x10000>;
> +            clocks = <&zynqmp_clk_PL0_REF>, <&zynqmp_clk_PL1_REF>;
> +            clock-names = "s_axi_aclk", "intf_clk";
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aee340630eca..280e66ccdd56 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1360,6 +1360,13 @@ F:	Documentation/devicetree/bindings/hwmon/adi,max31760.yaml
>  F:	Documentation/hwmon/max31760.rst
>  F:	drivers/hwmon/max31760.c
>  
> +ANALOG DEVICES INC GENERIC TDD ENGINE DRIVER

I am pretty sure G is before M. It's difficult, I know.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c192ac7357683f78c2e6d6e75adfcc29deb8c4ae


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: misc: adi-axi-tdd: Add new TDD engine driver
  2023-07-26  7:11 ` [PATCH 2/2] drivers: misc: adi-axi-tdd: " Eliza Balas
@ 2023-07-26 18:39   ` Krzysztof Kozlowski
  2023-07-27  6:40     ` Nuno Sá
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-26 18:39 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

On 26/07/2023 09:11, 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>
> ---
>  .../sysfs-bus-platform-drivers-adi-axi-tdd    | 158 ++++
>  MAINTAINERS                                   |   2 +
>  drivers/misc/Kconfig                          |  10 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/adi-axi-tdd.c                    | 753 ++++++++++++++++++
>  5 files changed, 924 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..eb5f3db7d0cb
> --- /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:           July 2023
> +KernelVersion:  6.5

We are in 6.5 now, so there is no way your driver will be in 6.5. Target
6.6 and use phb crystall ball for next release date (September).

...

> +
> +enum adi_axi_tdd_attribute_id {
> +	ADI_TDD_ATTR_VERSION,
> +	ADI_TDD_ATTR_CORE_ID,
> +	ADI_TDD_ATTR_SCRATCH,
> +	ADI_TDD_ATTR_MAGIC,
> +

...

> +
> +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);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, aclk);

Looks you have here double disable.

> +	if (ret)
> +		return ret;
> +
> +	st->clk.clk = devm_clk_get(&pdev->dev, "intf_clk");
> +	if (IS_ERR(st->clk.clk))
> +		return PTR_ERR(st->clk.clk);
> +
> +	ret = clk_prepare_enable(st->clk.clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, st->clk.clk);

Looks you have here double disable.


> +	if (ret)
> +		return ret;
> +
> +	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);

Wrap your lines. Limit is 80.

> +	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;
> +



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drivers: misc: adi-axi-tdd: Add new TDD engine driver
  2023-07-26 18:39   ` Krzysztof Kozlowski
@ 2023-07-27  6:40     ` Nuno Sá
  0 siblings, 0 replies; 9+ messages in thread
From: Nuno Sá @ 2023-07-27  6:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Eliza Balas
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	devicetree

On Wed, 2023-07-26 at 20:39 +0200, Krzysztof Kozlowski wrote:
> On 26/07/2023 09:11, 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>
> > ---
> >  .../sysfs-bus-platform-drivers-adi-axi-tdd    | 158 ++++
> >  MAINTAINERS                                   |   2 +
> >  drivers/misc/Kconfig                          |  10 +
> >  drivers/misc/Makefile                         |   1 +
> >  drivers/misc/adi-axi-tdd.c                    | 753 ++++++++++++++++++
> >  5 files changed, 924 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..eb5f3db7d0cb
> > --- /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:           July 2023
> > +KernelVersion:  6.5
> 
> We are in 6.5 now, so there is no way your driver will be in 6.5. Target
> 6.6 and use phb crystall ball for next release date (September).
> 
> ...
> 
> > +
> > +enum adi_axi_tdd_attribute_id {
> > +       ADI_TDD_ATTR_VERSION,
> > +       ADI_TDD_ATTR_CORE_ID,
> > +       ADI_TDD_ATTR_SCRATCH,
> > +       ADI_TDD_ATTR_MAGIC,
> > +
> 
> ...
> 
> > +
> > +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);
> > +
> > +       ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable,
> > aclk);
> 
> Looks you have here double disable.
> 
> > +       if (ret)
> > +               return ret;
> > +
> > +       st->clk.clk = devm_clk_get(&pdev->dev, "intf_clk");
> > +       if (IS_ERR(st->clk.clk))
> > +               return PTR_ERR(st->clk.clk);
> > +
> > +       ret = clk_prepare_enable(st->clk.clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_add_action_or_reset(&pdev->dev, adi_axi_tdd_clk_disable, st-
> > >clk.clk);
> 
> Looks you have here double disable.
> 

Not in here but it should actually drop the action and use devm_clk_get_enabled()

- Nuno Sá



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
  2023-07-26 18:35 ` [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: " Krzysztof Kozlowski
@ 2023-07-27  9:05   ` Balas, Eliza
  2023-07-27  9:26     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 9+ messages in thread
From: Balas, Eliza @ 2023-07-27  9:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	devicetree


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

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

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

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

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

Thank you,
Eliza

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
  2023-07-27  9:05   ` Balas, Eliza
@ 2023-07-27  9:26     ` Krzysztof Kozlowski
  2023-07-27  9:46       ` Balas, Eliza
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-07-27  9:26 UTC (permalink / raw)
  To: Balas, Eliza
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	devicetree

On 27/07/2023 11:05, Balas, Eliza wrote:
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Wednesday, July 26, 2023 21:35
>> 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>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
>> Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
>>
>> [External]
>>
>> On 26/07/2023 09:11, Eliza Balas wrote:
>>> Add device tree documentation for the AXI TDD Core.
>>> The generic TDD controller is in essence a waveform generator capable
>>> of addressing RF applications which require Time Division Duplexing,
>>> as well as controlling other modules of general applications through
>>> its dedicated 32 channel outputs.
>>>
>>> The reason of creating the generic TDD controller was to reduce the
>>> naming confusion around the existing repurposed TDD core built for
>>> AD9361, as well as expanding its number of output channels for systems
>>> which require more than six controlling signals.
>>
>> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on
>> the directory your patch is touching.
>>
>> Subject: drop driver. Bindings are for hardware, not drivers... unless driver is here a hardware term?
> 
> It is not a hardware term in this case, I will make the changes.
> 
>>>
>>> Signed-off-by: Eliza Balas <eliza.balas@analog.com>
>>> ---
>>>  .../devicetree/bindings/misc/adi,axi-tdd.yaml | 51 +++++++++++++++++++
>>>  MAINTAINERS                                   |  7 +++
>>>  2 files changed, 58 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
>>> b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
>>> new file mode 100644
>>> index 000000000000..1894c1c34d4f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
>>
>> Why is this in misc? No suitable directory?
> 
> I chose misc because I don't know where it should fit, I did not find a suitable
> subsystem to include this driver because this is a driver for an FPGA IP core.
> Do you have an idea where I should put it?

Directory based on what this device does. Whether some device is
implemented as FPGA core or dedicated circuitry, it does not matter. Few
Time Division Multiplexing devices are related to audio, so they are in
sound. I don't know if TDD is something else than TDM. If nothing fits,
can be misc, but again - just because device does no fit, not the drivers.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
  2023-07-27  9:26     ` Krzysztof Kozlowski
@ 2023-07-27  9:46       ` Balas, Eliza
  2023-07-27 11:40         ` Nuno Sá
  0 siblings, 1 reply; 9+ messages in thread
From: Balas, Eliza @ 2023-07-27  9:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	devicetree



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, July 27, 2023 12:27
> 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>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
> 
> [External]
> 
> On 27/07/2023 11:05, Balas, Eliza wrote:
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Wednesday, July 26, 2023 21:35
> >> 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>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> >> Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
> >>
> >> [External]
> >>
> >> On 26/07/2023 09:11, Eliza Balas wrote:
> >>> Add device tree documentation for the AXI TDD Core.
> >>> The generic TDD controller is in essence a waveform generator capable
> >>> of addressing RF applications which require Time Division Duplexing,
> >>> as well as controlling other modules of general applications through
> >>> its dedicated 32 channel outputs.
> >>>
> >>> The reason of creating the generic TDD controller was to reduce the
> >>> naming confusion around the existing repurposed TDD core built for
> >>> AD9361, as well as expanding its number of output channels for systems
> >>> which require more than six controlling signals.
> >>
> >> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE`
> on
> >> the directory your patch is touching.
> >>
> >> Subject: drop driver. Bindings are for hardware, not drivers... unless driver is here a hardware term?
> >
> > It is not a hardware term in this case, I will make the changes.
> >
> >>>
> >>> Signed-off-by: Eliza Balas <eliza.balas@analog.com>
> >>> ---
> >>>  .../devicetree/bindings/misc/adi,axi-tdd.yaml | 51 +++++++++++++++++++
> >>>  MAINTAINERS                                   |  7 +++
> >>>  2 files changed, 58 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> >>> b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> >>> new file mode 100644
> >>> index 000000000000..1894c1c34d4f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> >>
> >> Why is this in misc? No suitable directory?
> >
> > I chose misc because I don't know where it should fit, I did not find a suitable
> > subsystem to include this driver because this is a driver for an FPGA IP core.
> > Do you have an idea where I should put it?
> 
> Directory based on what this device does. Whether some device is
> implemented as FPGA core or dedicated circuitry, it does not matter. Few
> Time Division Multiplexing devices are related to audio, so they are in
> sound. I don't know if TDD is something else than TDM. If nothing fits,
> can be misc, but again - just because device does no fit, not the drivers.

This device resembles a bit with an IIO device (we are dealing with channels and the
sysfs interface follows the IIO specification), but is not registered into the IIO device tree, 
and does not rely on IIO kernel APIs. 
Do you think this device is a better fit into the IIO subsystem?

Thank you,
Eliza

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver
  2023-07-27  9:46       ` Balas, Eliza
@ 2023-07-27 11:40         ` Nuno Sá
  0 siblings, 0 replies; 9+ messages in thread
From: Nuno Sá @ 2023-07-27 11:40 UTC (permalink / raw)
  To: Balas, Eliza, Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	devicetree, Jonathan Cameron

On Thu, 2023-07-27 at 09:46 +0000, Balas, Eliza wrote:
> 
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Sent: Thursday, July 27, 2023 12:27
> > 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>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD
> > engine driver
> > 
> > [External]
> > 
> > On 27/07/2023 11:05, Balas, Eliza wrote:
> > > 
> > > > -----Original Message-----
> > > > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Sent: Wednesday, July 26, 2023 21:35
> > > > 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>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> > > > linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org
> > > > Subject: Re: [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new
> > > > TDD engine driver
> > > > 
> > > > [External]
> > > > 
> > > > On 26/07/2023 09:11, Eliza Balas wrote:
> > > > > Add device tree documentation for the AXI TDD Core.
> > > > > The generic TDD controller is in essence a waveform generator capable
> > > > > of addressing RF applications which require Time Division Duplexing,
> > > > > as well as controlling other modules of general applications through
> > > > > its dedicated 32 channel outputs.
> > > > > 
> > > > > The reason of creating the generic TDD controller was to reduce the
> > > > > naming confusion around the existing repurposed TDD core built for
> > > > > AD9361, as well as expanding its number of output channels for systems
> > > > > which require more than six controlling signals.
> > > > 
> > > > Please use subject prefixes matching the subsystem. You can get them for
> > > > example with `git log --oneline -- DIRECTORY_OR_FILE`
> > on
> > > > the directory your patch is touching.
> > > > 
> > > > Subject: drop driver. Bindings are for hardware, not drivers... unless driver
> > > > is here a hardware term?
> > > 
> > > It is not a hardware term in this case, I will make the changes.
> > > 
> > > > > 
> > > > > Signed-off-by: Eliza Balas <eliza.balas@analog.com>
> > > > > ---
> > > > >  .../devicetree/bindings/misc/adi,axi-tdd.yaml | 51 +++++++++++++++++++
> > > > >  MAINTAINERS                                   |  7 +++
> > > > >  2 files changed, 58 insertions(+)
> > > > >  create mode 100644
> > > > > Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > > b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..1894c1c34d4f
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/misc/adi,axi-tdd.yaml
> > > > 
> > > > Why is this in misc? No suitable directory?
> > > 
> > > I chose misc because I don't know where it should fit, I did not find a
> > > suitable
> > > subsystem to include this driver because this is a driver for an FPGA IP core.
> > > Do you have an idea where I should put it?
> > 
> > Directory based on what this device does. Whether some device is
> > implemented as FPGA core or dedicated circuitry, it does not matter. Few
> > Time Division Multiplexing devices are related to audio, so they are in
> > sound. I don't know if TDD is something else than TDM. If nothing fits,
> > can be misc, but again - just because device does no fit, not the drivers.
> 
> This device resembles a bit with an IIO device (we are dealing with channels and
> the
> sysfs interface follows the IIO specification), but is not registered into the IIO
> device tree, 
> and does not rely on IIO kernel APIs. 
> Do you think this device is a better fit into the IIO subsystem?
> 

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.

+cc Jonathan...

- Nuno Sá


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-07-27 11:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-26  7:11 [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: Add new TDD engine driver Eliza Balas
2023-07-26  7:11 ` [PATCH 2/2] drivers: misc: adi-axi-tdd: " Eliza Balas
2023-07-26 18:39   ` Krzysztof Kozlowski
2023-07-27  6:40     ` Nuno Sá
2023-07-26 18:35 ` [PATCH 1/2] Documentation: bindings: adi,axi-tdd.yaml: " Krzysztof Kozlowski
2023-07-27  9:05   ` Balas, Eliza
2023-07-27  9:26     ` Krzysztof Kozlowski
2023-07-27  9:46       ` Balas, Eliza
2023-07-27 11:40         ` Nuno Sá

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.