Add support for SW mutex register introduced in Tegra264 to provide
an option to share the interface between multiple firmwares and/or
VMs.
However, the hardware does not ensure any protection based on the
values. The driver/firmware should honor the peer who already holds
the mutex.
Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
v4 -> v6:
* Guard tegra_i2c_mutex_lock() and tegra_i2c_mutex_unlock() to
ensure that they are called on platforms which support SW
mutex.
v3 -> v4:
* Update timeout logic of tegra_i2c_mutex_lock() to use
read_poll_timeout APIs for improving timeout logic.
* Add tegra_i2c_mutex_acquired() to check if mutex is acquired
or not.
* Rename I2C_SW_MUTEX_ID as I2C_SW_MUTEX_ID_CCPLEX.
* Function tegra_i2c_poll_register() was moved unnecessarily, it
has now been moved to its original location.
* Use tegra_i2c_mutex_lock/unlock APIs in the tegra_i2c_xfer()
function. This ensures proper propagation of error in case
mutex lock fails.
Please note that as the function tegra_i2c_xfer() is
already guarded by the bus lock operation there is no need of
additional lock for the tegra_i2c_mutex_lock/unlock APIs.
v2 -> v3:
* Update tegra_i2c_mutex_trylock and tegra_i2c_mutex_unlock to
use readl and writel APIs instead of i2c_readl and i2c_writel
which use relaxed APIs.
* Use dev_warn instead of WARN_ON if mutex lock/unlock fails.
v1 -> v2:
* Fixed typos.
* Fix tegra_i2c_mutex_lock() logic.
* Add a timeout in tegra_i2c_mutex_lock() instead of polling for
mutex indefinitely.
---
drivers/i2c/busses/i2c-tegra.c | 97 ++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 20d5c8a6925d..88ee27f90526 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -137,6 +137,14 @@
#define I2C_MASTER_RESET_CNTRL 0x0a8
+#define I2C_SW_MUTEX 0x0ec
+#define I2C_SW_MUTEX_REQUEST GENMASK(3, 0)
+#define I2C_SW_MUTEX_GRANT GENMASK(7, 4)
+#define I2C_SW_MUTEX_ID_CCPLEX 9
+
+/* SW mutex acquire timeout value in microseconds. */
+#define I2C_SW_MUTEX_TIMEOUT_US (25 * USEC_PER_MSEC)
+
/* configuration load timeout in microseconds */
#define I2C_CONFIG_LOAD_TIMEOUT 1000000
@@ -210,6 +218,7 @@ enum msg_end_type {
* @has_interface_timing_reg: Has interface timing register to program the tuned
* timing settings.
* @has_hs_mode_support: Has support for high speed (HS) mode transfers.
+ * @has_mutex: Has mutex register for mutual exclusion with other firmwares or VMs.
*/
struct tegra_i2c_hw_feature {
bool has_continue_xfer_support;
@@ -237,6 +246,7 @@ struct tegra_i2c_hw_feature {
u32 setup_hold_time_hs_mode;
bool has_interface_timing_reg;
bool has_hs_mode_support;
+ bool has_mutex;
};
/**
@@ -381,6 +391,73 @@ static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data,
readsl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len);
}
+static int tegra_i2c_mutex_acquired(struct tegra_i2c_dev *i2c_dev)
+{
+ unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
+ u32 val, id;
+
+ val = readl(i2c_dev->base + reg);
+ id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
+
+ if (id != I2C_SW_MUTEX_ID_CCPLEX)
+ return 0;
+
+ return 1;
+}
+
+static int tegra_i2c_mutex_trylock(struct tegra_i2c_dev *i2c_dev)
+{
+ unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
+ u32 val, id;
+
+ val = readl(i2c_dev->base + reg);
+ id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
+ if (id != 0 && id != I2C_SW_MUTEX_ID_CCPLEX)
+ return 0;
+
+ val = FIELD_PREP(I2C_SW_MUTEX_REQUEST, I2C_SW_MUTEX_ID_CCPLEX);
+ writel(val, i2c_dev->base + reg);
+
+ return tegra_i2c_mutex_acquired(i2c_dev);
+}
+
+static int tegra_i2c_mutex_lock(struct tegra_i2c_dev *i2c_dev)
+{
+ int locked;
+ int ret;
+
+ if (i2c_dev->atomic_mode)
+ ret = read_poll_timeout_atomic(tegra_i2c_mutex_trylock, locked, locked,
+ USEC_PER_MSEC, I2C_SW_MUTEX_TIMEOUT_US,
+ false, i2c_dev);
+ else
+ ret = read_poll_timeout(tegra_i2c_mutex_trylock, locked, locked, USEC_PER_MSEC,
+ I2C_SW_MUTEX_TIMEOUT_US, false, i2c_dev);
+
+ if (!tegra_i2c_mutex_acquired(i2c_dev))
+ dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
+
+ return ret;
+}
+
+static int tegra_i2c_mutex_unlock(struct tegra_i2c_dev *i2c_dev)
+{
+ unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
+ u32 val, id;
+
+ val = readl(i2c_dev->base + reg);
+
+ id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
+ if (id && id != I2C_SW_MUTEX_ID_CCPLEX) {
+ dev_warn(i2c_dev->dev, "unable to unlock mutex, mutex is owned by: %u\n", id);
+ return -EPERM;
+ }
+
+ writel(0, i2c_dev->base + reg);
+
+ return 0;
+}
+
static void tegra_i2c_mask_irq(struct tegra_i2c_dev *i2c_dev, u32 mask)
{
u32 int_mask;
@@ -1422,6 +1499,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
return ret;
}
+
+ if (i2c_dev->hw->has_mutex) {
+ ret = tegra_i2c_mutex_lock(i2c_dev);
+ if (ret)
+ return ret;
+ }
+
for (i = 0; i < num; i++) {
enum msg_end_type end_type = MSG_END_STOP;
@@ -1451,6 +1535,12 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
break;
}
+ if (i2c_dev->hw->has_mutex) {
+ ret = tegra_i2c_mutex_unlock(i2c_dev);
+ if (ret)
+ return ret;
+ }
+
pm_runtime_put(i2c_dev->dev);
return ret ?: i;
@@ -1527,6 +1617,7 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
.setup_hold_time_hs_mode = 0x0,
.has_interface_timing_reg = false,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
@@ -1553,6 +1644,7 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
.setup_hold_time_hs_mode = 0x0,
.has_interface_timing_reg = false,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
@@ -1579,6 +1671,7 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
.setup_hold_time_hs_mode = 0x0,
.has_interface_timing_reg = false,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
@@ -1605,6 +1698,7 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
.setup_hold_time_hs_mode = 0x0,
.has_interface_timing_reg = true,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
@@ -1631,6 +1725,7 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
.setup_hold_time_hs_mode = 0,
.has_interface_timing_reg = true,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
@@ -1657,6 +1752,7 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
.setup_hold_time_hs_mode = 0,
.has_interface_timing_reg = true,
.has_hs_mode_support = false,
+ .has_mutex = false,
};
static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
@@ -1685,6 +1781,7 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
.setup_hold_time_hs_mode = 0x090909,
.has_interface_timing_reg = true,
.has_hs_mode_support = true,
+ .has_mutex = false,
};
static const struct of_device_id tegra_i2c_of_match[] = {
--
2.43.0Hi Kartik,
On Thu, Aug 28, 2025 at 11:29:32AM +0530, Kartik Rajput wrote:
> Add support for SW mutex register introduced in Tegra264 to provide
> an option to share the interface between multiple firmwares and/or
> VMs.
You could add a short description on how to use the mutex
register here.
> However, the hardware does not ensure any protection based on the
> values. The driver/firmware should honor the peer who already holds
> the mutex.
>
> Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
...
> @@ -381,6 +391,73 @@ static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data,
> readsl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len);
> }
>
> +static int tegra_i2c_mutex_acquired(struct tegra_i2c_dev *i2c_dev)
this is a bool function.
> +{
> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
> + u32 val, id;
> +
> + val = readl(i2c_dev->base + reg);
> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
> +
> + if (id != I2C_SW_MUTEX_ID_CCPLEX)
> + return 0;
> +
> + return 1;
return id != I2C_SW_MUTEX_ID_CCPLEX;
> +}
> +
> +static int tegra_i2c_mutex_trylock(struct tegra_i2c_dev *i2c_dev)
I think this can be bool.
> +{
> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
> + u32 val, id;
> +
> + val = readl(i2c_dev->base + reg);
> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
> + if (id != 0 && id != I2C_SW_MUTEX_ID_CCPLEX)
> + return 0;
> +
> + val = FIELD_PREP(I2C_SW_MUTEX_REQUEST, I2C_SW_MUTEX_ID_CCPLEX);
> + writel(val, i2c_dev->base + reg);
> +
> + return tegra_i2c_mutex_acquired(i2c_dev);
> +}
> +
> +static int tegra_i2c_mutex_lock(struct tegra_i2c_dev *i2c_dev)
> +{
> + int locked;
I guess this can be bool.
> + int ret;
> +
> + if (i2c_dev->atomic_mode)
> + ret = read_poll_timeout_atomic(tegra_i2c_mutex_trylock, locked, locked,
> + USEC_PER_MSEC, I2C_SW_MUTEX_TIMEOUT_US,
> + false, i2c_dev);
> + else
> + ret = read_poll_timeout(tegra_i2c_mutex_trylock, locked, locked, USEC_PER_MSEC,
> + I2C_SW_MUTEX_TIMEOUT_US, false, i2c_dev);
> +
> + if (!tegra_i2c_mutex_acquired(i2c_dev))
> + dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
I would try a few times before giving up.
Besides, is there a chance where ret is '0' and the mutex is not
acquired? If so, we are not signalling error if the mutex is not
acquired, but I think we should.
I would do:
if (...)
ret = ...
else
ret = ...
if (ret)
return ret;
if (!tegra_i2c_mutex_acquired(i2c_dev)) {
dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
return -ESOMETHING;
}
return 0;
Makes sense?
> +
> + return ret;
> +}
> +
> +static int tegra_i2c_mutex_unlock(struct tegra_i2c_dev *i2c_dev)
> +{
> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
> + u32 val, id;
> +
> + val = readl(i2c_dev->base + reg);
> +
> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
> + if (id && id != I2C_SW_MUTEX_ID_CCPLEX) {
> + dev_warn(i2c_dev->dev, "unable to unlock mutex, mutex is owned by: %u\n", id);
> + return -EPERM;
I would try a few times before giving up.
> + }
> +
> + writel(0, i2c_dev->base + reg);
> +
> + return 0;
> +}
> +
> static void tegra_i2c_mask_irq(struct tegra_i2c_dev *i2c_dev, u32 mask)
> {
> u32 int_mask;
> @@ -1422,6 +1499,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
> return ret;
> }
>
> +
no need for this extra blank line.
> + if (i2c_dev->hw->has_mutex) {
I would put this check in tegra_i2c_mutex_lock() and _unlock() in
order to avoid two level indentation here.
> + ret = tegra_i2c_mutex_lock(i2c_dev);
> + if (ret)
> + return ret;
> + }
> +
> for (i = 0; i < num; i++) {
> enum msg_end_type end_type = MSG_END_STOP;
>
> @@ -1451,6 +1535,12 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
> break;
> }
>
> + if (i2c_dev->hw->has_mutex) {
> + ret = tegra_i2c_mutex_unlock(i2c_dev);
> + if (ret)
> + return ret;
We are skipping pm_runtime_put(), though.
Thanks,
Andi
> + }
> +
> pm_runtime_put(i2c_dev->dev);
>
> return ret ?: i;
Hi Andi,
Thanks for reviewing the patch!
On 05/09/25 04:58, Andi Shyti wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Kartik,
>
> On Thu, Aug 28, 2025 at 11:29:32AM +0530, Kartik Rajput wrote:
>> Add support for SW mutex register introduced in Tegra264 to provide
>> an option to share the interface between multiple firmwares and/or
>> VMs.
>
> You could add a short description on how to use the mutex
> register here.
>
Ack.
>> However, the hardware does not ensure any protection based on the
>> values. The driver/firmware should honor the peer who already holds
>> the mutex.
>>
>> Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>
> ...
>
>> @@ -381,6 +391,73 @@ static void i2c_readsl(struct tegra_i2c_dev *i2c_dev, void *data,
>> readsl(i2c_dev->base + tegra_i2c_reg_addr(i2c_dev, reg), data, len);
>> }
>>
>> +static int tegra_i2c_mutex_acquired(struct tegra_i2c_dev *i2c_dev)
>
> this is a bool function.
>
Ack.
>> +{
>> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
>> + u32 val, id;
>> +
>> + val = readl(i2c_dev->base + reg);
>> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
>> +
>> + if (id != I2C_SW_MUTEX_ID_CCPLEX)
>> + return 0;
>> +
>> + return 1;
>
> return id != I2C_SW_MUTEX_ID_CCPLEX;
>
Ack.
>> +}
>> +
>> +static int tegra_i2c_mutex_trylock(struct tegra_i2c_dev *i2c_dev)
>
> I think this can be bool.
>
Ack.
>> +{
>> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
>> + u32 val, id;
>> +
>> + val = readl(i2c_dev->base + reg);
>> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
>> + if (id != 0 && id != I2C_SW_MUTEX_ID_CCPLEX)
>> + return 0;
>> +
>> + val = FIELD_PREP(I2C_SW_MUTEX_REQUEST, I2C_SW_MUTEX_ID_CCPLEX);
>> + writel(val, i2c_dev->base + reg);
>> +
>> + return tegra_i2c_mutex_acquired(i2c_dev);
>> +}
>> +
>> +static int tegra_i2c_mutex_lock(struct tegra_i2c_dev *i2c_dev)
>> +{
>> + int locked;
>
> I guess this can be bool.
>
Ack.
>> + int ret;
>> +
>> + if (i2c_dev->atomic_mode)
>> + ret = read_poll_timeout_atomic(tegra_i2c_mutex_trylock, locked, locked,
>> + USEC_PER_MSEC, I2C_SW_MUTEX_TIMEOUT_US,
>> + false, i2c_dev);
>> + else
>> + ret = read_poll_timeout(tegra_i2c_mutex_trylock, locked, locked, USEC_PER_MSEC,
>> + I2C_SW_MUTEX_TIMEOUT_US, false, i2c_dev);
>> +
>> + if (!tegra_i2c_mutex_acquired(i2c_dev))
>> + dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
>
> I would try a few times before giving up.
>
Yes, we are retrying with read_poll_timeout_* calls.
> Besides, is there a chance where ret is '0' and the mutex is not
In case of failure ret should always be set to -ETIMEDOUT.
> acquired? If so, we are not signalling error if the mutex is not
> acquired, but I think we should.
>
> I would do:
>
> if (...)
> ret = ...
> else
> ret = ...
>
> if (ret)
> return ret;
>
> if (!tegra_i2c_mutex_acquired(i2c_dev)) {
> dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
> return -ESOMETHING;
> }
>
> return 0;
>
> Makes sense?
I agree, we can simplify this logic by just checking the value of ret instead
of calling tegra_i2c_mutex_acquired() again as tegra_i2c_mutex_trylock() already
checks if we have acquired the mutex or not.
...
if (ret)
dev_warn(i2c_dev->dev, "failed to acquire mutex\n");
return ret;
}
>
>> +
>> + return ret;
>> +}
>> +
>> +static int tegra_i2c_mutex_unlock(struct tegra_i2c_dev *i2c_dev)
>> +{
>> + unsigned int reg = tegra_i2c_reg_addr(i2c_dev, I2C_SW_MUTEX);
>> + u32 val, id;
>> +
>> + val = readl(i2c_dev->base + reg);
>> +
>> + id = FIELD_GET(I2C_SW_MUTEX_GRANT, val);
>> + if (id && id != I2C_SW_MUTEX_ID_CCPLEX) {
>> + dev_warn(i2c_dev->dev, "unable to unlock mutex, mutex is owned by: %u\n", id);
>> + return -EPERM;
>
> I would try a few times before giving up.
>
Unlocking the mutex should fail if unlock is called and CCPLEX is not the owner.
In that case no need to retry.
>> + }
>> +
>> + writel(0, i2c_dev->base + reg);
>> +
>> + return 0;
>> +}
>> +
>> static void tegra_i2c_mask_irq(struct tegra_i2c_dev *i2c_dev, u32 mask)
>> {
>> u32 int_mask;
>> @@ -1422,6 +1499,13 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>> return ret;
>> }
>>
>> +
>
> no need for this extra blank line.
>
Ack.
>> + if (i2c_dev->hw->has_mutex) {
>
> I would put this check in tegra_i2c_mutex_lock() and _unlock() in
> order to avoid two level indentation here.
>
Ack. It will look cleaner if the check is moved inside the lock and unlock functions.
>> + ret = tegra_i2c_mutex_lock(i2c_dev);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> for (i = 0; i < num; i++) {
>> enum msg_end_type end_type = MSG_END_STOP;
>>
>> @@ -1451,6 +1535,12 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
>> break;
>> }
>>
>> + if (i2c_dev->hw->has_mutex) {
>> + ret = tegra_i2c_mutex_unlock(i2c_dev);
>> + if (ret)
>> + return ret;
>
> We are skipping pm_runtime_put(), though.
>
> Thanks,
> Andi
>
Yes, we skip calling pm_runtime_put() if unlocking fails. I will fix this in the next revision.
>> + }
>> +
>> pm_runtime_put(i2c_dev->dev);
>>
>> return ret ?: i;
Thanks & Regards,
Kartik
© 2016 - 2026 Red Hat, Inc.