[PATCH 0/2] TCB: Add DMA support to read the capture register AB

[PATCH 0/2] TCB: Add DMA support to read the capture register AB

[Dharma Balasubiramani]

This patch series adds support to enable the DMA support for TCB.

When DMA is used, the Register AB (TC_RAB) address must be configured as
source address of the transfer. TC_RAB provides the next unread value from
TC_RA and TC_RB. It may be read by the DMA after a request has been
triggered upon loading TC_RA or TC_RB.

-----------------------------------
This is tested on sam9x60 curiosity

root@sam9x60-curiosity-sd:~# cat /sys/bus/counter/devices/counter0/count0/capture0
258428554
root@sam9x60-curiosity-sd:~# devmem2 0xf800800c 
/dev/mem opened.
Memory mapped at address 0xb6f78000.
Read at address  0xF800800C (0xb6f7800c): 0x0F674E8A

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Dharma Balasubiramani (2):
      counter: microchip-tcb-capture: Retrieve and map parent base address
      counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

 drivers/counter/microchip-tcb-capture.c | 128 +++++++++++++++++++++++++++++++-
 include/soc/at91/atmel_tcb.h            |   1 +
 2 files changed, 126 insertions(+), 3 deletions(-)
---
base-commit: fefff2755f2aa4125dce2a1edfe7e545c7c621f2
change-id: 20250528-mchp-tcb-dma-5a29148fe6e6

Best regards,
-- 
Dharma Balasubiramani <dharma.b@microchip.com>

[PATCH 1/2] counter: microchip-tcb-capture: Retrieve and map parent base address

[Dharma Balasubiramani]

