[PATCH v2 1/3] dt-bindings: i2c: Add required properties

[PATCH v2 1/3] dt-bindings: i2c: Add required properties

[Akhil R]

Add required DT properties for Tegra I2C controllers.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
v1->v2:
  * Added all required properties

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

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
index 19aefc022c8b..0717f2304cfc 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
@@ -118,6 +118,13 @@ properties:
       - const: rx
       - const: tx
 
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
 allOf:
   - $ref: /schemas/i2c/i2c-controller.yaml
   - if:
@@ -171,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 v2 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>
---
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);
-	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 v2 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>
---
v1->v2: No changes

Related thread - https://lore.kernel.org/all/acdbf49c-1a73-a0b9-a10d-42d544be3117@arm.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 v2 1/3] dt-bindings: i2c: Add required properties

[Krzysztof Kozlowski]

On 16/05/2025 14:43, Akhil R wrote:
> Add required DT properties for Tegra I2C controllers.

Why? Required by whom/what? Some context or any justification is needed
here. Are you breaking the ABI (means: prove that you are not).

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
> v1->v2:
>   * Added all required properties
> 
>  .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> index 19aefc022c8b..0717f2304cfc 100644
> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> @@ -118,6 +118,13 @@ properties:
>        - const: rx
>        - const: tx
>  
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
>  allOf:
>    - $ref: /schemas/i2c/i2c-controller.yaml
>    - if:
> @@ -171,6 +178,18 @@ allOf:
>        properties:
>          power-domains: false
>  
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - nvidia,tegra194-i2c
> +    then:
> +        required:

Never tested, so quite dissapointing.

Test your patches before sending, not after.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: i2c: Add required properties

[Jon Hunter]

On 16/05/2025 13:53, Krzysztof Kozlowski wrote:
> On 16/05/2025 14:43, Akhil R wrote:
>> Add required DT properties for Tegra I2C controllers.
> 
> Why? Required by whom/what? Some context or any justification is needed
> here. Are you breaking the ABI (means: prove that you are not).
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
>>
>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>> ---
>> v1->v2:
>>    * Added all required properties
>>
>>   .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
>> index 19aefc022c8b..0717f2304cfc 100644
>> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
>> @@ -118,6 +118,13 @@ properties:
>>         - const: rx
>>         - const: tx
>>   
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +
>>   allOf:
>>     - $ref: /schemas/i2c/i2c-controller.yaml
>>     - if:
>> @@ -171,6 +178,18 @@ allOf:
>>         properties:
>>           power-domains: false
>>   
>> +  - if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              enum:
>> +                - nvidia,tegra194-i2c
>> +    then:
>> +        required:
> 
> Never tested, so quite dissapointing.
> 
> Test your patches before sending, not after.

ACK. Clearly we need to do a better job here. It is noted.

Thanks
Jon

-- 
nvpublic

RE: [PATCH v2 1/3] dt-bindings: i2c: Add required properties

[Akhil R]

> On 16/05/2025 13:53, Krzysztof Kozlowski wrote:
> > On 16/05/2025 14:43, Akhil R wrote:
> >> Add required DT properties for Tegra I2C controllers.
> >
> > Why? Required by whom/what? Some context or any justification is
> > needed here. Are you breaking the ABI (means: prove that you are not).
> >
> > Please use subject prefixes matching the subsystem. You can get them
> > for example with `git log --oneline -- DIRECTORY_OR_FILE` on the
> > directory your patch is touching. For bindings, the preferred subjects
> > are explained here:
> > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-
> > patches.html#i-for-patch-submitters
> >
> >>
> >> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> >> ---
> >> v1->v2:
> >>    * Added all required properties
> >>
> >>   .../bindings/i2c/nvidia,tegra20-i2c.yaml      | 19 +++++++++++++++++++
> >>   1 file changed, 19 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> >> b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> >> index 19aefc022c8b..0717f2304cfc 100644
> >> --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> >> +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
> >> @@ -118,6 +118,13 @@ properties:
> >>         - const: rx
> >>         - const: tx
> >>
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +  - interrupts
> >> +  - clocks
> >> +  - clock-names
> >> +
> >>   allOf:
> >>     - $ref: /schemas/i2c/i2c-controller.yaml
> >>     - if:
> >> @@ -171,6 +178,18 @@ allOf:
> >>         properties:
> >>           power-domains: false
> >>
> >> +  - if:
> >> +      not:
> >> +        properties:
> >> +          compatible:
> >> +            contains:
> >> +              enum:
> >> +                - nvidia,tegra194-i2c
> >> +    then:
> >> +        required:
> >
> > Never tested, so quite dissapointing.
> >
> > Test your patches before sending, not after.
> 
> ACK. Clearly we need to do a better job here. It is noted.

ACK to the comments. Updated the commit description and fixed the indentation
issue and sent out a new version.
https://lore.kernel.org/lkml/20250526052553.42766-1-akhilrajeev@nvidia.com/T/#t

I ran the below tests, did not see any error or warning with the new version.

make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml
make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.yaml

Regards,
Akhil