linux-kernel - Re: [PATCH 3/3] dmaengine: tegra: Add support for dma-channel-mask

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] [day] [month] [year] [list]
Message-ID: <YyHj2J1WblUiKkAL@orome>
Date:   Wed, 14 Sep 2022 16:23:20 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Akhil R <akhilrajeev@...dia.com>
Cc:     ldewangan@...dia.com, jonathanh@...dia.com, vkoul@...nel.org,
        p.zabel@...gutronix.de, dmaengine@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] dmaengine: tegra: Add support for dma-channel-mask

On Tue, Sep 13, 2022 at 09:22:51PM +0530, Akhil R wrote:
> Add support for dma-channel-mask so that only the specified channels
> are used. This helps to reserve some channels for the firmware.
> 
> This was initially achieved by limiting the channel number to 31 in
> the driver and adjusting the register address to skip channel0 which
> was reserved for a firmware. Now, with this change, the driver can
> align more to the actual hardware which has 32 channels.
> 
> Signed-off-by: Akhil R <akhilrajeev@...dia.com>
> ---
>  drivers/dma/tegra186-gpc-dma.c | 35 ++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
> index fa9bda4a2bc6..f69a800c4f80 100644
> --- a/drivers/dma/tegra186-gpc-dma.c
> +++ b/drivers/dma/tegra186-gpc-dma.c
> @@ -161,7 +161,10 @@
>  #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5 msec */
>  
>  /* Channel base address offset from GPCDMA base address */
> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
> +
> +/* Default channel mask reserving channel0 */
> +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK	0xfffffffe
>  
>  struct tegra_dma;
>  struct tegra_dma_channel;
> @@ -246,6 +249,7 @@ struct tegra_dma {
>  	const struct tegra_dma_chip_data *chip_data;
>  	unsigned long sid_m2d_reserved;
>  	unsigned long sid_d2m_reserved;
> +	u32 chan_mask;
>  	void __iomem *base_addr;
>  	struct device *dev;
>  	struct dma_device dma_dev;
> @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec,
>  }
>  
>  static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>  	.channel_reg_size = SZ_64K,
>  	.max_dma_count = SZ_1G,
>  	.hw_support_pause = false,
> @@ -1296,7 +1300,7 @@ static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
>  };
>  
>  static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>  	.channel_reg_size = SZ_64K,
>  	.max_dma_count = SZ_1G,
>  	.hw_support_pause = true,
> @@ -1304,7 +1308,7 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
>  };
>  
>  static const struct tegra_dma_chip_data tegra234_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>  	.channel_reg_size = SZ_64K,
>  	.max_dma_count = SZ_1G,
>  	.hw_support_pause = true,
> @@ -1380,15 +1384,28 @@ static int tegra_dma_probe(struct platform_device *pdev)
>  	}
>  	stream_id = iommu_spec->ids[0] & 0xffff;
>  
> +	ret = device_property_read_u32(&pdev->dev, "dma-channel-mask",
> +				       &tdma->chan_mask);
> +	if (ret) {
> +		dev_warn(&pdev->dev,
> +			 "Missing dma-channel-mask property, using default channel mask %#x\n",
> +			 TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK);
> +		tdma->chan_mask = TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK;
> +	}
> +
>  	INIT_LIST_HEAD(&tdma->dma_dev.channels);
>  	for (i = 0; i < cdata->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc = &tdma->channels[i];
>  
> +		/* Check for channel mask */
> +		if (!((1 << i) & tdma->chan_mask))

It's more idiomatic to reverse the operands and use the BIT macro, which
checkpatch should've warned about:

	if (!(tdma->chan_mask & BIT(i)))

There's a few other occurrences of that below.

Thierry

> +			continue;
> +
>  		tdc->irq = platform_get_irq(pdev, i);
>  		if (tdc->irq < 0)
>  			return tdc->irq;
>  
> -		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET +
> +		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
>  					i * cdata->channel_reg_size;
>  		snprintf(tdc->name, sizeof(tdc->name), "gpcdma.%d", i);
>  		tdc->tdma = tdma;
> @@ -1450,7 +1467,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
>  	}
>  
>  	dev_info(&pdev->dev, "GPC DMA driver register %d channels\n",
> -		 cdata->nr_channels);
> +		 hweight_long(tdma->chan_mask));
>  
>  	return 0;
>  }
> @@ -1473,6 +1490,9 @@ static int __maybe_unused tegra_dma_pm_suspend(struct device *dev)
>  	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc = &tdma->channels[i];
>  
> +		if (!((1 << i) & tdma->chan_mask))
> +			continue;
> +
>  		if (tdc->dma_desc) {
>  			dev_err(tdma->dev, "channel %u busy\n", i);
>  			return -EBUSY;
> @@ -1492,6 +1512,9 @@ static int __maybe_unused tegra_dma_pm_resume(struct device *dev)
>  	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc = &tdma->channels[i];
>  
> +		if (!((1 << i) & tdma->chan_mask))
> +			continue;
> +
>  		tegra_dma_program_sid(tdc, tdc->stream_id);
>  	}
>  
> -- 
> 2.17.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)