Retrieve the parent TCB controller's platform_device and map its MMIO
region using devm_ioremap_resource(). This allows direct register access
through a base address, which is required for features like DMA that need
physical addresses.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/counter/microchip-tcb-capture.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
index 1a299d1f350b..9634da75bd1a 100644
--- a/drivers/counter/microchip-tcb-capture.c
+++ b/drivers/counter/microchip-tcb-capture.c
@@ -12,6 +12,7 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <uapi/linux/counter/microchip-tcb-capture.h>
@@ -29,7 +30,9 @@
 
 struct mchp_tc_data {
 	const struct atmel_tcb_config *tc_cfg;
+	struct platform_device *pdev;
 	struct regmap *regmap;
+	void __iomem *base;
 	int qdec_mode;
 	int num_channels;
 	int channel[2];
@@ -479,6 +482,8 @@ static int mchp_tc_probe(struct platform_device *pdev)
 	const struct atmel_tcb_config *tcb_config;
 	const struct of_device_id *match;
 	struct counter_device *counter;
+	struct platform_device *parent_pdev;
+	struct resource *parent_res;
 	struct mchp_tc_data *priv;
 	char clk_name[7];
 	struct regmap *regmap;
@@ -491,6 +496,18 @@ static int mchp_tc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	priv = counter_priv(counter);
 
+	parent_pdev = of_find_device_by_node(np->parent);
+	if (!parent_pdev)
+		return -EPROBE_DEFER;
+
+	parent_res = platform_get_resource(parent_pdev, IORESOURCE_MEM, 0);
+	if (!parent_res)
+		return -EINVAL;
+
+	priv->base = devm_ioremap_resource(&parent_pdev->dev, parent_res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
 	match = of_match_node(atmel_tc_of_match, np->parent);
 	tcb_config = match->data;
 	if (!tcb_config) {
@@ -563,6 +580,7 @@ static int mchp_tc_probe(struct platform_device *pdev)
 
 	priv->tc_cfg = tcb_config;
 	priv->regmap = regmap;
+	priv->pdev = pdev;
 	counter->name = dev_name(&pdev->dev);
 	counter->parent = &pdev->dev;
 	counter->ops = &mchp_tc_ops;

-- 
2.43.0

[PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

[Dharma Balasubiramani]

Add optional DMA-based data transfer support to read the TC_RAB register,
which provides the next unread captured value from either RA or RB. This
improves performance and offloads CPU when mchp,use-dma-cap is enabled in
the device tree.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 drivers/counter/microchip-tcb-capture.c | 110 +++++++++++++++++++++++++++++++-
 include/soc/at91/atmel_tcb.h            |   1 +
 2 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
index 9634da75bd1a..fa177edc6803 100644
--- a/drivers/counter/microchip-tcb-capture.c
+++ b/drivers/counter/microchip-tcb-capture.c
@@ -6,6 +6,9 @@
  */
 #include <linux/clk.h>
 #include <linux/counter.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
@@ -28,9 +31,19 @@
 #define ATMEL_TC_QDEN			BIT(8)
 #define ATMEL_TC_POSEN			BIT(9)
 
+struct mchp_tc_dma {
+	struct dma_chan *chan;
+	struct dma_slave_config slave_cfg;
+	u32 *buf;
+	dma_addr_t addr;
+	phys_addr_t phy_addr;
+	bool enabled;
+};
+
 struct mchp_tc_data {
 	const struct atmel_tcb_config *tc_cfg;
 	struct platform_device *pdev;
+	struct mchp_tc_dma dma;
 	struct regmap *regmap;
 	void __iomem *base;
 	int qdec_mode;
@@ -74,6 +87,61 @@ static struct counter_synapse mchp_tc_count_synapses[] = {
 	}
 };
 
+static void mchp_tc_dma_remove(void *data)
+{
+	struct mchp_tc_data *priv = data;
+
+	if (priv->dma.buf)
+		dma_free_coherent(&priv->pdev->dev, sizeof(u32),
+				  priv->dma.buf, priv->dma.addr);
+
+	if (priv->dma.chan)
+		dma_release_channel(priv->dma.chan);
+}
+
+static int mchp_tc_dma_transfer(struct mchp_tc_data *priv, u32 *val)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct device *dev = &priv->pdev->dev;
+	dma_cookie_t cookie;
+	int ret;
+
+	ret = dmaengine_slave_config(priv->dma.chan, &priv->dma.slave_cfg);
+	if (ret) {
+		dev_err(dev, "DMA slave_config failed (%d)\n", ret);
+		return ret;
+	}
+
+	desc = dmaengine_prep_dma_memcpy(priv->dma.chan,
+					 priv->dma.addr,
+					 priv->dma.slave_cfg.src_addr,
+					 sizeof(u32),
+					 DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
+	if (!desc) {
+		dev_err(dev, "DMA prep descriptor failed\n");
+		return -ENOMEM;
+	}
+
+	cookie = dmaengine_submit(desc);
+	if (dma_submit_error(cookie)) {
+		dev_err(dev, "DMA submit error (%d)\n", cookie);
+		return cookie ?: -EIO;
+	}
+
+	dma_async_issue_pending(priv->dma.chan);
+
+	ret = dma_sync_wait(priv->dma.chan, cookie);
+	if (ret) {
+		dev_err(dev, "DMA transfer timed out (%d)\n", ret);
+		return ret;
+	}
+
+	/* Retrieve the 32-bit value the engine just copied */
+	*val = le32_to_cpu(*(u32 *)priv->dma.buf);
+
+	return 0;
+}
+
 static int mchp_tc_count_function_read(struct counter_device *counter,
 				       struct counter_count *count,
 				       enum counter_function *function)
@@ -260,20 +328,25 @@ static int mchp_tc_count_cap_read(struct counter_device *counter,
 				  struct counter_count *count, size_t idx, u64 *val)
 {
 	struct mchp_tc_data *const priv = counter_priv(counter);
-	u32 cnt;
+	u32 cnt, reg_offset;
 	int ret;
 
 	switch (idx) {
 	case COUNTER_MCHP_EXCAP_RA:
-		ret = regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], RA), &cnt);
+		reg_offset = ATMEL_TC_REG((priv->channel[0]), RA);
 		break;
 	case COUNTER_MCHP_EXCAP_RB:
-		ret = regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], RB), &cnt);
+		reg_offset = ATMEL_TC_REG((priv->channel[0]), RB);
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	if (!priv->dma.enabled)
+		ret = regmap_read(priv->regmap, reg_offset, &cnt);
+	else
+		ret = mchp_tc_dma_transfer(priv, &cnt);
+
 	if (ret < 0)
 		return ret;
 
@@ -578,6 +651,7 @@ static int mchp_tc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	priv->dma.phy_addr = parent_res->start;
 	priv->tc_cfg = tcb_config;
 	priv->regmap = regmap;
 	priv->pdev = pdev;
@@ -589,6 +663,36 @@ static int mchp_tc_probe(struct platform_device *pdev)
 	counter->num_signals = ARRAY_SIZE(mchp_tc_count_signals);
 	counter->signals = mchp_tc_count_signals;
 
+	/* Check the dma flag */
+	priv->dma.enabled = of_property_read_bool(np, "mchp,use-dma-cap") ? true : false;
+
+	if (priv->dma.enabled) {
+		/* Initialise DMA */
+		priv->dma.buf = dma_alloc_coherent(&pdev->dev, sizeof(u32),
+						   &priv->dma.addr, GFP_KERNEL);
+		if (!priv->dma.buf)
+			return -ENOMEM;
+
+		priv->dma.chan = dma_request_chan(&parent_pdev->dev, "rx");
+		if (IS_ERR(priv->dma.chan))
+			return -EINVAL;
+
+		dev_info(&pdev->dev, "Using %s (rx) for DMA transfers\n",
+			 dma_chan_name(priv->dma.chan));
+
+		/* Configure DMA channel to read TC AB register */
+		priv->dma.slave_cfg.direction = DMA_DEV_TO_MEM;
+		priv->dma.slave_cfg.src_addr = priv->dma.phy_addr + ATMEL_TC_REG(priv->channel[0],
+										 RAB);
+		priv->dma.slave_cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		priv->dma.slave_cfg.src_maxburst = 1;
+		priv->dma.slave_cfg.dst_maxburst = 1;
+
+		ret = devm_add_action_or_reset(&pdev->dev, mchp_tc_dma_remove, priv);
+		if (ret)
+			return ret;
+	}
+
 	i = of_irq_get(np->parent, 0);
 	if (i == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
diff --git a/include/soc/at91/atmel_tcb.h b/include/soc/at91/atmel_tcb.h
index 26b56a07bd1f..9fad7f58a56a 100644
--- a/include/soc/at91/atmel_tcb.h
+++ b/include/soc/at91/atmel_tcb.h
@@ -243,6 +243,7 @@ extern const u8 atmel_tc_divisors[5];
 #define ATMEL_TC_RA	0x14		/* register A */
 #define ATMEL_TC_RB	0x18		/* register B */
 #define ATMEL_TC_RC	0x1c		/* register C */
+#define ATMEL_TC_RAB	0x0c		/* register AB */
 
 #define ATMEL_TC_SR	0x20		/* status (read-only) */
 /* Status-only flags */

-- 
2.43.0

Re: [PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

[kernel test robot]

Hi Dharma,

kernel test robot noticed the following build warnings:

[auto build test WARNING on fefff2755f2aa4125dce2a1edfe7e545c7c621f2]

url:    https://github.com/intel-lab-lkp/linux/commits/Dharma-Balasubiramani/counter-microchip-tcb-capture-Retrieve-and-map-parent-base-address/20250528-141627
base:   fefff2755f2aa4125dce2a1edfe7e545c7c621f2
patch link:    https://lore.kernel.org/r/20250528-mchp-tcb-dma-v1-2-083a41fb7b51%40microchip.com
patch subject: [PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads
config: arm-randconfig-r121-20250529 (https://download.01.org/0day-ci/archive/20250529/202505291723.WYIWqmvP-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.3.0
reproduce: (https://download.01.org/0day-ci/archive/20250529/202505291723.WYIWqmvP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505291723.WYIWqmvP-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/counter/microchip-tcb-capture.c:140:16: sparse: sparse: cast to restricted __le32

vim +140 drivers/counter/microchip-tcb-capture.c

   101	
   102	static int mchp_tc_dma_transfer(struct mchp_tc_data *priv, u32 *val)
   103	{
   104		struct dma_async_tx_descriptor *desc;
   105		struct device *dev = &priv->pdev->dev;
   106		dma_cookie_t cookie;
   107		int ret;
   108	
   109		ret = dmaengine_slave_config(priv->dma.chan, &priv->dma.slave_cfg);
   110		if (ret) {
   111			dev_err(dev, "DMA slave_config failed (%d)\n", ret);
   112			return ret;
   113		}
   114	
   115		desc = dmaengine_prep_dma_memcpy(priv->dma.chan,
   116						 priv->dma.addr,
   117						 priv->dma.slave_cfg.src_addr,
   118						 sizeof(u32),
   119						 DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
   120		if (!desc) {
   121			dev_err(dev, "DMA prep descriptor failed\n");
   122			return -ENOMEM;
   123		}
   124	
   125		cookie = dmaengine_submit(desc);
   126		if (dma_submit_error(cookie)) {
   127			dev_err(dev, "DMA submit error (%d)\n", cookie);
   128			return cookie ?: -EIO;
   129		}
   130	
   131		dma_async_issue_pending(priv->dma.chan);
   132	
   133		ret = dma_sync_wait(priv->dma.chan, cookie);
   134		if (ret) {
   135			dev_err(dev, "DMA transfer timed out (%d)\n", ret);
   136			return ret;
   137		}
   138	
   139		/* Retrieve the 32-bit value the engine just copied */
 > 140		*val = le32_to_cpu(*(u32 *)priv->dma.buf);
   141	
   142		return 0;
   143	}
   144	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 0/2] TCB: Add DMA support to read the capture register AB

[David Lechner]

Please include the counter: prefix in the subject of the cover letter
as well. It makes it easier to see at a glance what this series might
be about.

On 5/28/25 1:13 AM, Dharma Balasubiramani wrote:
> This patch series adds support to enable the DMA support for TCB.
> 
> When DMA is used, the Register AB (TC_RAB) address must be configured as
> source address of the transfer. TC_RAB provides the next unread value from
> TC_RA and TC_RB. It may be read by the DMA after a request has been
> triggered upon loading TC_RA or TC_RB.

Can you please explain what problem this series is solving and why we
need this change?

Re: [PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

[David Lechner]

On 5/28/25 1:13 AM, Dharma Balasubiramani wrote:
> Add optional DMA-based data transfer support to read the TC_RAB register,
> which provides the next unread captured value from either RA or RB. This
> improves performance and offloads CPU when mchp,use-dma-cap is enabled in
> the device tree.

It looks like this is using DMA to read a single register in the implementation
of a sysfs read. Do you have measurements to show the performance difference?
I find it hard to believe that this would actually make a significant difference
compared to the overhead of the read syscall to read the sysfs attribute.

Re: [PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

[Dharma.B]

On 29/05/25 9:03 pm, David Lechner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 5/28/25 1:13 AM, Dharma Balasubiramani wrote:
>> Add optional DMA-based data transfer support to read the TC_RAB register,
>> which provides the next unread captured value from either RA or RB. This
>> improves performance and offloads CPU when mchp,use-dma-cap is enabled in
>> the device tree.
> 
> It looks like this is using DMA to read a single register in the implementation
> of a sysfs read. Do you have measurements to show the performance difference?
> I find it hard to believe that this would actually make a significant difference
> compared to the overhead of the read syscall to read the sysfs attribute.
> 
Hi David,

Thanks for the feedback.

You're right — in our current testing setup, I didn't observe any 
significant performance benefit from using DMA to read the TC_RAB 
register via sysfs. I benchmarked both DMA-based and direct MMIO 
register access using a userspace program generating high-frequency 
capture events, and the overhead of the sysfs read path seems to 
dominate in both cases.

Our initial motivation for using DMA was that the TCB IP in Microchip 
SoCs includes optional DMA support specifically for capture value 
transfers. I wanted to evaluate the potential benefit of offloading CPU 
load when frequent capture events are occurring. However, in practice, 
the complexity added (especially due to blocking behavior in atomic 
contexts like watch) does not appear to be justified, at least via sysfs 
or simple polling.

I also tried routing the DMA-based read through the 
COUNTER_COMPONENT_EXTENSION watch path, but as you may expect, that 
ended up hanging due to blocking behavior in non-sleepable contexts. So 
that route seems unsuitable without a more complex asynchronous 
buffering model.

Would you suggest exploring a different approach or a more appropriate 
interface for DMA-based capture (e.g., via a dedicated ioctl or char 
device with async support)? I’m happy to rework it if there's a suitable 
context where DMA adds measurable value.

Thanks again for your review and time.

-- 
With Best Regards,
Dharma B.

Re: [PATCH 0/2] TCB: Add DMA support to read the capture register AB

[Dharma.B]

On 29/05/25 8:56 pm, David Lechner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Please include the counter: prefix in the subject of the cover letter
> as well. It makes it easier to see at a glance what this series might
> be about.

Sure, I will take care of this next time, Thanks.

> 
> On 5/28/25 1:13 AM, Dharma Balasubiramani wrote:
>> This patch series adds support to enable the DMA support for TCB.
>>
>> When DMA is used, the Register AB (TC_RAB) address must be configured as
>> source address of the transfer. TC_RAB provides the next unread value from
>> TC_RA and TC_RB. It may be read by the DMA after a request has been
>> triggered upon loading TC_RA or TC_RB.
> 
> Can you please explain what problem this series is solving and why we
> need this change?

This isn't solving any issues but I'm trying to make use of the feature.
> 


-- 
With Best Regards,
Dharma B.

Re: [PATCH 2/2] counter: microchip-tcb-capture: Add DMA support for TC_RAB register reads

[David Lechner]

On 6/4/25 1:15 AM, Dharma.B@microchip.com wrote:
> On 29/05/25 9:03 pm, David Lechner wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 5/28/25 1:13 AM, Dharma Balasubiramani wrote:
>>> Add optional DMA-based data transfer support to read the TC_RAB register,
>>> which provides the next unread captured value from either RA or RB. This
>>> improves performance and offloads CPU when mchp,use-dma-cap is enabled in
>>> the device tree.
>>
>> It looks like this is using DMA to read a single register in the implementation
>> of a sysfs read. Do you have measurements to show the performance difference?
>> I find it hard to believe that this would actually make a significant difference
>> compared to the overhead of the read syscall to read the sysfs attribute.
>>
> Hi David,
> 
> Thanks for the feedback.
> 
> You're right — in our current testing setup, I didn't observe any 
> significant performance benefit from using DMA to read the TC_RAB 
> register via sysfs. I benchmarked both DMA-based and direct MMIO 
> register access using a userspace program generating high-frequency 
> capture events, and the overhead of the sysfs read path seems to 
> dominate in both cases.
> 
> Our initial motivation for using DMA was that the TCB IP in Microchip 
> SoCs includes optional DMA support specifically for capture value 
> transfers. I wanted to evaluate the potential benefit of offloading CPU 
> load when frequent capture events are occurring. However, in practice, 
> the complexity added (especially due to blocking behavior in atomic 
> contexts like watch) does not appear to be justified, at least via sysfs 
> or simple polling.
> 
> I also tried routing the DMA-based read through the 
> COUNTER_COMPONENT_EXTENSION watch path, but as you may expect, that 
> ended up hanging due to blocking behavior in non-sleepable contexts. So 
> that route seems unsuitable without a more complex asynchronous 
> buffering model.
> 
> Would you suggest exploring a different approach or a more appropriate 
> interface for DMA-based capture (e.g., via a dedicated ioctl or char 
> device with async support)? I’m happy to rework it if there's a suitable 
> context where DMA adds measurable value.
> 
> Thanks again for your review and time.
> 

Adding a feature just to make use of something a chip can do doesn't
seem like the wisest approach. Without know how people will actually
want to use it, we would only be guessing during the design of the
userspace interface. It would be better to wait until there is an
actual real-world use case and design something around that need.