Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 14 Sep 2022 16:23:20 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Akhil R <akhilrajeev@nvidia.com>
Cc:     ldewangan@nvidia.com, jonathanh@nvidia.com, vkoul@kernel.org,
        p.zabel@pengutronix.de, dmaengine@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] dmaengine: tegra: Add support for dma-channel-mask
Message-ID: <YyHj2J1WblUiKkAL@orome>
References: <20220913155251.59375-1-akhilrajeev@nvidia.com>
 <20220913155251.59375-4-akhilrajeev@nvidia.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="xLNYABVN+YKfQJcC"
Content-Disposition: inline
In-Reply-To: <20220913155251.59375-4-akhilrajeev@nvidia.com>
User-Agent: Mutt/2.2.7 (2022-08-07)
Precedence: bulk


--xLNYABVN+YKfQJcC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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.
>=20
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/dma/tegra186-gpc-dma.c | 35 ++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>=20
> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dm=
a.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 */
> =20
>  /* 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
> =20
>  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 o=
f_phandle_args *dma_spec,
>  }
> =20
>  static const struct tegra_dma_chip_data tegra186_dma_chip_data =3D {
> -	.nr_channels =3D 31,
> +	.nr_channels =3D 32,
>  	.channel_reg_size =3D SZ_64K,
>  	.max_dma_count =3D SZ_1G,
>  	.hw_support_pause =3D false,
> @@ -1296,7 +1300,7 @@ static const struct tegra_dma_chip_data tegra186_dm=
a_chip_data =3D {
>  };
> =20
>  static const struct tegra_dma_chip_data tegra194_dma_chip_data =3D {
> -	.nr_channels =3D 31,
> +	.nr_channels =3D 32,
>  	.channel_reg_size =3D SZ_64K,
>  	.max_dma_count =3D SZ_1G,
>  	.hw_support_pause =3D true,
> @@ -1304,7 +1308,7 @@ static const struct tegra_dma_chip_data tegra194_dm=
a_chip_data =3D {
>  };
> =20
>  static const struct tegra_dma_chip_data tegra234_dma_chip_data =3D {
> -	.nr_channels =3D 31,
> +	.nr_channels =3D 32,
>  	.channel_reg_size =3D SZ_64K,
>  	.max_dma_count =3D SZ_1G,
>  	.hw_support_pause =3D true,
> @@ -1380,15 +1384,28 @@ static int tegra_dma_probe(struct platform_device=
 *pdev)
>  	}
>  	stream_id =3D iommu_spec->ids[0] & 0xffff;
> =20
> +	ret =3D 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 =3D TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK;
> +	}
> +
>  	INIT_LIST_HEAD(&tdma->dma_dev.channels);
>  	for (i =3D 0; i < cdata->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc =3D &tdma->channels[i];
> =20
> +		/* 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 =3D platform_get_irq(pdev, i);
>  		if (tdc->irq < 0)
>  			return tdc->irq;
> =20
> -		tdc->chan_base_offset =3D TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET +
> +		tdc->chan_base_offset =3D TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
>  					i * cdata->channel_reg_size;
>  		snprintf(tdc->name, sizeof(tdc->name), "gpcdma.%d", i);
>  		tdc->tdma =3D tdma;
> @@ -1450,7 +1467,7 @@ static int tegra_dma_probe(struct platform_device *=
pdev)
>  	}
> =20
>  	dev_info(&pdev->dev, "GPC DMA driver register %d channels\n",
> -		 cdata->nr_channels);
> +		 hweight_long(tdma->chan_mask));
> =20
>  	return 0;
>  }
> @@ -1473,6 +1490,9 @@ static int __maybe_unused tegra_dma_pm_suspend(stru=
ct device *dev)
>  	for (i =3D 0; i < tdma->chip_data->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc =3D &tdma->channels[i];
> =20
> +		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(struc=
t device *dev)
>  	for (i =3D 0; i < tdma->chip_data->nr_channels; i++) {
>  		struct tegra_dma_channel *tdc =3D &tdma->channels[i];
> =20
> +		if (!((1 << i) & tdma->chan_mask))
> +			continue;
> +
>  		tegra_dma_program_sid(tdc, tdc->stream_id);
>  	}
> =20
> --=20
> 2.17.1
>=20

--xLNYABVN+YKfQJcC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmMh49gACgkQ3SOs138+
s6Fctg/8C7aEA24X7yjt/TsXn4e4T+pcs7nRF6M6b3V5/lgNvnbDoUICZndvyN4G
bBXdqpm2G/DylASHLWZDEsZA5iwtZrwr9/ybf3nJwGu2m4RjQ/OebNtKrrDLDfqT
FibanEgWO4SdllNYsJ3xCCdPcxQHX1xftnpUlZNIgDieaZSBjBfqpMZ1qGHIDgj4
RN0H8Bjm/K/st7pkr5/XtqGGbsH/Qk0IVx3WrEfi3RnanVr3qv27njMQ9wC6RT89
djtpbx7OUnSVCcv7OiTdmc1k7oJ7j+qjXu5+KJ91fR5wwvT1ZczymBRTe/n97dMG
OjDTIRS8QGfFXVMv4kTvkGKGRVT3jARCNGRKmVTWlFAV9vWLzQg3SrkHcVY20PXX
ZeUydiE+BT4Kgqy/R/ddmQVmHv5TtH9vwrKvDpTen4Jwp0DV5RcMYe4FSZ6ihXyr
TYjUURB82dOVGRTlPbXzQUWkMlqzm2sk70H733nvye26cdAf81g7JivZUBhWDXGa
/NdmHqRF1iKTPge0xk4UuWvtFgzr0PLdrQjtMyf8jondQZLRyCuNj75LwSF4q4n/
MTgQHy4W73phdVBsaWUUxYWtDIaa+d6jaEgqECntD4qpUGGBeIOXkSmo3QiMuaS3
WdCY2/Dj9JJaeM2xLQukJVxj+X8eGg5LB08lXedrmuCAX1BLXFc=
=1RE1
-----END PGP SIGNATURE-----

--xLNYABVN+YKfQJcC--