[PATCH 0/3] dmaengine: arm-dma350: add support for shared interrupt mode

[PATCH 0/3] dmaengine: arm-dma350: add support for shared interrupt mode

[Jun Guo]

The arm dma350 controller's hardware implementation varies: some
designs dedicate a separate interrupt line for each channel, while
others have all channels sharing a single interrupt.This patch adds
support for the hardware design where all DMA channels share a
single interrupt.

This series introduces the following enhancements for arm dma350
controller support on arm64 platforms:

Patch 1: Add a compatible string "cix,sky1-dma-350" for the cix
sky1 SoC.
Patch 2: Implement support for the shared interrupt design of the DMA
controller.
Patch 3: add DT nodes for DMA.

The patches have been tested on CIX SKY1 platform, the test steps are
as follows:
    % echo 2000 > /sys/module/dmatest/parameters/timeout
    % echo 1 > /sys/module/dmatest/parameters/iterations
    % echo " " > /sys/module/dmatest/parameters/channel
    % echo 1 > /sys/module/dmatest/parameters/run

Jun Guo (3):
  dt-bindings: dma: arm-dma350: update DT binding docs
  dma: arm-dma350: add support for shared interrupt mode
  arm64: dts: cix: add DT nodes for DMA

 .../devicetree/bindings/dma/arm,dma-350.yaml  |   6 +-
 arch/arm64/boot/dts/cix/sky1.dtsi             |   7 ++
 drivers/dma/arm-dma350.c                      | 115 ++++++++++++++++--
 3 files changed, 117 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Jun Guo]

- Add new compatible strings to the DT binding documents to support
 cix sky1 SoC.

Signed-off-by: Jun Guo <jun.guo@cixtech.com>
---
 Documentation/devicetree/bindings/dma/arm,dma-350.yaml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/arm,dma-350.yaml b/Documentation/devicetree/bindings/dma/arm,dma-350.yaml
index 429f682f15d8..3baf1ba5523d 100644
--- a/Documentation/devicetree/bindings/dma/arm,dma-350.yaml
+++ b/Documentation/devicetree/bindings/dma/arm,dma-350.yaml
@@ -14,7 +14,11 @@ allOf:
 
 properties:
   compatible:
-    const: arm,dma-350
+    oneOf:
+      - items:
+          - enum:
+              - cix,sky1-dma-350
+          - const: arm,dma-350
 
   reg:
     items:
-- 
2.34.1

[PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Jun Guo]

- The arm dma350 controller's hardware implementation varies: some
 designs dedicate a separate interrupt line for each channel, while
 others have all channels sharing a single interrupt.This patch adds
 support for the hardware design where all DMA channels share a
 single interrupt.

Signed-off-by: Jun Guo <jun.guo@cixtech.com>
---
 drivers/dma/arm-dma350.c | 115 +++++++++++++++++++++++++++++++++++----
 1 file changed, 105 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/arm-dma350.c b/drivers/dma/arm-dma350.c
index 9efe2ca7d5ec..cb1907be18d0 100644
--- a/drivers/dma/arm-dma350.c
+++ b/drivers/dma/arm-dma350.c
@@ -14,6 +14,7 @@
 #include "virt-dma.h"
 
 #define DMAINFO			0x0f00
+#define DRIVER_NAME		"arm-dma350"
 
 #define DMA_BUILDCFG0		0xb0
 #define DMA_CFG_DATA_WIDTH	GENMASK(18, 16)
@@ -142,6 +143,9 @@
 #define LINK_LINKADDR		BIT(30)
 #define LINK_LINKADDRHI		BIT(31)
 
+/* DMA NONSECURE CONTROL REGISTER */
+#define DMANSECCTRL		0x20c
+#define INTREN_ANYCHINTR_EN	BIT(0)
 
 enum ch_ctrl_donetype {
 	CH_CTRL_DONETYPE_NONE = 0,
@@ -192,6 +196,7 @@ struct d350_chan {
 
 struct d350 {
 	struct dma_device dma;
+	void __iomem *base;
 	int nchan;
 	int nreq;
 	struct d350_chan channels[] __counted_by(nchan);
@@ -461,7 +466,61 @@ static void d350_issue_pending(struct dma_chan *chan)
 	spin_unlock_irqrestore(&dch->vc.lock, flags);
 }
 
-static irqreturn_t d350_irq(int irq, void *data)
+static irqreturn_t d350_global_irq(int irq, void *data)
+{
+	struct d350 *dmac = (struct d350 *)data;
+	struct device *dev = dmac->dma.dev;
+	irqreturn_t ret = IRQ_NONE;
+	int i;
+
+	for (i = 0; i < dmac->nchan; i++) {
+		struct d350_chan *dch = &dmac->channels[i];
+		u32 ch_status;
+
+		ch_status = readl(dch->base + CH_STATUS);
+		if (!ch_status)
+			continue;
+
+		ret = IRQ_HANDLED;
+
+		if (ch_status & CH_STAT_INTR_ERR) {
+			struct virt_dma_desc *vd = &dch->desc->vd;
+			u32 errinfo = readl_relaxed(dch->base + CH_ERRINFO);
+
+			if (errinfo &
+			    (CH_ERRINFO_AXIRDPOISERR | CH_ERRINFO_AXIRDRESPERR))
+				vd->tx_result.result = DMA_TRANS_READ_FAILED;
+			else if (errinfo & CH_ERRINFO_AXIWRRESPERR)
+				vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
+			else
+				vd->tx_result.result = DMA_TRANS_ABORTED;
+
+			vd->tx_result.residue = d350_get_residue(dch);
+		} else if (!(ch_status & CH_STAT_INTR_DONE)) {
+			dev_warn(dev, "Channel %d unexpected IRQ: 0x%08x\n", i,
+				 ch_status);
+		}
+
+		writel_relaxed(ch_status, dch->base + CH_STATUS);
+
+		spin_lock(&dch->vc.lock);
+		if (ch_status & CH_STAT_INTR_DONE) {
+			vchan_cookie_complete(&dch->desc->vd);
+			dch->status = DMA_COMPLETE;
+			dch->residue = 0;
+			d350_start_next(dch);
+		} else if (ch_status & CH_STAT_INTR_ERR) {
+			vchan_cookie_complete(&dch->desc->vd);
+			dch->status = DMA_ERROR;
+			dch->residue = dch->desc->vd.tx_result.residue;
+		}
+		spin_unlock(&dch->vc.lock);
+	}
+
+	return ret;
+}
+
+static irqreturn_t d350_channel_irq(int irq, void *data)
 {
 	struct d350_chan *dch = data;
 	struct device *dev = dch->vc.chan.device->dev;
@@ -506,10 +565,18 @@ static irqreturn_t d350_irq(int irq, void *data)
 static int d350_alloc_chan_resources(struct dma_chan *chan)
 {
 	struct d350_chan *dch = to_d350_chan(chan);
-	int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
-			      dev_name(&dch->vc.chan.dev->device), dch);
-	if (!ret)
-		writel_relaxed(CH_INTREN_DONE | CH_INTREN_ERR, dch->base + CH_INTREN);
+	int ret = 0;
+
+	if (dch->irq) {
+		ret = request_irq(dch->irq, d350_channel_irq, IRQF_SHARED,
+				  dev_name(&dch->vc.chan.dev->device), dch);
+		if (ret) {
+			dev_err(chan->device->dev, "Failed to request IRQ %d\n", dch->irq);
+			return ret;
+		}
+	}
+
+	writel_relaxed(CH_INTREN_DONE | CH_INTREN_ERR, dch->base + CH_INTREN);
 
 	return ret;
 }
@@ -526,7 +593,7 @@ static void d350_free_chan_resources(struct dma_chan *chan)
 static int d350_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct d350 *dmac;
+	struct d350 *dmac = NULL;
 	void __iomem *base;
 	u32 reg;
 	int ret, nchan, dw, aw, r, p;
@@ -556,6 +623,7 @@ static int d350_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dmac->nchan = nchan;
+	dmac->base = base;
 
 	reg = readl_relaxed(base + DMAINFO + DMA_BUILDCFG1);
 	dmac->nreq = FIELD_GET(DMA_CFG_NUM_TRIGGER_IN, reg);
@@ -582,6 +650,26 @@ static int d350_probe(struct platform_device *pdev)
 	dmac->dma.device_issue_pending = d350_issue_pending;
 	INIT_LIST_HEAD(&dmac->dma.channels);
 
+	/* Cix Sky1 has a common host IRQ for all its channels. */
+	if (of_device_is_compatible(pdev->dev.of_node, "cix,sky1-dma-350")) {
+		int host_irq = platform_get_irq(pdev, 0);
+
+		if (host_irq < 0)
+			return dev_err_probe(dev, host_irq,
+					     "Failed to get IRQ\n");
+
+		ret = devm_request_irq(&pdev->dev, host_irq, d350_global_irq,
+				       IRQF_SHARED, DRIVER_NAME, dmac);
+		if (ret)
+			return dev_err_probe(
+				dev, ret,
+				"Failed to request the combined IRQ %d\n",
+				host_irq);
+
+		/* Combined Non-Secure Channel Interrupt Enable */
+		writel_relaxed(INTREN_ANYCHINTR_EN, dmac->base + DMANSECCTRL);
+	}
+
 	/* Would be nice to have per-channel caps for this... */
 	memset = true;
 	for (int i = 0; i < nchan; i++) {
@@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
 			dev_warn(dev, "No command link support on channel %d\n", i);
 			continue;
 		}
-		dch->irq = platform_get_irq(pdev, i);
-		if (dch->irq < 0)
-			return dev_err_probe(dev, dch->irq,
-					     "Failed to get IRQ for channel %d\n", i);
+
+		if (!of_device_is_compatible(pdev->dev.of_node,
+					     "cix,sky1-dma-350")) {
+			dch->irq = platform_get_irq(pdev, i);
+			if (dch->irq < 0)
+				return dev_err_probe(
+					dev, dch->irq,
+					"Failed to get IRQ for channel %d\n",
+					i);
+		}
 
 		dch->has_wrap = FIELD_GET(CH_CFG_HAS_WRAP, reg);
 		dch->has_trig = FIELD_GET(CH_CFG_HAS_TRIGIN, reg) &
@@ -640,6 +734,7 @@ static void d350_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id d350_of_match[] __maybe_unused = {
+	{ .compatible = "cix,sky1-dma-350" },
 	{ .compatible = "arm,dma-350" },
 	{}
 };
-- 
2.34.1

[PATCH 3/3] arm64: dts: cix: add DT nodes for DMA

[Jun Guo]

- Add the device tree node for the dma controller of the CIX SKY1 SoC.

Signed-off-by: Jun Guo <jun.guo@cixtech.com>
---
 arch/arm64/boot/dts/cix/sky1.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
index 189b9a3be55c..8bd7136e822d 100644
--- a/arch/arm64/boot/dts/cix/sky1.dtsi
+++ b/arch/arm64/boot/dts/cix/sky1.dtsi
@@ -353,6 +353,13 @@ iomuxc: pinctrl@4170000 {
 			reg = <0x0 0x04170000 0x0 0x1000>;
 		};
 
+		fch_dmac: dma-controller@4190000 {
+			compatible = "cix,sky1-dma-350", "arm,dma-350";
+			reg = <0x0 0x4190000 0x0 0x10000>;
+			interrupts = <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>;
+			#dma-cells = <1>;
+		};
+
 		mbox_ap2se: mailbox@5060000 {
 			compatible = "cix,sky1-mbox";
 			reg = <0x0 0x05060000 0x0 0x10000>;
-- 
2.34.1

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Krzysztof Kozlowski]

On 17/11/2025 02:59, Jun Guo wrote:
> - Add new compatible strings to the DT binding documents to support

This is not a list.

Also, subject is completely redundant. Everything is an update. Why are
you repeating DT binding docs?

>  cix sky1 SoC.
> 
> Signed-off-by: Jun Guo <jun.guo@cixtech.com>
> ---
You just broke all existing platforms. Please test your code properly.

Best regards,
Krzysztof

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Krzysztof Kozlowski]

On 17/11/2025 02:59, Jun Guo wrote:
> - The arm dma350 controller's hardware implementation varies: some

That's not a list. Look at git history to learn how to write expected
commit messages.

>  designs dedicate a separate interrupt line for each channel, while
>  others have all channels sharing a single interrupt.This patch adds
>  support for the hardware design where all DMA channels share a
>  single interrupt.
> 
> Signed-off-by: Jun Guo <jun.guo@cixtech.com>
> ---
>  drivers/dma/ar



> @@ -526,7 +593,7 @@ static void d350_free_chan_resources(struct dma_chan *chan)
>  static int d350_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct d350 *dmac;
> +	struct d350 *dmac = NULL;
>  	void __iomem *base;
>  	u32 reg;
>  	int ret, nchan, dw, aw, r, p;
> @@ -556,6 +623,7 @@ static int d350_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dmac->nchan = nchan;
> +	dmac->base = base;
>  
>  	reg = readl_relaxed(base + DMAINFO + DMA_BUILDCFG1);
>  	dmac->nreq = FIELD_GET(DMA_CFG_NUM_TRIGGER_IN, reg);
> @@ -582,6 +650,26 @@ static int d350_probe(struct platform_device *pdev)
>  	dmac->dma.device_issue_pending = d350_issue_pending;
>  	INIT_LIST_HEAD(&dmac->dma.channels);
>  
> +	/* Cix Sky1 has a common host IRQ for all its channels. */
> +	if (of_device_is_compatible(pdev->dev.of_node, "cix,sky1-dma-350")) {

No, see further

> +		int host_irq = platform_get_irq(pdev, 0);
> +
> +		if (host_irq < 0)
> +			return dev_err_probe(dev, host_irq,
> +					     "Failed to get IRQ\n");
> +
> +		ret = devm_request_irq(&pdev->dev, host_irq, d350_global_irq,
> +				       IRQF_SHARED, DRIVER_NAME, dmac);
> +		if (ret)
> +			return dev_err_probe(
> +				dev, ret,
> +				"Failed to request the combined IRQ %d\n",
> +				host_irq);
> +
> +		/* Combined Non-Secure Channel Interrupt Enable */
> +		writel_relaxed(INTREN_ANYCHINTR_EN, dmac->base + DMANSECCTRL);
> +	}
> +
>  	/* Would be nice to have per-channel caps for this... */
>  	memset = true;
>  	for (int i = 0; i < nchan; i++) {
> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>  			dev_warn(dev, "No command link support on channel %d\n", i);
>  			continue;
>  		}
> -		dch->irq = platform_get_irq(pdev, i);
> -		if (dch->irq < 0)
> -			return dev_err_probe(dev, dch->irq,
> -					     "Failed to get IRQ for channel %d\n", i);
> +
> +		if (!of_device_is_compatible(pdev->dev.of_node,
> +					     "cix,sky1-dma-350")) {

No, use driver match data for that. Sprinkling compatibles everywhere
does not scale.

Also, this is in contrary with the binding, which did not say your
device has no interrupts.


Best regards,
Krzysztof

Re: [PATCH 3/3] arm64: dts: cix: add DT nodes for DMA

[Krzysztof Kozlowski]

On 17/11/2025 02:59, Jun Guo wrote:
> - Add the device tree node for the dma controller of the CIX SKY1 SoC.

And here again...

> 
> Signed-off-by: Jun Guo <jun.guo@cixtech.com>
> ---
>  arch/arm64/boot/dts/cix/sky1.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Jun Guo]

On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 02:59, Jun Guo wrote:
>> - Add new compatible strings to the DT binding documents to support
> This is not a list.
> 
> Also, subject is completely redundant. Everything is an update. Why are
> you repeating DT binding docs?
> 
Thank you. I will incorporate your feedback in the next version.>>   cix 
sky1 SoC.
>>
>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>> ---
> You just broke all existing platforms. Please test your code properly.
The patch includes proper checks. Since this platform is the first user 
of the driver in the current codebase, the change won't affect other 
platforms.


Best regards,
Jun

Re: [PATCH 3/3] arm64: dts: cix: add DT nodes for DMA

[Jun Guo]

On 11/17/2025 2:14 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 02:59, Jun Guo wrote:
>> - Add the device tree node for the dma controller of the CIX SKY1 SoC.
> And here again...
> 
Thank you. I will incorporate your feedback in the next version.>> 
Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>> ---
>>   arch/arm64/boot/dts/cix/sky1.dtsi | 7 +++++++
>>   1 file changed, 7 insertions(+)

Best regards,
Jun

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Krzysztof Kozlowski]

On 17/11/2025 08:07, Jun Guo wrote:
> 
> On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
>> On 17/11/2025 02:59, Jun Guo wrote:
>>> - Add new compatible strings to the DT binding documents to support
>> This is not a list.
>>
>> Also, subject is completely redundant. Everything is an update. Why are
>> you repeating DT binding docs?
>>
> Thank you. I will incorporate your feedback in the next version.>>   cix 
> sky1 SoC.
>>>
>>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>>> ---
>> You just broke all existing platforms. Please test your code properly.
> The patch includes proper checks. Since this platform is the first user 

Nah, tests are here incomplete - look at the binding and DTS users...
nothing there, so you cannot test it.

> of the driver in the current codebase, the change won't affect other 
> platforms.

NAK, and you keep pushing... I just told you it will break everyone,
which is obvious from the diff.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Krzysztof Kozlowski]

On 17/11/2025 08:11, Krzysztof Kozlowski wrote:
> On 17/11/2025 08:07, Jun Guo wrote:
>>
>> On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
>>> On 17/11/2025 02:59, Jun Guo wrote:
>>>> - Add new compatible strings to the DT binding documents to support
>>> This is not a list.
>>>
>>> Also, subject is completely redundant. Everything is an update. Why are
>>> you repeating DT binding docs?
>>>
>> Thank you. I will incorporate your feedback in the next version.>>   cix 
>> sky1 SoC.
>>>>
>>>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>>>> ---
>>> You just broke all existing platforms. Please test your code properly.
>> The patch includes proper checks. Since this platform is the first user 
> 
> Nah, tests are here incomplete - look at the binding and DTS users...
> nothing there, so you cannot test it.
> 
>> of the driver in the current codebase, the change won't affect other 
>> platforms.
> 
> NAK, and you keep pushing... I just told you it will break everyone,
> which is obvious from the diff.

