linux-kernel - Re: [PATCH v3 2/5] i2c: tegra: Do not configure DMA if not supported

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <z3evk6j53hbgf426kc4ltdv4dbisoqnwkfwhapyenpadhey6v7@zvbljg5svppi>
Date: Tue, 10 Jun 2025 10:28:11 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Kartik Rajput <kkartik@...dia.com>
Cc: akhilrajeev@...dia.com, andi.shyti@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, jonathanh@...dia.com, ldewangan@...dia.com, 
	digetx@...il.com, linux-i2c@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/5] i2c: tegra: Do not configure DMA if not supported

On Mon, Jun 09, 2025 at 03:04:17PM +0530, Kartik Rajput wrote:
> On Tegra264, not all I2C controllers have the necessary interface to
> GPC DMA, this causes failures when function tegra_i2c_init_dma()
> is called.
> 
> Ensure that "dmas" device-tree property is present before initializing
> DMA in function tegra_i2c_init_dma().
> 
> Signed-off-by: Kartik Rajput <kkartik@...dia.com>
> ---
> v1 -> v2:
> 	* Update commit message to clarify that some I2C controllers may
> 	  not have the necessary interface to GPC DMA.
> ---
>  drivers/i2c/busses/i2c-tegra.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index ebd51165c46b..c7237d26b813 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -448,6 +448,9 @@ static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
>  	if (IS_VI(i2c_dev))
>  		return 0;
>  
> +	if (!device_property_present(i2c_dev->dev, "dmas"))
> +		return 0;

I know that you use the OF-independent variant here, but has this been
tested on ACPI?

Originally the intention behind this code was to get some sort of
validation of the DT (i.e. dmas property is desired, so we want to flag
if it isn't provided) with the fallback existing mostly just so things
can operate in the absence (or if APB/GPC DMA isn't available for some
reason).

If we now solely make this depend on the availability of the DT (or
ACPI) property, then we loose all of that validation. I suppose we have
DT schema to check for these kinds of things now, but since we're not
marking these properties as required, there's really no validation at
all anymore.

My concern is that if somebody's left out the dmas/dma-names properties
by accident, they may not get what they were asking for and we have no
hints to provide whatsoever. Maybe that's okay if we provide the base
DT, which has been unmodified for a while.

If that's what we want to do, it no longer makes sense to keep the
IS_VI() check above, though, because that's just redundant now.

Thierry

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