[PATCH v4 0/2] Tegra ADMA fixes

[PATCH v4 0/2] Tegra ADMA fixes

[Mohan Kumar D]

- Fix build error due to 64-by-32 division
- Additional check for adma max page

Mohan Kumar D (2):
  dmaengine: tegra210-adma: Fix build error due to 64-by-32 division
  dmaengine: tegra210-adma: check for adma max page

 drivers/dma/tegra210-adma.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Mohan Kumar D]

Kernel test robot reported the build errors on 32-bit platforms due to
plain 64-by-32 division. Following build erros were reported.

   "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] undefined!
    ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
    tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"

This can be fixed by using div_u64() for the adma address space

Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
Cc: stable@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3-lkp@intel.com/
Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
---
 drivers/dma/tegra210-adma.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 6896da8ac7ef..a0bd4822ed80 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -887,7 +887,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
 	const struct tegra_adma_chip_data *cdata;
 	struct tegra_adma *tdma;
 	struct resource *res_page, *res_base;
-	int ret, i, page_no;
+	u64 page_no, page_offset;
+	int ret, i;
 
 	cdata = of_device_get_match_data(&pdev->dev);
 	if (!cdata) {
@@ -914,10 +915,16 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
 		res_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "global");
 		if (res_base) {
-			page_no = (res_page->start - res_base->start) / cdata->ch_base_offset;
-			if (page_no <= 0)
+			if (WARN_ON(res_page->start <= res_base->start))
 				return -EINVAL;
-			tdma->ch_page_no = page_no - 1;
+
+			page_offset = res_page->start - res_base->start;
+			page_no = div_u64(page_offset, cdata->ch_base_offset);
+
+			if (WARN_ON(page_no == 0))
+				return -EINVAL;
+
+			tdma->ch_page_no = lower_32_bits(page_no) - 1;
 			tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
 			if (IS_ERR(tdma->base_addr))
 				return PTR_ERR(tdma->base_addr);
-- 
2.25.1

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Jon Hunter]

On 05/02/2025 03:31, Mohan Kumar D wrote:
> Kernel test robot reported the build errors on 32-bit platforms due to
> plain 64-by-32 division. Following build erros were reported.
> 
>     "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] undefined!
>      ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
>      tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"
> 
> This can be fixed by using div_u64() for the adma address space
> 
> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> Cc: stable@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3-lkp@intel.com/
> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> ---
>   drivers/dma/tegra210-adma.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 6896da8ac7ef..a0bd4822ed80 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -887,7 +887,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>   	const struct tegra_adma_chip_data *cdata;
>   	struct tegra_adma *tdma;
>   	struct resource *res_page, *res_base;
> -	int ret, i, page_no;
> +	u64 page_no, page_offset;
> +	int ret, i;
>   
>   	cdata = of_device_get_match_data(&pdev->dev);
>   	if (!cdata) {
> @@ -914,10 +915,16 @@ static int tegra_adma_probe(struct platform_device *pdev)
>   
>   		res_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "global");
>   		if (res_base) {
> -			page_no = (res_page->start - res_base->start) / cdata->ch_base_offset;
> -			if (page_no <= 0)
> +			if (WARN_ON(res_page->start <= res_base->start))
>   				return -EINVAL;
> -			tdma->ch_page_no = page_no - 1;
> +
> +			page_offset = res_page->start - res_base->start;
> +			page_no = div_u64(page_offset, cdata->ch_base_offset);
> +
> +			if (WARN_ON(page_no == 0))
> +				return -EINVAL;

Sorry to be pedantic but should this now be ...

if (WARN_ON((page_no == 0) || (page_no > UINT_MAX)))

Jon

> +
> +			tdma->ch_page_no = lower_32_bits(page_no) - 1;
>   			tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
>   			if (IS_ERR(tdma->base_addr))
>   				return PTR_ERR(tdma->base_addr);

-- 
nvpublic

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Jon Hunter]