But if that was intentional change of ABI, then could be fine, but you
must provide in commit msg proper detailed rationale WHY you are
changing ABI and WHAT is the ABI impact of that change.

Best regards,
Krzysztof

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Jun Guo]

On 11/17/2025 2:13 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 02:59, Jun Guo wrote:
>> - The arm dma350 controller's hardware implementation varies: some
> That's not a list. Look at git history to learn how to write expected
> commit messages.
> 
Thank you. I will incorporate your feedback in the next version.>> 
designs dedicate a separate interrupt line for each channel, while
>>   others have all channels sharing a single interrupt.This patch adds
>>   support for the hardware design where all DMA channels share a
>>   single interrupt.
>>
>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>> ---
>>   drivers/dma/ar
> 
> 
>> @@ -526,7 +593,7 @@ static void d350_free_chan_resources(struct dma_chan *chan)
>>   static int d350_probe(struct platform_device *pdev)
>>   {
>>        struct device *dev = &pdev->dev;
>> -     struct d350 *dmac;
>> +     struct d350 *dmac = NULL;
>>        void __iomem *base;
>>        u32 reg;
>>        int ret, nchan, dw, aw, r, p;
>> @@ -556,6 +623,7 @@ static int d350_probe(struct platform_device *pdev)
>>                return -ENOMEM;
>>
>>        dmac->nchan = nchan;
>> +     dmac->base = base;
>>
>>        reg = readl_relaxed(base + DMAINFO + DMA_BUILDCFG1);
>>        dmac->nreq = FIELD_GET(DMA_CFG_NUM_TRIGGER_IN, reg);
>> @@ -582,6 +650,26 @@ static int d350_probe(struct platform_device *pdev)
>>        dmac->dma.device_issue_pending = d350_issue_pending;
>>        INIT_LIST_HEAD(&dmac->dma.channels);
>>
>> +     /* Cix Sky1 has a common host IRQ for all its channels. */
>> +     if (of_device_is_compatible(pdev->dev.of_node, "cix,sky1-dma-350")) {
> No, see further
> 
>> +             int host_irq = platform_get_irq(pdev, 0);
>> +
>> +             if (host_irq < 0)
>> +                     return dev_err_probe(dev, host_irq,
>> +                                          "Failed to get IRQ\n");
>> +
>> +             ret = devm_request_irq(&pdev->dev, host_irq, d350_global_irq,
>> +                                    IRQF_SHARED, DRIVER_NAME, dmac);
>> +             if (ret)
>> +                     return dev_err_probe(
>> +                             dev, ret,
>> +                             "Failed to request the combined IRQ %d\n",
>> +                             host_irq);
>> +
>> +             /* Combined Non-Secure Channel Interrupt Enable */
>> +             writel_relaxed(INTREN_ANYCHINTR_EN, dmac->base + DMANSECCTRL);
>> +     }
>> +
>>        /* Would be nice to have per-channel caps for this... */
>>        memset = true;
>>        for (int i = 0; i < nchan; i++) {
>> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>>                        dev_warn(dev, "No command link support on channel %d\n", i);
>>                        continue;
>>                }
>> -             dch->irq = platform_get_irq(pdev, i);
>> -             if (dch->irq < 0)
>> -                     return dev_err_probe(dev, dch->irq,
>> -                                          "Failed to get IRQ for channel %d\n", i);
>> +
>> +             if (!of_device_is_compatible(pdev->dev.of_node,
>> +                                          "cix,sky1-dma-350")) {
> No, use driver match data for that. Sprinkling compatibles everywhere
> does not scale.
> 
> Also, this is in contrary with the binding, which did not say your
> device has no interrupts.
Ok, I'll rework the patch based on your feedback.

Best regards,
Jun

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Robin Murphy]

On 2025-11-17 1:59 am, Jun Guo wrote:
> - The arm dma350 controller's hardware implementation varies: some
>   designs dedicate a separate interrupt line for each channel, while
>   others have all channels sharing a single interrupt.This patch adds
>   support for the hardware design where all DMA channels share a
>   single interrupt.

We already request the channel interrupts as shared, precisely because 
they could well end up muxed to the same physical interrupt line. I 
missed that the dedicated combined interrupt output had its own separate 
enable, but for that we may as well just set INTREN_ANYCHINTR_EN 
unconditionally - the rest of this seems pointless.

Thanks,
Robin.

> Signed-off-by: Jun Guo <jun.guo@cixtech.com>
> ---
>   drivers/dma/arm-dma350.c | 115 +++++++++++++++++++++++++++++++++++----
>   1 file changed, 105 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma/arm-dma350.c b/drivers/dma/arm-dma350.c
> index 9efe2ca7d5ec..cb1907be18d0 100644
> --- a/drivers/dma/arm-dma350.c
> +++ b/drivers/dma/arm-dma350.c
> @@ -14,6 +14,7 @@
>   #include "virt-dma.h"
>   
>   #define DMAINFO			0x0f00
> +#define DRIVER_NAME		"arm-dma350"
>   
>   #define DMA_BUILDCFG0		0xb0
>   #define DMA_CFG_DATA_WIDTH	GENMASK(18, 16)
> @@ -142,6 +143,9 @@
>   #define LINK_LINKADDR		BIT(30)
>   #define LINK_LINKADDRHI		BIT(31)
>   
> +/* DMA NONSECURE CONTROL REGISTER */
> +#define DMANSECCTRL		0x20c
> +#define INTREN_ANYCHINTR_EN	BIT(0)
>   
>   enum ch_ctrl_donetype {
>   	CH_CTRL_DONETYPE_NONE = 0,
> @@ -192,6 +196,7 @@ struct d350_chan {
>   
>   struct d350 {
>   	struct dma_device dma;
> +	void __iomem *base;
>   	int nchan;
>   	int nreq;
>   	struct d350_chan channels[] __counted_by(nchan);
> @@ -461,7 +466,61 @@ static void d350_issue_pending(struct dma_chan *chan)
>   	spin_unlock_irqrestore(&dch->vc.lock, flags);
>   }
>   
> -static irqreturn_t d350_irq(int irq, void *data)
> +static irqreturn_t d350_global_irq(int irq, void *data)
> +{
> +	struct d350 *dmac = (struct d350 *)data;
> +	struct device *dev = dmac->dma.dev;
> +	irqreturn_t ret = IRQ_NONE;
> +	int i;
> +
> +	for (i = 0; i < dmac->nchan; i++) {
> +		struct d350_chan *dch = &dmac->channels[i];
> +		u32 ch_status;
> +
> +		ch_status = readl(dch->base + CH_STATUS);
> +		if (!ch_status)
> +			continue;
> +
> +		ret = IRQ_HANDLED;
> +
> +		if (ch_status & CH_STAT_INTR_ERR) {
> +			struct virt_dma_desc *vd = &dch->desc->vd;
> +			u32 errinfo = readl_relaxed(dch->base + CH_ERRINFO);
> +
> +			if (errinfo &
> +			    (CH_ERRINFO_AXIRDPOISERR | CH_ERRINFO_AXIRDRESPERR))
> +				vd->tx_result.result = DMA_TRANS_READ_FAILED;
> +			else if (errinfo & CH_ERRINFO_AXIWRRESPERR)
> +				vd->tx_result.result = DMA_TRANS_WRITE_FAILED;
> +			else
> +				vd->tx_result.result = DMA_TRANS_ABORTED;
> +
> +			vd->tx_result.residue = d350_get_residue(dch);
> +		} else if (!(ch_status & CH_STAT_INTR_DONE)) {
> +			dev_warn(dev, "Channel %d unexpected IRQ: 0x%08x\n", i,
> +				 ch_status);
> +		}
> +
> +		writel_relaxed(ch_status, dch->base + CH_STATUS);
> +
> +		spin_lock(&dch->vc.lock);
> +		if (ch_status & CH_STAT_INTR_DONE) {
> +			vchan_cookie_complete(&dch->desc->vd);
> +			dch->status = DMA_COMPLETE;
> +			dch->residue = 0;
> +			d350_start_next(dch);
> +		} else if (ch_status & CH_STAT_INTR_ERR) {
> +			vchan_cookie_complete(&dch->desc->vd);
> +			dch->status = DMA_ERROR;
> +			dch->residue = dch->desc->vd.tx_result.residue;
> +		}
> +		spin_unlock(&dch->vc.lock);
> +	}
> +
> +	return ret;
> +}
> +
> +static irqreturn_t d350_channel_irq(int irq, void *data)
>   {
>   	struct d350_chan *dch = data;
>   	struct device *dev = dch->vc.chan.device->dev;
> @@ -506,10 +565,18 @@ static irqreturn_t d350_irq(int irq, void *data)
>   static int d350_alloc_chan_resources(struct dma_chan *chan)
>   {
>   	struct d350_chan *dch = to_d350_chan(chan);
> -	int ret = request_irq(dch->irq, d350_irq, IRQF_SHARED,
> -			      dev_name(&dch->vc.chan.dev->device), dch);
> -	if (!ret)
> -		writel_relaxed(CH_INTREN_DONE | CH_INTREN_ERR, dch->base + CH_INTREN);
> +	int ret = 0;
> +
> +	if (dch->irq) {
> +		ret = request_irq(dch->irq, d350_channel_irq, IRQF_SHARED,
> +				  dev_name(&dch->vc.chan.dev->device), dch);
> +		if (ret) {
> +			dev_err(chan->device->dev, "Failed to request IRQ %d\n", dch->irq);
> +			return ret;
> +		}
> +	}
> +
> +	writel_relaxed(CH_INTREN_DONE | CH_INTREN_ERR, dch->base + CH_INTREN);
>   
>   	return ret;
>   }
> @@ -526,7 +593,7 @@ static void d350_free_chan_resources(struct dma_chan *chan)
>   static int d350_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> -	struct d350 *dmac;
> +	struct d350 *dmac = NULL;
>   	void __iomem *base;
>   	u32 reg;
>   	int ret, nchan, dw, aw, r, p;
> @@ -556,6 +623,7 @@ static int d350_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   
>   	dmac->nchan = nchan;
> +	dmac->base = base;
>   
>   	reg = readl_relaxed(base + DMAINFO + DMA_BUILDCFG1);
>   	dmac->nreq = FIELD_GET(DMA_CFG_NUM_TRIGGER_IN, reg);
> @@ -582,6 +650,26 @@ static int d350_probe(struct platform_device *pdev)
>   	dmac->dma.device_issue_pending = d350_issue_pending;
>   	INIT_LIST_HEAD(&dmac->dma.channels);
>   
> +	/* Cix Sky1 has a common host IRQ for all its channels. */
> +	if (of_device_is_compatible(pdev->dev.of_node, "cix,sky1-dma-350")) {
> +		int host_irq = platform_get_irq(pdev, 0);
> +
> +		if (host_irq < 0)
> +			return dev_err_probe(dev, host_irq,
> +					     "Failed to get IRQ\n");
> +
> +		ret = devm_request_irq(&pdev->dev, host_irq, d350_global_irq,
> +				       IRQF_SHARED, DRIVER_NAME, dmac);
> +		if (ret)
> +			return dev_err_probe(
> +				dev, ret,
> +				"Failed to request the combined IRQ %d\n",
> +				host_irq);
> +
> +		/* Combined Non-Secure Channel Interrupt Enable */
> +		writel_relaxed(INTREN_ANYCHINTR_EN, dmac->base + DMANSECCTRL);
> +	}
> +
>   	/* Would be nice to have per-channel caps for this... */
>   	memset = true;
>   	for (int i = 0; i < nchan; i++) {
> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>   			dev_warn(dev, "No command link support on channel %d\n", i);
>   			continue;
>   		}
> -		dch->irq = platform_get_irq(pdev, i);
> -		if (dch->irq < 0)
> -			return dev_err_probe(dev, dch->irq,
> -					     "Failed to get IRQ for channel %d\n", i);
> +
> +		if (!of_device_is_compatible(pdev->dev.of_node,
> +					     "cix,sky1-dma-350")) {
> +			dch->irq = platform_get_irq(pdev, i);
> +			if (dch->irq < 0)
> +				return dev_err_probe(
> +					dev, dch->irq,
> +					"Failed to get IRQ for channel %d\n",
> +					i);
> +		}
>   
>   		dch->has_wrap = FIELD_GET(CH_CFG_HAS_WRAP, reg);
>   		dch->has_trig = FIELD_GET(CH_CFG_HAS_TRIGIN, reg) &
> @@ -640,6 +734,7 @@ static void d350_remove(struct platform_device *pdev)
>   }
>   
>   static const struct of_device_id d350_of_match[] __maybe_unused = {
> +	{ .compatible = "cix,sky1-dma-350" },
>   	{ .compatible = "arm,dma-350" },
>   	{}
>   };

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Jun Guo]

On 11/17/2025 2:13 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 02:59, Jun Guo wrote:
>> - The arm dma350 controller's hardware implementation varies: some
> That's not a list. Look at git history to learn how to write expected
> commit messages.
> 
>>   designs dedicate a separate interrupt line for each channel, while
>>   others have all channels sharing a single interrupt.This patch adds
>>   support for the hardware design where all DMA channels share a
>>   single interrupt.
>>
>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>> ---
>>   drivers/dma/ar
> 
> 
>> @@ -526,7 +593,7 @@ static void d350_free_chan_resources(struct dma_chan *chan)
>>   static int d350_probe(struct platform_device *pdev)
>>   {
>>        struct device *dev = &pdev->dev;
>> -     struct d350 *dmac;
>> +     struct d350 *dmac = NULL;
>>        void __iomem *base;
>>        u32 reg;
>>        int ret, nchan, dw, aw, r, p;
>> @@ -556,6 +623,7 @@ static int d350_probe(struct platform_device *pdev)
>>                return -ENOMEM;
>>
>>        dmac->nchan = nchan;
>> +     dmac->base = base;
>>
>>        reg = readl_relaxed(base + DMAINFO + DMA_BUILDCFG1);
>>        dmac->nreq = FIELD_GET(DMA_CFG_NUM_TRIGGER_IN, reg);
>> @@ -582,6 +650,26 @@ static int d350_probe(struct platform_device *pdev)
>>        dmac->dma.device_issue_pending = d350_issue_pending;
>>        INIT_LIST_HEAD(&dmac->dma.channels);
>>
>> +     /* Cix Sky1 has a common host IRQ for all its channels. */
>> +     if (of_device_is_compatible(pdev->dev.of_node, "cix,sky1-dma-350")) {
> No, see further
> 
>> +             int host_irq = platform_get_irq(pdev, 0);
>> +
>> +             if (host_irq < 0)
>> +                     return dev_err_probe(dev, host_irq,
>> +                                          "Failed to get IRQ\n");
>> +
>> +             ret = devm_request_irq(&pdev->dev, host_irq, d350_global_irq,
>> +                                    IRQF_SHARED, DRIVER_NAME, dmac);
>> +             if (ret)
>> +                     return dev_err_probe(
>> +                             dev, ret,
>> +                             "Failed to request the combined IRQ %d\n",
>> +                             host_irq);
>> +
>> +             /* Combined Non-Secure Channel Interrupt Enable */
>> +             writel_relaxed(INTREN_ANYCHINTR_EN, dmac->base + DMANSECCTRL);
>> +     }
>> +
>>        /* Would be nice to have per-channel caps for this... */
>>        memset = true;
>>        for (int i = 0; i < nchan; i++) {
>> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>>                        dev_warn(dev, "No command link support on channel %d\n", i);
>>                        continue;
>>                }
>> -             dch->irq = platform_get_irq(pdev, i);
>> -             if (dch->irq < 0)
>> -                     return dev_err_probe(dev, dch->irq,
>> -                                          "Failed to get IRQ for channel %d\n", i);
>> +
>> +             if (!of_device_is_compatible(pdev->dev.of_node,
>> +                                          "cix,sky1-dma-350")) {
> No, use driver match data for that. Sprinkling compatibles everywhere
> does not scale.
> 
I have another question: by "driver match data", are you referring to 
the data variable in the struct of_device_id?
> Also, this is in contrary with the binding, which did not say your
> device has no interrupts.
I need to clarify here: the issue with my chip platform is not the lack 
of interrupts, but that all DMA channels share a single interrupt. The 
current driver, however, defaults to assigning an individual interrupt 
for each DMA channel.

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Krzysztof Kozlowski]

On 17/11/2025 12:37, Jun Guo wrote:
>>>        /* Would be nice to have per-channel caps for this... */
>>>        memset = true;
>>>        for (int i = 0; i < nchan; i++) {
>>> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>>>                        dev_warn(dev, "No command link support on channel %d\n", i);
>>>                        continue;
>>>                }
>>> -             dch->irq = platform_get_irq(pdev, i);
>>> -             if (dch->irq < 0)
>>> -                     return dev_err_probe(dev, dch->irq,
>>> -                                          "Failed to get IRQ for channel %d\n", i);
>>> +
>>> +             if (!of_device_is_compatible(pdev->dev.of_node,
>>> +                                          "cix,sky1-dma-350")) {
>> No, use driver match data for that. Sprinkling compatibles everywhere
>> does not scale.
>>
> I have another question: by "driver match data", are you referring to 
> the data variable in the struct of_device_id?

Yes.

>> Also, this is in contrary with the binding, which did not say your
>> device has no interrupts.
> I need to clarify here: the issue with my chip platform is not the lack 
> of interrupts, but that all DMA channels share a single interrupt. The 
> current driver, however, defaults to assigning an individual interrupt 
> for each DMA channel.

I am speaking about binding, which said that first interrupt is only for
channel 0. You need to restrict/change it for your variant.

Best regards,
Krzysztof

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Jun Guo]

On 11/17/2025 7:44 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 12:37, Jun Guo wrote:
>>>>         /* Would be nice to have per-channel caps for this... */
>>>>         memset = true;
>>>>         for (int i = 0; i < nchan; i++) {
>>>> @@ -595,10 +683,16 @@ static int d350_probe(struct platform_device *pdev)
>>>>                         dev_warn(dev, "No command link support on channel %d\n", i);
>>>>                         continue;
>>>>                 }
>>>> -             dch->irq = platform_get_irq(pdev, i);
>>>> -             if (dch->irq < 0)
>>>> -                     return dev_err_probe(dev, dch->irq,
>>>> -                                          "Failed to get IRQ for channel %d\n", i);
>>>> +
>>>> +             if (!of_device_is_compatible(pdev->dev.of_node,
>>>> +                                          "cix,sky1-dma-350")) {
>>> No, use driver match data for that. Sprinkling compatibles everywhere
>>> does not scale.
>>>
>> I have another question: by "driver match data", are you referring to
>> the data variable in the struct of_device_id?
> Yes.
> 
OK.​ Thanks for the reminder.>>> Also, this is in contrary with the 
binding, which did not say your
>>> device has no interrupts.
>> I need to clarify here: the issue with my chip platform is not the lack
>> of interrupts, but that all DMA channels share a single interrupt. The
>> current driver, however, defaults to assigning an individual interrupt
>> for each DMA channel.
> I am speaking about binding, which said that first interrupt is only for
> channel 0. You need to restrict/change it for your variant.
Okay, I see your point. Thanks.

Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt mode

[Jun Guo]

On 11/17/2025 7:32 PM, Robin Murphy wrote:
> On 2025-11-17 1:59 am, Jun Guo wrote:
>> - The arm dma350 controller's hardware implementation varies: some
>>   designs dedicate a separate interrupt line for each channel, while
>>   others have all channels sharing a single interrupt.This patch adds
>>   support for the hardware design where all DMA channels share a
>>   single interrupt.
> 
> We already request the channel interrupts as shared, precisely because
> they could well end up muxed to the same physical interrupt line. I
> missed that the dedicated combined interrupt output had its own separate
> enable, but for that we may as well just set INTREN_ANYCHINTR_EN
> unconditionally - the rest of this seems pointless.
I'm not entirely sure if enabling the INTREN_ANYCHINTR_EN bit to 1 would 
affect the current default scenario where each channel is assigned its 
own interrupt. The hardware design of the chip I'm currently working 
with does not support testing the scenario of individual interrupts per 
channel. If it doesn't cause any issues, I will change it to an 
unconditional configuration.

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Jun Guo]

On 11/17/2025 3:13 PM, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
> 
> On 17/11/2025 08:11, Krzysztof Kozlowski wrote:
>> On 17/11/2025 08:07, Jun Guo wrote:
>>>
>>> On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
>>>> On 17/11/2025 02:59, Jun Guo wrote:
>>>>> - Add new compatible strings to the DT binding documents to support
>>>> This is not a list.
>>>>
>>>> Also, subject is completely redundant. Everything is an update. Why are
>>>> you repeating DT binding docs?
>>>>
>>> Thank you. I will incorporate your feedback in the next version.>>   cix
>>> sky1 SoC.
>>>>>
>>>>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>>>>> ---
>>>> You just broke all existing platforms. Please test your code properly.
>>> The patch includes proper checks. Since this platform is the first user
>>
>> Nah, tests are here incomplete - look at the binding and DTS users...
>> nothing there, so you cannot test it.
>>
>>> of the driver in the current codebase, the change won't affect other
>>> platforms.
>>
>> NAK, and you keep pushing... I just told you it will break everyone,
>> which is obvious from the diff.
> 
> But if that was intentional change of ABI, then could be fine, but you
> must provide in commit msg proper detailed rationale WHY you are
> changing ABI and WHAT is the ABI impact of that change.
> 
I'm not entirely sure if I fully grasped your point, but I also 
identified the potential issue. I've reworked the patch to accommodate 
both the default DTS case with a single compatible string and the 
scenario where platform-specific compatible strings need to be added. 
Could you please review it again and confirm if this aligns with your 
intent?

properties:
   compatible:
     contains:
       enum:
         - cix,sky1-dma-350
         - arm,dma-350

Best regards,
Jun

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Krzysztof Kozlowski]

On 17/11/2025 13:51, Jun Guo wrote:
> 
> On 11/17/2025 3:13 PM, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL
>>
>> On 17/11/2025 08:11, Krzysztof Kozlowski wrote:
>>> On 17/11/2025 08:07, Jun Guo wrote:
>>>>
>>>> On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
>>>>> On 17/11/2025 02:59, Jun Guo wrote:
>>>>>> - Add new compatible strings to the DT binding documents to support
>>>>> This is not a list.
>>>>>
>>>>> Also, subject is completely redundant. Everything is an update. Why are
>>>>> you repeating DT binding docs?
>>>>>
>>>> Thank you. I will incorporate your feedback in the next version.>>   cix
>>>> sky1 SoC.
>>>>>>
>>>>>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>>>>>> ---
>>>>> You just broke all existing platforms. Please test your code properly.
>>>> The patch includes proper checks. Since this platform is the first user
>>>
>>> Nah, tests are here incomplete - look at the binding and DTS users...
>>> nothing there, so you cannot test it.
>>>
>>>> of the driver in the current codebase, the change won't affect other
>>>> platforms.
>>>
>>> NAK, and you keep pushing... I just told you it will break everyone,
>>> which is obvious from the diff.
>>
>> But if that was intentional change of ABI, then could be fine, but you
>> must provide in commit msg proper detailed rationale WHY you are
>> changing ABI and WHAT is the ABI impact of that change.
>>
> I'm not entirely sure if I fully grasped your point, but I also 
> identified the potential issue. I've reworked the patch to accommodate 
> both the default DTS case with a single compatible string and the 
> scenario where platform-specific compatible strings need to be added. 
> Could you please review it again and confirm if this aligns with your 
> intent?
> 
> properties:
>    compatible:
>      contains:

Which example uses such syntax?

>        enum:
>          - cix,sky1-dma-350
>          - arm,dma-350

It's completely different than previous, so I don't know what you want
to achieve, but maybe you wanted to explain lack of compatibility in the
commit msg. Anyway, send proper patches with proper justification (and
for above would really need to be proper).

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: dma: arm-dma350: update DT binding docs

[Jun Guo]

On 11/17/2025 9:29 PM, Krzysztof Kozlowski wrote:
> On 17/11/2025 13:51, Jun Guo wrote:
>> On 11/17/2025 3:13 PM, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL
>>>
>>> On 17/11/2025 08:11, Krzysztof Kozlowski wrote:
>>>> On 17/11/2025 08:07, Jun Guo wrote:
>>>>> On 11/17/2025 2:11 PM, Krzysztof Kozlowski wrote:
>>>>>> On 17/11/2025 02:59, Jun Guo wrote:
>>>>>>> - Add new compatible strings to the DT binding documents to support
>>>>>> This is not a list.
>>>>>>
>>>>>> Also, subject is completely redundant. Everything is an update. Why are
>>>>>> you repeating DT binding docs?
>>>>>>
>>>>> Thank you. I will incorporate your feedback in the next version.>>   cix
>>>>> sky1 SoC.
>>>>>>> Signed-off-by: Jun Guo<jun.guo@cixtech.com>
>>>>>>> ---
>>>>>> You just broke all existing platforms. Please test your code properly.
>>>>> The patch includes proper checks. Since this platform is the first user
>>>> Nah, tests are here incomplete - look at the binding and DTS users...
>>>> nothing there, so you cannot test it.
>>>>
>>>>> of the driver in the current codebase, the change won't affect other
>>>>> platforms.
>>>> NAK, and you keep pushing... I just told you it will break everyone,
>>>> which is obvious from the diff.
>>> But if that was intentional change of ABI, then could be fine, but you
>>> must provide in commit msg proper detailed rationale WHY you are
>>> changing ABI and WHAT is the ABI impact of that change.
>>>
>> I'm not entirely sure if I fully grasped your point, but I also
>> identified the potential issue. I've reworked the patch to accommodate
>> both the default DTS case with a single compatible string and the
>> scenario where platform-specific compatible strings need to be added.
>> Could you please review it again and confirm if this aligns with your
>> intent?
>>
>> properties:
>>     compatible:
>>       contains:
> Which example uses such syntax?
> 
>>         enum:
>>           - cix,sky1-dma-350
>>           - arm,dma-350
> It's completely different than previous, so I don't know what you want
> to achieve, but maybe you wanted to explain lack of compatibility in the
> commit msg. Anyway, send proper patches with proper justification (and
> for above would really need to be proper).
> 
OK.Thank you for your prompt​ reply.> Best regards,
> Krzysztof

Best regards,
Jun