linux-kernel - Re: [PATCH v4 2/3] i2c: tegra: make reset an optional property

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250610095157.12138-1-akhilrajeev@nvidia.com>
Date: Tue, 10 Jun 2025 15:21:57 +0530
From: Akhil R <akhilrajeev@...dia.com>
To: <thierry.reding@...il.com>
CC: <akhilrajeev@...dia.com>, <andi.shyti@...nel.org>, <conor+dt@...nel.org>,
	<devicetree@...r.kernel.org>, <digetx@...il.com>, <jonathanh@...dia.com>,
	<krzk+dt@...nel.org>, <ldewangan@...dia.com>, <linux-i2c@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-tegra@...r.kernel.org>,
	<p.zabel@...gutronix.de>, <robh@...nel.org>
Subject: Re: [PATCH v4 2/3] i2c: tegra: make reset an optional property

On Tue, 10 Jun 2025 10:32:56 +0200, Thierry Reding wrote:
>> For controllers that has an internal software reset, make the reset
>> property optional. This is useful in systems that choose to restrict
>> reset control from Linux.
>> 
>> Signed-off-by: Akhil R <akhilrajeev@...dia.com>
>> ---
>> v3->v4: No change
>> v2->v3: No change
>> v1->v2:
>>   * Call devm_reset_control_get_optional_exclusive() unconditionally.
>>   * Add more delay based on HW recommendation.
>> 
>>  drivers/i2c/busses/i2c-tegra.c | 33 +++++++++++++++++++++++++++++++--
>>  1 file changed, 31 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>> index 87976e99e6d0..22ddbae9d847 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -134,6 +134,8 @@
>>  #define I2C_MST_FIFO_STATUS_TX			GENMASK(23, 16)
>>  #define I2C_MST_FIFO_STATUS_RX			GENMASK(7, 0)
>>  
>> +#define I2C_MASTER_RESET_CNTRL			0x0a8
>> +
>>  /* configuration load timeout in microseconds */
>>  #define I2C_CONFIG_LOAD_TIMEOUT			1000000
>>  
>> @@ -184,6 +186,9 @@ enum msg_end_type {
>>   * @has_mst_fifo: The I2C controller contains the new MST FIFO interface that
>>   *		provides additional features and allows for longer messages to
>>   *		be transferred in one go.
>> + * @has_mst_reset: The I2C controller contains MASTER_RESET_CTRL register which
>> + *		provides an alternative to controller reset when configured as
>> + *		I2C master
>>   * @quirks: I2C adapter quirks for limiting write/read transfer size and not
>>   *		allowing 0 length transfers.
>>   * @supports_bus_clear: Bus Clear support to recover from bus hang during
>> @@ -213,6 +218,7 @@ struct tegra_i2c_hw_feature {
>>  	bool has_multi_master_mode;
>>  	bool has_slcg_override_reg;
>>  	bool has_mst_fifo;
>> +	bool has_mst_reset;
>>  	const struct i2c_adapter_quirks *quirks;
>>  	bool supports_bus_clear;
>>  	bool has_apb_dma;
>> @@ -604,6 +610,20 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
>>  	return 0;
>>  }
>>  
>> +static int tegra_i2c_master_reset(struct tegra_i2c_dev *i2c_dev)
>> +{
>> +	if (!i2c_dev->hw->has_mst_reset)
>> +		return -EOPNOTSUPP;
>> +
>> +	i2c_writel(i2c_dev, 0x1, I2C_MASTER_RESET_CNTRL);
>> +	udelay(2);
>> +
>> +	i2c_writel(i2c_dev, 0x0, I2C_MASTER_RESET_CNTRL);
>> +	udelay(2);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>>  {
>>  	u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh, non_hs_mode;
>> @@ -621,8 +641,10 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>>  	 */
>>  	if (handle)
>>  		err = acpi_evaluate_object(handle, "_RST", NULL, NULL);
>
> How is the internal reset handled on ACPI? Does the _RST method do the
> internal reset?

Right now, devices using ACPI would have to rely on the _RST method implementation.
It is unlikely that it implements an internal reset mechanism.

Do you suggest adding a check with 'acpi_has_method(handle, "_RST")' and fallback to
internal reset when it is false?

Regards,
Akhil