* [PATCH v2 2/3] i2c: tegra: make reset an optional property
2025-05-16 12:43 [PATCH v2 1/3] dt-bindings: i2c: Add required properties Akhil R
@ 2025-05-16 12:43 ` Akhil R
2025-05-16 12:43 ` [PATCH v2 3/3] i2c: tegra: Remove dma_sync_*() calls Akhil R
2025-05-16 12:53 ` [PATCH v2 1/3] dt-bindings: i2c: Add required properties Krzysztof Kozlowski
2 siblings, 0 replies; 6+ messages in thread
From: Akhil R @ 2025-05-16 12:43 UTC (permalink / raw)
To: andi.shyti, robh, krzk+dt, onor+dt, thierry.reding, jonathanh,
ldewangan, digetx, p.zabel, linux-i2c, devicetree, linux-tegra,
linux-kernel
Cc: 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] i2c: tegra: Remove dma_sync_*() calls
2025-05-16 12:43 [PATCH v2 1/3] dt-bindings: i2c: Add required properties Akhil R
2025-05-16 12:43 ` [PATCH v2 2/3] i2c: tegra: make reset an optional property Akhil R
@ 2025-05-16 12:43 ` Akhil R
2025-05-16 12:53 ` [PATCH v2 1/3] dt-bindings: i2c: Add required properties Krzysztof Kozlowski
2 siblings, 0 replies; 6+ messages in thread
From: Akhil R @ 2025-05-16 12:43 UTC (permalink / raw)
To: andi.shyti, robh, krzk+dt, onor+dt, thierry.reding, jonathanh,
ldewangan, digetx, p.zabel, linux-i2c, devicetree, linux-tegra,
linux-kernel
Cc: Akhil R, Robin Murphy
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] dt-bindings: i2c: Add required properties
2025-05-16 12:43 [PATCH v2 1/3] dt-bindings: i2c: Add required properties Akhil R
2025-05-16 12:43 ` [PATCH v2 2/3] i2c: tegra: make reset an optional property Akhil R
2025-05-16 12:43 ` [PATCH v2 3/3] i2c: tegra: Remove dma_sync_*() calls Akhil R
@ 2025-05-16 12:53 ` Krzysztof Kozlowski
2025-05-16 13:59 ` Jon Hunter
2 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-16 12:53 UTC (permalink / raw)
To: Akhil R, andi.shyti, robh, krzk+dt, thierry.reding, jonathanh,
ldewangan, digetx, p.zabel, linux-i2c, devicetree, linux-tegra,
linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread