[PATCH v6 0/3] i2c: tegra: Improve reset and DMA operations

[PATCH v6 0/3] i2c: tegra: Improve reset and DMA operations

[Akhil R]

This patch series contains the following changes:

1. Fix ACPI reset error handling by using device_reset() instead of
   acpi_evaluate_object() directly, which properly handles ACPI error codes.

2. Add support for internal software reset as a fallback when external
   reset control is not available, particularly useful for platforms that
   restrict reset control from Linux or don't implement ACPI _RST method.

3. Remove unnecessary dma_sync_*() calls on dma_alloc_coherent() buffers,
   which were redundant and could cause issues.

v5->v6:
- Use device_reset() instead of acpi_evaluate_object() directly
- Added a brief description for internal reset.
- Added a cover letter.

v5: https://lore.kernel.org/linux-tegra/20250704064704.23003-1-akhilrajeev@nvidia.com/T/#m21ab8542cf92d6daafb6db9dc90b03eb63de8bbc

Akhil R (3):
  i2c: tegra: Fix reset error handling with ACPI
  i2c: tegra: Use internal reset when reset property is not available
  i2c: tegra: Remove dma_sync_*() calls

 drivers/i2c/busses/i2c-tegra.c | 77 ++++++++++++++++------------------
 1 file changed, 36 insertions(+), 41 deletions(-)

-- 
2.50.1

[PATCH v6 1/3] i2c: tegra: Fix reset error handling with ACPI

[Akhil R]

The acpi_evaluate_object() returns an ACPI error code and not
Linux one. For the some platforms the err will have positive code
which may be interpreted incorrectly. Use device_reset for reset
control which handles it correctly.

Fixes: bd2fdedbf2ba ("i2c: tegra: Add the ACPI support")
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 049b4d154c23..687d1e608abc 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -607,7 +607,6 @@ static int tegra_i2c_wait_for_config_load(struct tegra_i2c_dev *i2c_dev)
 static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 {
 	u32 val, clk_divisor, clk_multiplier, tsu_thd, tlow, thigh, non_hs_mode;
-	acpi_handle handle = ACPI_HANDLE(i2c_dev->dev);
 	struct i2c_timings *t = &i2c_dev->timings;
 	int err;
 
@@ -619,11 +618,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 	 * emit a noisy warning on error, which won't stay unnoticed and
 	 * won't hose machine entirely.
 	 */
-	if (handle)
-		err = acpi_evaluate_object(handle, "_RST", NULL, NULL);
-	else
-		err = reset_control_reset(i2c_dev->rst);
-
+	err = device_reset(i2c_dev->dev);
 	WARN_ON_ONCE(err);
 
 	if (IS_DVC(i2c_dev))
@@ -1666,19 +1661,6 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
 		i2c_dev->is_vi = true;
 }
 