On 05/02/2025 11:27, Jon Hunter wrote:
> 
> 
> On 05/02/2025 03:31, Mohan Kumar D wrote:
>> Kernel test robot reported the build errors on 32-bit platforms due to
>> plain 64-by-32 division. Following build erros were reported.
>>
>>     "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] 
>> undefined!
>>      ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
>>      tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"
>>
>> This can be fixed by using div_u64() for the adma address space
>>
>> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
>> Cc: stable@vger.kernel.org
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3- 
>> lkp@intel.com/
>> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 6896da8ac7ef..a0bd4822ed80 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -887,7 +887,8 @@ static int tegra_adma_probe(struct platform_device 
>> *pdev)
>>       const struct tegra_adma_chip_data *cdata;
>>       struct tegra_adma *tdma;
>>       struct resource *res_page, *res_base;
>> -    int ret, i, page_no;
>> +    u64 page_no, page_offset;
>> +    int ret, i;
>>       cdata = of_device_get_match_data(&pdev->dev);
>>       if (!cdata) {
>> @@ -914,10 +915,16 @@ static int tegra_adma_probe(struct 
>> platform_device *pdev)
>>           res_base = platform_get_resource_byname(pdev, 
>> IORESOURCE_MEM, "global");
>>           if (res_base) {
>> -            page_no = (res_page->start - res_base->start) / cdata- 
>> >ch_base_offset;
>> -            if (page_no <= 0)
>> +            if (WARN_ON(res_page->start <= res_base->start))
>>                   return -EINVAL;
>> -            tdma->ch_page_no = page_no - 1;
>> +
>> +            page_offset = res_page->start - res_base->start;
>> +            page_no = div_u64(page_offset, cdata->ch_base_offset);
>> +
>> +            if (WARN_ON(page_no == 0))
>> +                return -EINVAL;
> 
> Sorry to be pedantic but should this now be ...
> 
> if (WARN_ON((page_no == 0) || (page_no > UINT_MAX)))

Nevermind, patch 2/2 addresses this.

Jon

-- 
nvpublic

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Wed, Feb 05, 2025 at 09:01:30AM +0530, Mohan Kumar D wrote:
> Kernel test robot reported the build errors on 32-bit platforms due to
> plain 64-by-32 division. Following build erros were reported.
> 
>    "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] undefined!
>     ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
>     tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"
> 
> This can be fixed by using div_u64() for the adma address space
> 
> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> Cc: stable@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3-lkp@intel.com/
> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Vinod Koul]

Hi Mohan,

On 05-02-25, 09:01, Mohan Kumar D wrote:
> Kernel test robot reported the build errors on 32-bit platforms due to
> plain 64-by-32 division. Following build erros were reported.

Patch should describe the change! Please revise subject to describe that
and not fix build error... This can come in changelog and below para is
apt

> 
>    "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] undefined!
>     ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
>     tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"
> 
> This can be fixed by using div_u64() for the adma address space
> 
> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> Cc: stable@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3-lkp@intel.com/
> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 6896da8ac7ef..a0bd4822ed80 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -887,7 +887,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  	const struct tegra_adma_chip_data *cdata;
>  	struct tegra_adma *tdma;
>  	struct resource *res_page, *res_base;
> -	int ret, i, page_no;
> +	u64 page_no, page_offset;
> +	int ret, i;
>  
>  	cdata = of_device_get_match_data(&pdev->dev);
>  	if (!cdata) {
> @@ -914,10 +915,16 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  
>  		res_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "global");
>  		if (res_base) {
> -			page_no = (res_page->start - res_base->start) / cdata->ch_base_offset;
> -			if (page_no <= 0)
> +			if (WARN_ON(res_page->start <= res_base->start))
>  				return -EINVAL;
> -			tdma->ch_page_no = page_no - 1;
> +
> +			page_offset = res_page->start - res_base->start;
> +			page_no = div_u64(page_offset, cdata->ch_base_offset);
> +
> +			if (WARN_ON(page_no == 0))
> +				return -EINVAL;
> +
> +			tdma->ch_page_no = lower_32_bits(page_no) - 1;
>  			tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
>  			if (IS_ERR(tdma->base_addr))
>  				return PTR_ERR(tdma->base_addr);
> -- 
> 2.25.1

-- 
~Vinod

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Mohan Kumar D]

On 10-02-2025 16:32, Vinod Koul wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Mohan,
>
> On 05-02-25, 09:01, Mohan Kumar D wrote:
>> Kernel test robot reported the build errors on 32-bit platforms due to
>> plain 64-by-32 division. Following build erros were reported.
> Patch should describe the change! Please revise subject to describe that
> and not fix build error... This can come in changelog and below para is
> apt
Sure, Will update the subject and resend the updated patch
>>     "ERROR: modpost: "__udivdi3" [drivers/dma/tegra210-adma.ko] undefined!
>>      ld: drivers/dma/tegra210-adma.o: in function `tegra_adma_probe':
>>      tegra210-adma.c:(.text+0x12cf): undefined reference to `__udivdi3'"
>>
>> This can be fixed by using div_u64() for the adma address space
>>
>> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
>> Cc: stable@vger.kernel.org
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202412250204.GCQhdKe3-lkp@intel.com/
>> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 6896da8ac7ef..a0bd4822ed80 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -887,7 +887,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>>        const struct tegra_adma_chip_data *cdata;
>>        struct tegra_adma *tdma;
>>        struct resource *res_page, *res_base;
>> -     int ret, i, page_no;
>> +     u64 page_no, page_offset;
>> +     int ret, i;
>>
>>        cdata = of_device_get_match_data(&pdev->dev);
>>        if (!cdata) {
>> @@ -914,10 +915,16 @@ static int tegra_adma_probe(struct platform_device *pdev)
>>
>>                res_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "global");
>>                if (res_base) {
>> -                     page_no = (res_page->start - res_base->start) / cdata->ch_base_offset;
>> -                     if (page_no <= 0)
>> +                     if (WARN_ON(res_page->start <= res_base->start))
>>                                return -EINVAL;
>> -                     tdma->ch_page_no = page_no - 1;
>> +
>> +                     page_offset = res_page->start - res_base->start;
>> +                     page_no = div_u64(page_offset, cdata->ch_base_offset);
>> +
>> +                     if (WARN_ON(page_no == 0))
>> +                             return -EINVAL;
>> +
>> +                     tdma->ch_page_no = lower_32_bits(page_no) - 1;
>>                        tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
>>                        if (IS_ERR(tdma->base_addr))
>>                                return PTR_ERR(tdma->base_addr);
>> --
>> 2.25.1
> --
> ~Vinod

Re: [PATCH v4 1/2] dmaengine: tegra210-adma: Fix build error due to 64-by-32 division

[Vinod Koul]

On 10-02-25, 16:45, Mohan Kumar D wrote:
> 
> On 10-02-2025 16:32, Vinod Koul wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > Hi Mohan,
> > 
> > On 05-02-25, 09:01, Mohan Kumar D wrote:
> > > Kernel test robot reported the build errors on 32-bit platforms due to
> > > plain 64-by-32 division. Following build erros were reported.
> > Patch should describe the change! Please revise subject to describe that
> > and not fix build error... This can come in changelog and below para is
> > apt
> Sure, Will update the subject and resend the updated patch

Please collect the acks as well

-- 
~Vinod

[PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

[Mohan Kumar D]

Have additional check for max channel page during the probe
to cover if any offset overshoot happens due to wrong DT
configuration.

Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
Cc: stable@vger.kernel.org
Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
---
 drivers/dma/tegra210-adma.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index a0bd4822ed80..801740ad8e0d 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -83,7 +83,9 @@ struct tegra_adma;
  * @nr_channels: Number of DMA channels available.
  * @ch_fifo_size_mask: Mask for FIFO size field.
  * @sreq_index_offset: Slave channel index offset.
+ * @max_page: Maximum ADMA Channel Page.
  * @has_outstanding_reqs: If DMA channel can have outstanding requests.
+ * @set_global_pg_config: Global page programming.
  */
 struct tegra_adma_chip_data {
 	unsigned int (*adma_get_burst_config)(unsigned int burst_size);
@@ -99,6 +101,7 @@ struct tegra_adma_chip_data {
 	unsigned int nr_channels;
 	unsigned int ch_fifo_size_mask;
 	unsigned int sreq_index_offset;
+	unsigned int max_page;
 	bool has_outstanding_reqs;
 	void (*set_global_pg_config)(struct tegra_adma *tdma);
 };
@@ -854,6 +857,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
 	.nr_channels		= 22,
 	.ch_fifo_size_mask	= 0xf,
 	.sreq_index_offset	= 2,
+	.max_page		= 0,
 	.has_outstanding_reqs	= false,
 	.set_global_pg_config	= NULL,
 };
@@ -871,6 +875,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
 	.nr_channels		= 32,
 	.ch_fifo_size_mask	= 0x1f,
 	.sreq_index_offset	= 4,
+	.max_page		= 4,
 	.has_outstanding_reqs	= true,
 	.set_global_pg_config	= tegra186_adma_global_page_config,
 };
@@ -921,7 +926,7 @@ static int tegra_adma_probe(struct platform_device *pdev)
 			page_offset = res_page->start - res_base->start;
 			page_no = div_u64(page_offset, cdata->ch_base_offset);
 
-			if (WARN_ON(page_no == 0))
+			if (WARN_ON(page_no == 0 || page_no > cdata->max_page))
 				return -EINVAL;
 
 			tdma->ch_page_no = lower_32_bits(page_no) - 1;
-- 
2.25.1

Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

[Jon Hunter]

On 05/02/2025 03:31, Mohan Kumar D wrote:
> Have additional check for max channel page during the probe
> to cover if any offset overshoot happens due to wrong DT
> configuration.
> 
> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> ---
>   drivers/dma/tegra210-adma.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index a0bd4822ed80..801740ad8e0d 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -83,7 +83,9 @@ struct tegra_adma;
>    * @nr_channels: Number of DMA channels available.
>    * @ch_fifo_size_mask: Mask for FIFO size field.
>    * @sreq_index_offset: Slave channel index offset.
> + * @max_page: Maximum ADMA Channel Page.
>    * @has_outstanding_reqs: If DMA channel can have outstanding requests.
> + * @set_global_pg_config: Global page programming.
>    */
>   struct tegra_adma_chip_data {
>   	unsigned int (*adma_get_burst_config)(unsigned int burst_size);
> @@ -99,6 +101,7 @@ struct tegra_adma_chip_data {
>   	unsigned int nr_channels;
>   	unsigned int ch_fifo_size_mask;
>   	unsigned int sreq_index_offset;
> +	unsigned int max_page;
>   	bool has_outstanding_reqs;
>   	void (*set_global_pg_config)(struct tegra_adma *tdma);
>   };
> @@ -854,6 +857,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>   	.nr_channels		= 22,
>   	.ch_fifo_size_mask	= 0xf,
>   	.sreq_index_offset	= 2,
> +	.max_page		= 0,
>   	.has_outstanding_reqs	= false,
>   	.set_global_pg_config	= NULL,
>   };
> @@ -871,6 +875,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>   	.nr_channels		= 32,
>   	.ch_fifo_size_mask	= 0x1f,
>   	.sreq_index_offset	= 4,
> +	.max_page		= 4,
>   	.has_outstanding_reqs	= true,
>   	.set_global_pg_config	= tegra186_adma_global_page_config,
>   };
> @@ -921,7 +926,7 @@ static int tegra_adma_probe(struct platform_device *pdev)
>   			page_offset = res_page->start - res_base->start;
>   			page_no = div_u64(page_offset, cdata->ch_base_offset);
>   
> -			if (WARN_ON(page_no == 0))
> +			if (WARN_ON(page_no == 0 || page_no > cdata->max_page))

So no one should ever specify the 'page' region for Tegra210, correct? 
If they did then this would always fail. I don't know if it is also 
worth checking if someone has a 'page' region for a device that has 
max_page == 0?

Jon
-- 
nvpublic

Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

[Mohan Kumar D]

On 05-02-2025 17:10, Jon Hunter wrote:
>
>
> On 05/02/2025 03:31, Mohan Kumar D wrote:
>> Have additional check for max channel page during the probe
>> to cover if any offset overshoot happens due to wrong DT
>> configuration.
>>
>> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index a0bd4822ed80..801740ad8e0d 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -83,7 +83,9 @@ struct tegra_adma;
>>    * @nr_channels: Number of DMA channels available.
>>    * @ch_fifo_size_mask: Mask for FIFO size field.
>>    * @sreq_index_offset: Slave channel index offset.
>> + * @max_page: Maximum ADMA Channel Page.
>>    * @has_outstanding_reqs: If DMA channel can have outstanding 
>> requests.
>> + * @set_global_pg_config: Global page programming.
>>    */
>>   struct tegra_adma_chip_data {
>>       unsigned int (*adma_get_burst_config)(unsigned int burst_size);
>> @@ -99,6 +101,7 @@ struct tegra_adma_chip_data {
>>       unsigned int nr_channels;
>>       unsigned int ch_fifo_size_mask;
>>       unsigned int sreq_index_offset;
>> +    unsigned int max_page;
>>       bool has_outstanding_reqs;
>>       void (*set_global_pg_config)(struct tegra_adma *tdma);
>>   };
>> @@ -854,6 +857,7 @@ static const struct tegra_adma_chip_data 
>> tegra210_chip_data = {
>>       .nr_channels        = 22,
>>       .ch_fifo_size_mask    = 0xf,
>>       .sreq_index_offset    = 2,
>> +    .max_page        = 0,
>>       .has_outstanding_reqs    = false,
>>       .set_global_pg_config    = NULL,
>>   };
>> @@ -871,6 +875,7 @@ static const struct tegra_adma_chip_data 
>> tegra186_chip_data = {
>>       .nr_channels        = 32,
>>       .ch_fifo_size_mask    = 0x1f,
>>       .sreq_index_offset    = 4,
>> +    .max_page        = 4,
>>       .has_outstanding_reqs    = true,
>>       .set_global_pg_config    = tegra186_adma_global_page_config,
>>   };
>> @@ -921,7 +926,7 @@ static int tegra_adma_probe(struct 
>> platform_device *pdev)
>>               page_offset = res_page->start - res_base->start;
>>               page_no = div_u64(page_offset, cdata->ch_base_offset);
>>   -            if (WARN_ON(page_no == 0))
>> +            if (WARN_ON(page_no == 0 || page_no > cdata->max_page))
>
> So no one should ever specify the 'page' region for Tegra210, correct? 
> If they did then this would always fail. I don't know if it is also 
> worth checking if someone has a 'page' region for a device that has 
> max_page == 0?
Yes, DT binding specifies "page" should not be used for Tegra210 and 
above conditions takes care. I believe above condition should suffice.
>
> Jon

Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3743 bytes --]

On Wed, Feb 05, 2025 at 06:28:34PM +0530, Mohan Kumar D wrote:
> 
> On 05-02-2025 17:10, Jon Hunter wrote:
> > 
> > 
> > On 05/02/2025 03:31, Mohan Kumar D wrote:
> > > Have additional check for max channel page during the probe
> > > to cover if any offset overshoot happens due to wrong DT
> > > configuration.
> > > 
> > > Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> > > ---
> > >   drivers/dma/tegra210-adma.c | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> > > index a0bd4822ed80..801740ad8e0d 100644
> > > --- a/drivers/dma/tegra210-adma.c
> > > +++ b/drivers/dma/tegra210-adma.c
> > > @@ -83,7 +83,9 @@ struct tegra_adma;
> > >    * @nr_channels: Number of DMA channels available.
> > >    * @ch_fifo_size_mask: Mask for FIFO size field.
> > >    * @sreq_index_offset: Slave channel index offset.
> > > + * @max_page: Maximum ADMA Channel Page.
> > >    * @has_outstanding_reqs: If DMA channel can have outstanding
> > > requests.
> > > + * @set_global_pg_config: Global page programming.
> > >    */
> > >   struct tegra_adma_chip_data {
> > >       unsigned int (*adma_get_burst_config)(unsigned int burst_size);
> > > @@ -99,6 +101,7 @@ struct tegra_adma_chip_data {
> > >       unsigned int nr_channels;
> > >       unsigned int ch_fifo_size_mask;
> > >       unsigned int sreq_index_offset;
> > > +    unsigned int max_page;
> > >       bool has_outstanding_reqs;
> > >       void (*set_global_pg_config)(struct tegra_adma *tdma);
> > >   };
> > > @@ -854,6 +857,7 @@ static const struct tegra_adma_chip_data
> > > tegra210_chip_data = {
> > >       .nr_channels        = 22,
> > >       .ch_fifo_size_mask    = 0xf,
> > >       .sreq_index_offset    = 2,
> > > +    .max_page        = 0,
> > >       .has_outstanding_reqs    = false,
> > >       .set_global_pg_config    = NULL,
> > >   };
> > > @@ -871,6 +875,7 @@ static const struct tegra_adma_chip_data
> > > tegra186_chip_data = {
> > >       .nr_channels        = 32,
> > >       .ch_fifo_size_mask    = 0x1f,
> > >       .sreq_index_offset    = 4,
> > > +    .max_page        = 4,
> > >       .has_outstanding_reqs    = true,
> > >       .set_global_pg_config    = tegra186_adma_global_page_config,
> > >   };
> > > @@ -921,7 +926,7 @@ static int tegra_adma_probe(struct
> > > platform_device *pdev)
> > >               page_offset = res_page->start - res_base->start;
> > >               page_no = div_u64(page_offset, cdata->ch_base_offset);
> > >   -            if (WARN_ON(page_no == 0))
> > > +            if (WARN_ON(page_no == 0 || page_no > cdata->max_page))
> > 
> > So no one should ever specify the 'page' region for Tegra210, correct?
> > If they did then this would always fail. I don't know if it is also
> > worth checking if someone has a 'page' region for a device that has
> > max_page == 0?
> Yes, DT binding specifies "page" should not be used for Tegra210 and above
> conditions takes care. I believe above condition should suffice.

Yes, if you get this wrong, the DT validation should already catch it.
We could always make it very clear by adding an explicit check and a
corresponding error message, but I'm not sure it's really worth it in
this case. If we do, then maybe we should add error messages to these
other cases as well.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Wed, Feb 05, 2025 at 09:01:31AM +0530, Mohan Kumar D wrote:
> Have additional check for max channel page during the probe
> to cover if any offset overshoot happens due to wrong DT
> configuration.
> 
> Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mohan Kumar D <mkumard@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 0/2] Tegra ADMA fixes

[Jon Hunter]

On 05/02/2025 03:31, Mohan Kumar D wrote:
> - Fix build error due to 64-by-32 division
> - Additional check for adma max page
> 
> Mohan Kumar D (2):
>    dmaengine: tegra210-adma: Fix build error due to 64-by-32 division
>    dmaengine: tegra210-adma: check for adma max page
> 
>   drivers/dma/tegra210-adma.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 


Thanks! For the series ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic