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

From: Thierry Reding
Date: Wed Sep 14 2022 - 10:24:30 EST


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@xxxxxxxxxx>
> ---
> 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
>

Attachment: signature.asc
Description: PGP signature