-static int tegra_i2c_init_reset(struct tegra_i2c_dev *i2c_dev)
-{
-	if (ACPI_HANDLE(i2c_dev->dev))
-		return 0;
-
-	i2c_dev->rst = devm_reset_control_get_exclusive(i2c_dev->dev, "i2c");
-	if (IS_ERR(i2c_dev->rst))
-		return dev_err_probe(i2c_dev->dev, PTR_ERR(i2c_dev->rst),
-				      "failed to get reset control\n");
-
-	return 0;
-}
-
 static int tegra_i2c_init_clocks(struct tegra_i2c_dev *i2c_dev)
 {
 	int err;
@@ -1788,10 +1770,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 
 	tegra_i2c_parse_dt(i2c_dev);
 
-	err = tegra_i2c_init_reset(i2c_dev);
-	if (err)
-		return err;
-
 	err = tegra_i2c_init_clocks(i2c_dev);
 	if (err)
 		return err;
-- 
2.50.1

[PATCH v6 2/3] i2c: tegra: Use internal reset when reset property is not available

[Akhil R]

For controllers that has an internal software reset, make the reset
property optional. This provides and option to use I2C in systems
that choose to restrict reset control from Linux or not to implement
the ACPI _RST method.

Internal reset was not required when the reset control was mandatory.
But on platforms where the resets are outside the control of Linux,
this had to be implemented by just returning success from BPMP or with
an empty _RST method in the ACPI table, basically ignoring the reset.

While the internal reset is not identical to the hard reset of the
controller, this will reset all the internal state of the controller
including FIFOs. This may slightly alter the behaviour in systems
which were ignoring the reset but it should not cause any functional
difference since all the required I2C registers are configured after
this reset, just as in boot. Considering that this sequence is hit
during the boot or during the I2C recovery path from an error, the
internal reset provides a better alternative than just ignoring the
reset.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 687d1e608abc..e291b8586214 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;
@@ -603,6 +609,26 @@ 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;
+
+	/*
+	 * Writing 1 to I2C_MASTER_RESET_CNTRL will reset all internal state of
+	 * Master logic including FIFOs. Clear this bit to 0 for normal operation.
+	 * SW needs to wait for 2us after assertion and de-assertion of this soft
+	 * reset.
+	 */
+	i2c_writel(i2c_dev, 0x1, I2C_MASTER_RESET_CNTRL);
+	fsleep(2);
+
+	i2c_writel(i2c_dev, 0x0, I2C_MASTER_RESET_CNTRL);
+	fsleep(2);
+
+	return 0;
+}
 
 static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 {
@@ -619,6 +645,9 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 	 * won't hose machine entirely.
 	 */
 	err = device_reset(i2c_dev->dev);
+	if (err == -ENOENT)
+		err = tegra_i2c_master_reset(i2c_dev);
+
 	WARN_ON_ONCE(err);
 
 	if (IS_DVC(i2c_dev))
@@ -1467,6 +1496,7 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = false,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = false,
 	.has_apb_dma = true,
@@ -1491,6 +1521,7 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = false,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = false,
 	.has_apb_dma = true,
@@ -1515,6 +1546,7 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = false,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = true,
@@ -1539,6 +1571,7 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = true,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = true,
@@ -1563,6 +1596,7 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = true,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = true,
@@ -1587,6 +1621,7 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
 	.has_multi_master_mode = false,
 	.has_slcg_override_reg = true,
 	.has_mst_fifo = false,
+	.has_mst_reset = false,
 	.quirks = &tegra_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = false,
@@ -1611,6 +1646,7 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
 	.has_multi_master_mode = true,
 	.has_slcg_override_reg = true,
 	.has_mst_fifo = true,
+	.has_mst_reset = true,
 	.quirks = &tegra194_i2c_quirks,
 	.supports_bus_clear = true,
 	.has_apb_dma = false,
-- 
2.50.1

[PATCH v6 3/3] i2c: tegra: Remove dma_sync_*() calls

[Akhil R]

Calling dma_sync_*() on a buffer from dma_alloc_coherent() is pointless.
The driver should not be doing its own bounce-buffering if the buffer is
allocated through dma_alloc_coherent().

Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-tegra.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index e291b8586214..04a9610c0736 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1293,17 +1293,9 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 
 	if (i2c_dev->dma_mode) {
 		if (i2c_dev->msg_read) {
-			dma_sync_single_for_device(i2c_dev->dma_dev,
-						   i2c_dev->dma_phys,
-						   xfer_size, DMA_FROM_DEVICE);
-
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
 			if (err)
 				return err;
-		} else {
-			dma_sync_single_for_cpu(i2c_dev->dma_dev,
-						i2c_dev->dma_phys,
-						xfer_size, DMA_TO_DEVICE);
 		}
 	}
 
@@ -1313,11 +1305,6 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 		if (i2c_dev->dma_mode) {
 			memcpy(i2c_dev->dma_buf + I2C_PACKET_HEADER_SIZE,
 			       msg->buf, i2c_dev->msg_len);
-
-			dma_sync_single_for_device(i2c_dev->dma_dev,
-						   i2c_dev->dma_phys,
-						   xfer_size, DMA_TO_DEVICE);
-
 			err = tegra_i2c_dma_submit(i2c_dev, xfer_size);
 			if (err)
 				return err;
@@ -1358,13 +1345,8 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 			return -ETIMEDOUT;
 		}
 
-		if (i2c_dev->msg_read && i2c_dev->msg_err == I2C_ERR_NONE) {
-			dma_sync_single_for_cpu(i2c_dev->dma_dev,
-						i2c_dev->dma_phys,
-						xfer_size, DMA_FROM_DEVICE);
-
+		if (i2c_dev->msg_read && i2c_dev->msg_err == I2C_ERR_NONE)
 			memcpy(i2c_dev->msg_buf, i2c_dev->dma_buf, i2c_dev->msg_len);
-		}
 	}
 
 	time_left = tegra_i2c_wait_completion(i2c_dev, &i2c_dev->msg_complete,
-- 
2.50.1

Re: [PATCH v6 1/3] i2c: tegra: Fix reset error handling with ACPI

[Andy Shevchenko]

On Thu, Jul 10, 2025 at 06:42:04PM +0530, Akhil R wrote:
> The acpi_evaluate_object() returns an ACPI error code and not
> Linux one. For the some platforms the err will have positive code
> which may be interpreted incorrectly. Use device_reset for reset

device_reset()

> control which handles it correctly.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 2/3] i2c: tegra: Use internal reset when reset property is not available

[Andy Shevchenko]

On Thu, Jul 10, 2025 at 06:42:05PM +0530, Akhil R wrote:
> For controllers that has an internal software reset, make the reset
> property optional. This provides and option to use I2C in systems
> that choose to restrict reset control from Linux or not to implement
> the ACPI _RST method.
> 
> Internal reset was not required when the reset control was mandatory.
> But on platforms where the resets are outside the control of Linux,
> this had to be implemented by just returning success from BPMP or with
> an empty _RST method in the ACPI table, basically ignoring the reset.
> 
> While the internal reset is not identical to the hard reset of the
> controller, this will reset all the internal state of the controller
> including FIFOs. This may slightly alter the behaviour in systems
> which were ignoring the reset but it should not cause any functional
> difference since all the required I2C registers are configured after
> this reset, just as in boot. Considering that this sequence is hit
> during the boot or during the I2C recovery path from an error, the
> internal reset provides a better alternative than just ignoring the
> reset.

...

I would perhaps expand the comment here to explain ENOENT check and what do we
do in this case. (Note, no rewriting of the existing, just adding a paragraph)

	*
	* In case ... we compare with -ENOENT ...
	* ...
	*/

>  	err = device_reset(i2c_dev->dev);
> +	if (err == -ENOENT)
> +		err = tegra_i2c_master_reset(i2c_dev);

>  	WARN_ON_ONCE(err);

Other that that, LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 1/3] i2c: tegra: Fix reset error handling with ACPI

[Andi Shyti]

Hi Akhil,

On Thu, Jul 10, 2025 at 04:50:29PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 10, 2025 at 06:42:04PM +0530, Akhil R wrote:
> > The acpi_evaluate_object() returns an ACPI error code and not
> > Linux one. For the some platforms the err will have positive code
> > which may be interpreted incorrectly. Use device_reset for reset
> 
> device_reset()

no need to resend, I can fix it.

Andi

> > control which handles it correctly.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v6 2/3] i2c: tegra: Use internal reset when reset property is not available

[Andi Shyti]

Hi Akhil,

On Thu, Jul 10, 2025 at 04:53:01PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 10, 2025 at 06:42:05PM +0530, Akhil R wrote:
> > For controllers that has an internal software reset, make the reset
> > property optional. This provides and option to use I2C in systems
> > that choose to restrict reset control from Linux or not to implement
> > the ACPI _RST method.
> > 
> > Internal reset was not required when the reset control was mandatory.
> > But on platforms where the resets are outside the control of Linux,
> > this had to be implemented by just returning success from BPMP or with
> > an empty _RST method in the ACPI table, basically ignoring the reset.
> > 
> > While the internal reset is not identical to the hard reset of the
> > controller, this will reset all the internal state of the controller
> > including FIFOs. This may slightly alter the behaviour in systems
> > which were ignoring the reset but it should not cause any functional
> > difference since all the required I2C registers are configured after
> > this reset, just as in boot. Considering that this sequence is hit
> > during the boot or during the I2C recovery path from an error, the
> > internal reset provides a better alternative than just ignoring the
> > reset.
> 
> ...
> 
> I would perhaps expand the comment here to explain ENOENT check and what do we
> do in this case. (Note, no rewriting of the existing, just adding a paragraph)
> 
> 	*
> 	* In case ... we compare with -ENOENT ...
> 	* ...
> 	*/

