lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <00586912-661e-4092-a69d-87defe26db59@arm.com>
Date: Mon, 17 Nov 2025 11:32:03 +0000
From: Robin Murphy <robin.murphy@....com>
To: Jun Guo <jun.guo@...tech.com>, peter.chen@...tech.com,
 fugang.duan@...tech.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, vkoul@...nel.org, ychuang3@...oton.com,
 schung@...oton.com
Cc: dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, cix-kernel-upstream@...tech.com,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/3] dma: arm-dma350: add support for shared interrupt
 mode

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@...tech.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" },
>   	{}
>   };

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