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

[Akhil R <akhilrajeev@nvidia.com>]

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@nvidia.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