If you write it here I can expand your comment before merging.

Or if you prefer sending a v7 is still fine.

Thanks,
Andi

> >  	err = device_reset(i2c_dev->dev);
> > +	if (err == -ENOENT)
> > +		err = tegra_i2c_master_reset(i2c_dev);
> 
> >  	WARN_ON_ONCE(err);
> 
> Other that that, LGTM,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v6 2/3] i2c: tegra: Use internal reset when reset property is not available

[Akhil R]

On Fri, 11 Jul 2025 18:00:45 +0200, Andi Shyti wrote:

...

>> I would perhaps expand the comment here to explain ENOENT check and what do we
>> do in this case. (Note, no rewriting of the existing, just adding a paragraph)
>> 
>> 	*
>> 	* In case ... we compare with -ENOENT ...
>> 	* ...
>> 	*/
>
> If you write it here I can expand your comment before merging.
>
> Or if you prefer sending a v7 is still fine.

Hi Andi,

I thought to update the comments as below. Please let me know if this can be
folded in. I can send a v7 if that is easier to merge.

	/*
	 * Reset the controller before initializing it.
	 * In case if device_reset() returns -ENOENT, i.e. when the reset is
	 * not available, the internal software reset will be used if it is
	 * supported by the controller.
	 */
	err = device_reset(i2c_dev->dev);
	if (err == -ENOENT)
		err = tegra_i2c_master_reset(i2c_dev);

	/*
	 * The reset shouldn't ever fail in practice. The failure will be a
	 * sign of a severe problem that needs to be resolved. Still we don't
	 * want to fail the initialization completely because this may break
	 * kernel boot up since voltage regulators use I2C. Hence, we will
	 * emit a noisy warning on error, which won't stay unnoticed and
	 * won't hose machine entirely.
	 */
	WARN_ON_ONCE(err);


Thanks & Regards,
Akhil

Re: [PATCH v6 2/3] i2c: tegra: Use internal reset when reset property is not available

[Andy Shevchenko]

On Mon, Jul 14, 2025 at 10:52:25AM +0530, Akhil R wrote:
> On Fri, 11 Jul 2025 18:00:45 +0200, Andi Shyti wrote:

...

> >> I would perhaps expand the comment here to explain ENOENT check and what do we
> >> do in this case. (Note, no rewriting of the existing, just adding a paragraph)
> >> 
> >> 	*
> >> 	* In case ... we compare with -ENOENT ...
> >> 	* ...
> >> 	*/
> >
> > If you write it here I can expand your comment before merging.
> >
> > Or if you prefer sending a v7 is still fine.
> 
> Hi Andi,
> 
> I thought to update the comments as below. Please let me know if this can be
> folded in. I can send a v7 if that is easier to merge.
> 
> 	/*
> 	 * Reset the controller before initializing it.
> 	 * In case if device_reset() returns -ENOENT, i.e. when the reset is
> 	 * not available, the internal software reset will be used if it is
> 	 * supported by the controller.
> 	 */
> 	err = device_reset(i2c_dev->dev);
> 	if (err == -ENOENT)
> 		err = tegra_i2c_master_reset(i2c_dev);
> 
> 	/*
> 	 * The reset shouldn't ever fail in practice. The failure will be a
> 	 * sign of a severe problem that needs to be resolved. Still we don't
> 	 * want to fail the initialization completely because this may break
> 	 * kernel boot up since voltage regulators use I2C. Hence, we will
> 	 * emit a noisy warning on error, which won't stay unnoticed and
> 	 * won't hose machine entirely.
> 	 */
> 	WARN_ON_ONCE(err);

Good for me.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 0/3] i2c: tegra: Improve reset and DMA operations

[Andi Shyti]

Hi Akhil,

with the changes we agreed afterwards...

> Akhil R (3):
>   i2c: tegra: Fix reset error handling with ACPI

... merged to i2c/i2c-host-fixes

>   i2c: tegra: Use internal reset when reset property is not available
>   i2c: tegra: Remove dma_sync_*() calls

mergedt to i2c/i2c-host-next.

Thanks,
Andi