[PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[Akhil R]

Specify the properties which are essential for the Tegra I2C driver to
function correctly. Though all the existing DT nodes have these
properties already, it was not mandated by the DT bindings.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
v2->v3:
  * Updated commit description on the details and fixed indentation
    issue.
v1->v2:
  * Added all required properties

 .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
index b57ae6963e62..c1d38e6ff7d7 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
@@ -97,7 +97,9 @@ properties:
 
   resets:
     items:
-      - description: module reset
+      - description: |
+          Module reset. This property is optional for controllers in Tegra194 and later
+          chips where an internal software reset is available as an alternative.
 
   reset-names:
     items:
@@ -116,6 +118,13 @@ properties:
       - const: rx
       - const: tx
 
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
 allOf:
   - $ref: /schemas/i2c/i2c-controller.yaml
   - if:
@@ -169,6 +178,18 @@ allOf:
       properties:
         power-domains: false
 
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - nvidia,tegra194-i2c
+    then:
+      required:
+        - resets
+        - reset-names
+
 unevaluatedProperties: false
 
 examples:
-- 
2.43.2

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

[Akhil R]

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>
---
 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);
-	else
+	else if (i2c_dev->rst)
 		err = reset_control_reset(i2c_dev->rst);
+	else
+		err = tegra_i2c_master_reset(i2c_dev);
 
 	WARN_ON_ONCE(err);
 
@@ -1467,6 +1489,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 +1514,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 +1539,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 +1564,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 +1589,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 +1614,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 +1639,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,
@@ -1666,7 +1695,7 @@ 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");
+	i2c_dev->rst = devm_reset_control_get_optional_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");
-- 
2.43.2

[PATCH v3 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>
---
 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 22ddbae9d847..b10a4bc9cb34 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1292,17 +1292,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);
 		}
 	}
 
@@ -1312,11 +1304,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;
@@ -1357,13 +1344,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.43.2

Re: [PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[Krzysztof Kozlowski]

On 26/05/2025 07:25, Akhil R wrote:
> Specify the properties which are essential for the Tegra I2C driver to
> function correctly. Though all the existing DT nodes have these
> properties already, it was not mandated by the DT bindings.

I was rather expecting to see explanation why these were missing.

Fixes: f10a9b722f80 ("dt-bindings: i2c: tegra: Convert to json-schema")


> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
> v2->v3:
>   * Updated commit description on the details and fixed indentation
>     issue.
> v1->v2:
>   * Added all required properties
> 
>  .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 23 ++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> index b57ae6963e62..c1d38e6ff7d7 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> @@ -97,7 +97,9 @@ properties:
>  
>    resets:
>      items:
> -      - description: module reset
> +      - description: |

Do not need '|' unless you need to preserve formatting.


> +          Module reset. This property is optional for controllers in Tegra194 and later

Your binding says Tegra210 requires it, but 210 feels like something
later than 194. Maybe that's obvious for people knowing that device?

Anyway, please wrap at 80 (see kernel coding style).

> +          chips where an internal software reset is available as an alternative.
>  
>    reset-names:
>      items:
> @@ -116,6 +118,13 @@ properties:
>        - const: rx
>        - const: tx
>  
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names

dmas, dma-names, power-domains, see TXT binding... or extend commit
description why this should be different comparing to original binding.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[Krzysztof Kozlowski]

On 26/05/2025 07:40, Krzysztof Kozlowski wrote:
> On 26/05/2025 07:25, Akhil R wrote:
>> Specify the properties which are essential for the Tegra I2C driver to
>> function correctly. Though all the existing DT nodes have these
>> properties already, it was not mandated by the DT bindings.
> 
> I was rather expecting to see explanation why these were missing.

To clarify: I meant, explain the bug/mistake. Rest of explanation is ok.

> 
> Fixes: f10a9b722f80 ("dt-bindings: i2c: tegra: Convert to json-schema")
> 
> 


Best regards,
Krzysztof

RE: [PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[Akhil R]

> On 26/05/2025 07:40, Krzysztof Kozlowski wrote:
> > On 26/05/2025 07:25, Akhil R wrote:
> >> Specify the properties which are essential for the Tegra I2C driver
> >> to function correctly. Though all the existing DT nodes have these
> >> properties already, it was not mandated by the DT bindings.
> >
> > I was rather expecting to see explanation why these were missing.
> 
> To clarify: I meant, explain the bug/mistake. Rest of explanation is ok.

Oh. Okay. It would have been just missed when it was converted.
Shall I send a new version mentioning that it was missed initially?
Or would the fixes tag suffice as an explanation?

> 
> >
> > Fixes: f10a9b722f80 ("dt-bindings: i2c: tegra: Convert to
> > json-schema")
> >

Regards,
Akhil

Re: [PATCH v3 1/3] dt-bindings: i2c: nvidia,tegra20-i2c: Specify the required properties

[Krzysztof Kozlowski]

On 02/06/2025 13:10, Akhil R wrote:
>>
>>> +          Module reset. This property is optional for controllers in Tegra194 and later
>>
>> Your binding says Tegra210 requires it, but 210 feels like something
>> later than 194. Maybe that's obvious for people knowing that device?
> 
> Tegra210 is in fact an older device than T194. I will probably mention the chip names
> instead of “later” in the next version.
> 
>>
>> Anyway, please wrap at 80 (see kernel coding style).
> 
> Okay. Will update.
> checkpatch.pl don’t warn now till 100 characters. I thought it was updated.

Checkpatch is a tool, not coding style. Coding style document describes
the coding style.



Best regards,
Krzysztof