| Message ID | 20220720133758.11481-2-asmaa@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | UBUNTU: SAUCE: i2c-mlxbf.c: support lock mechanism | expand |
There is an exit point within the modified function after the lock has been set but the lock isn't being released if we return at this point. This return is within the for loop following the setting of the lock bit. On Wed, Jul 20, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote: > Buglink: https://bugs.launchpad.net/bugs/1981105 > > Support the I2C lock mechanism, otherwise there could be > unexpected behavior when an i2c bus is accessed by > several entities like the linux driver, ATF driver and UEFI driver. > > Make sure to pick up the ATF/UEFI image to accompany this change > because at boot time ATF will ensure that the lock is released. > > Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> > --- > drivers/i2c/busses/i2c-mlxbf.c | 40 ++++++++++++++++++++++++++++++---- > 1 file changed, 36 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-mlxbf.c > b/drivers/i2c/busses/i2c-mlxbf.c > index 0ac9e70244a7..b951443b91f8 100644 > --- a/drivers/i2c/busses/i2c-mlxbf.c > +++ b/drivers/i2c/busses/i2c-mlxbf.c > @@ -340,7 +340,8 @@ enum { > * Timeout is given in microsends. Note also that timeout handling is not > * exact. > */ > -#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ > +#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ > +#define SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */ > > /* Encapsulates timing parameters */ > struct mlx_i2c_timings { > @@ -573,6 +574,25 @@ static bool mlx_smbus_master_wait_for_idle(struct > mlx_i2c_priv *priv) > return false; > } > > +/* > + * wait for the lock to be released before acquiring it. > + */ > +static bool mlx_smbus_master_lock(struct mlx_i2c_priv *priv) > +{ > + if (mlx_smbus_poll(priv->smbus->io, SMBUS_MASTER_GW, > + 1 << MASTER_LOCK_BIT_OFF, true, > + SMBUS_LOCK_POLL_TIMEOUT)) > + return true; > + > + return false; > +} > + > +static void mlx_smbus_master_unlock(struct mlx_i2c_priv *priv) > +{ > + /* Clear the gw to clear the lock */ > + writel(0, priv->smbus->io + SMBUS_MASTER_GW); > +} > + > /* > * Poll SMBus master status and return transaction status, > * i.e. whether succeeded or failed. I2C and SMBus fault codes > @@ -744,9 +764,17 @@ static int mlx_smbus_start_transaction(struct > mlx_i2c_priv *priv, > slave = request->slave & 0x7f; > addr = slave << 1; > > - /* First of all, check whether the HW is idle */ > - if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) > + /* Try to acquire the smbus gw lock before any reads of the GW > register since > + * a read sets the lock. > + */ > + if (WARN_ON(!mlx_smbus_master_lock(priv))) > + return -EBUSY; > + > + /* Check whether the HW is idle */ > + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) { > + mlx_smbus_master_unlock(priv); > return -EBUSY; > + } > > /* Set first byte */ > data_desc[data_idx++] = addr; > @@ -802,8 +830,10 @@ static int mlx_smbus_start_transaction(struct > mlx_i2c_priv *priv, > if (write_en) { > ret = mlx_smbus_enable(priv, slave, write_len, block_en, > pec_en, 0); > - if (ret != 0) > + if (ret) { > + mlx_smbus_master_unlock(priv); > return ret; > + } > } > > if (read_en) { > @@ -830,6 +860,8 @@ static int mlx_smbus_start_transaction(struct > mlx_i2c_priv *priv, > SMBUS_MASTER_FSM_PS_STATE_MASK); > } > > + mlx_smbus_master_unlock(priv); > + > return ret; > } > > -- > 2.30.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team >
I am not sure I understand the question. I addressed Tim's comment and released the lock in the second loop exit point.
+ if (WARN_ON(!mlx_smbus_master_lock(priv)))
+ return -EBUSY;
+
+ /* Check whether the HW is idle */
+ if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) {
+ mlx_smbus_master_unlock(priv);
return -EBUSY;
+ }
From: Zachary Tahenakos <zachary.tahenakos@canonical.com>
Sent: Thursday, July 21, 2022 9:35 AM
To: Asmaa Mnebhi <asmaa@nvidia.com>
Cc: kernel-team@lists.ubuntu.com
Subject: NACK: [SRU][F:linux-bluefield][PATCH v3 1/1] UBUNTU: SAUCE: i2c-mlxbf.c: support lock mechanism
There is an exit point within the modified function after the lock has been set but the lock isn't being released if we return at this point. This return is within the for loop following the setting of the lock bit.
On Wed, Jul 20, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>> wrote:
Buglink: https://bugs.launchpad.net/bugs/1981105<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1981105&data=05%7C01%7Casmaa%40nvidia.com%7Cc84f4aa7261740ed1bc308da6b1dda64%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940073221061486%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jQJIAEetDruX%2FDpXESCdyswqH4dM8nhuhmQ%2F50CKDmE%3D&reserved=0>
Support the I2C lock mechanism, otherwise there could be
unexpected behavior when an i2c bus is accessed by
several entities like the linux driver, ATF driver and UEFI driver.
Make sure to pick up the ATF/UEFI image to accompany this change
because at boot time ATF will ensure that the lock is released.
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>>
---
drivers/i2c/busses/i2c-mlxbf.c | 40 ++++++++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 0ac9e70244a7..b951443b91f8 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -340,7 +340,8 @@ enum {
* Timeout is given in microsends. Note also that timeout handling is not
* exact.
*/
-#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */
+#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */
+#define SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */
/* Encapsulates timing parameters */
struct mlx_i2c_timings {
@@ -573,6 +574,25 @@ static bool mlx_smbus_master_wait_for_idle(struct mlx_i2c_priv *priv)
return false;
}
+/*
+ * wait for the lock to be released before acquiring it.
+ */
+static bool mlx_smbus_master_lock(struct mlx_i2c_priv *priv)
+{
+ if (mlx_smbus_poll(priv->smbus->io, SMBUS_MASTER_GW,
+ 1 << MASTER_LOCK_BIT_OFF, true,
+ SMBUS_LOCK_POLL_TIMEOUT))
+ return true;
+
+ return false;
+}
+
+static void mlx_smbus_master_unlock(struct mlx_i2c_priv *priv)
+{
+ /* Clear the gw to clear the lock */
+ writel(0, priv->smbus->io + SMBUS_MASTER_GW);
+}
+
/*
* Poll SMBus master status and return transaction status,
* i.e. whether succeeded or failed. I2C and SMBus fault codes
@@ -744,9 +764,17 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv,
slave = request->slave & 0x7f;
addr = slave << 1;
- /* First of all, check whether the HW is idle */
- if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv)))
+ /* Try to acquire the smbus gw lock before any reads of the GW register since
+ * a read sets the lock.
+ */
+ if (WARN_ON(!mlx_smbus_master_lock(priv)))
+ return -EBUSY;
+
+ /* Check whether the HW is idle */
+ if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) {
+ mlx_smbus_master_unlock(priv);
return -EBUSY;
+ }
/* Set first byte */
data_desc[data_idx++] = addr;
@@ -802,8 +830,10 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv,
if (write_en) {
ret = mlx_smbus_enable(priv, slave, write_len, block_en,
pec_en, 0);
- if (ret != 0)
+ if (ret) {
+ mlx_smbus_master_unlock(priv);
return ret;
+ }
}
if (read_en) {
@@ -830,6 +860,8 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv,
SMBUS_MASTER_FSM_PS_STATE_MASK);
}
+ mlx_smbus_master_unlock(priv);
+
return ret;
}
--
2.30.1
--
kernel-team mailing list
kernel-team@lists.ubuntu.com<mailto:kernel-team@lists.ubuntu.com>
https://lists.ubuntu.com/mailman/listinfo/kernel-team<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.ubuntu.com%2Fmailman%2Flistinfo%2Fkernel-team&data=05%7C01%7Casmaa%40nvidia.com%7Cc84f4aa7261740ed1bc308da6b1dda64%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940073221061486%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HyHm5a7B%2F%2F%2F3OW2UxXLlS3schQSsZ%2BZ4XWAZj3XSgcE%3D&reserved=0>
I'm talking about this here:
if (flags & I2C_F_WRITE) {
write_en = 1;
write_len += operation->length;
if (data_idx + operation->length >
MASTER_DATA_DESC_SIZE)
return -ENOBUFS;
memcpy(data_desc + data_idx,
operation->buffer, operation->length);
data_idx += operation->length;
}
This is part of the for loop shortly after setting the lock. There is a
conditional return that does not clear the lock before returning.
On Thu, Jul 21, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
> I am not sure I understand the question. I addressed Tim’s comment and
> released the lock in the second loop exit point.
>
>
> + if (WARN_ON(!mlx_smbus_master_lock(priv)))
> + return -EBUSY;
> +
> + /* Check whether the HW is idle */
> + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) {
> + mlx_smbus_master_unlock(priv);
> return -EBUSY;
> + }
>
>
> *From:* Zachary Tahenakos <zachary.tahenakos@canonical.com>
> *Sent:* Thursday, July 21, 2022 9:35 AM
> *To:* Asmaa Mnebhi <asmaa@nvidia.com>
> *Cc:* kernel-team@lists.ubuntu.com
> *Subject:* NACK: [SRU][F:linux-bluefield][PATCH v3 1/1] UBUNTU: SAUCE:
> i2c-mlxbf.c: support lock mechanism
>
>
>
> There is an exit point within the modified function after the lock has
> been set but the lock isn't being released if we return at this point. This
> return is within the for loop following the setting of the lock bit.
>
>
>
> On Wed, Jul 20, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com> wrote:
>
> Buglink: https://bugs.launchpad.net/bugs/1981105
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1981105&data=05%7C01%7Casmaa%40nvidia.com%7Cc84f4aa7261740ed1bc308da6b1dda64%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940073221061486%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jQJIAEetDruX%2FDpXESCdyswqH4dM8nhuhmQ%2F50CKDmE%3D&reserved=0>
>
> Support the I2C lock mechanism, otherwise there could be
> unexpected behavior when an i2c bus is accessed by
> several entities like the linux driver, ATF driver and UEFI driver.
>
> Make sure to pick up the ATF/UEFI image to accompany this change
> because at boot time ATF will ensure that the lock is released.
>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> ---
> drivers/i2c/busses/i2c-mlxbf.c | 40 ++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mlxbf.c
> b/drivers/i2c/busses/i2c-mlxbf.c
> index 0ac9e70244a7..b951443b91f8 100644
> --- a/drivers/i2c/busses/i2c-mlxbf.c
> +++ b/drivers/i2c/busses/i2c-mlxbf.c
> @@ -340,7 +340,8 @@ enum {
> * Timeout is given in microsends. Note also that timeout handling is not
> * exact.
> */
> -#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */
> +#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */
> +#define SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */
>
> /* Encapsulates timing parameters */
> struct mlx_i2c_timings {
> @@ -573,6 +574,25 @@ static bool mlx_smbus_master_wait_for_idle(struct
> mlx_i2c_priv *priv)
> return false;
> }
>
> +/*
> + * wait for the lock to be released before acquiring it.
> + */
> +static bool mlx_smbus_master_lock(struct mlx_i2c_priv *priv)
> +{
> + if (mlx_smbus_poll(priv->smbus->io, SMBUS_MASTER_GW,
> + 1 << MASTER_LOCK_BIT_OFF, true,
> + SMBUS_LOCK_POLL_TIMEOUT))
> + return true;
> +
> + return false;
> +}
> +
> +static void mlx_smbus_master_unlock(struct mlx_i2c_priv *priv)
> +{
> + /* Clear the gw to clear the lock */
> + writel(0, priv->smbus->io + SMBUS_MASTER_GW);
> +}
> +
> /*
> * Poll SMBus master status and return transaction status,
> * i.e. whether succeeded or failed. I2C and SMBus fault codes
> @@ -744,9 +764,17 @@ static int mlx_smbus_start_transaction(struct
> mlx_i2c_priv *priv,
> slave = request->slave & 0x7f;
> addr = slave << 1;
>
> - /* First of all, check whether the HW is idle */
> - if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv)))
> + /* Try to acquire the smbus gw lock before any reads of the GW
> register since
> + * a read sets the lock.
> + */
> + if (WARN_ON(!mlx_smbus_master_lock(priv)))
> + return -EBUSY;
> +
> + /* Check whether the HW is idle */
> + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) {
> + mlx_smbus_master_unlock(priv);
> return -EBUSY;
> + }
>
> /* Set first byte */
> data_desc[data_idx++] = addr;
> @@ -802,8 +830,10 @@ static int mlx_smbus_start_transaction(struct
> mlx_i2c_priv *priv,
> if (write_en) {
> ret = mlx_smbus_enable(priv, slave, write_len, block_en,
> pec_en, 0);
> - if (ret != 0)
> + if (ret) {
> + mlx_smbus_master_unlock(priv);
> return ret;
> + }
> }
>
> if (read_en) {
> @@ -830,6 +860,8 @@ static int mlx_smbus_start_transaction(struct
> mlx_i2c_priv *priv,
> SMBUS_MASTER_FSM_PS_STATE_MASK);
> }
>
> + mlx_smbus_master_unlock(priv);
> +
> return ret;
> }
>
> --
> 2.30.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.ubuntu.com%2Fmailman%2Flistinfo%2Fkernel-team&data=05%7C01%7Casmaa%40nvidia.com%7Cc84f4aa7261740ed1bc308da6b1dda64%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940073221061486%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HyHm5a7B%2F%2F%2F3OW2UxXLlS3schQSsZ%2BZ4XWAZj3XSgcE%3D&reserved=0>
>
>
Thanks! From: Zachary Tahenakos <zachary.tahenakos@canonical.com> Sent: Thursday, July 21, 2022 9:46 AM To: Asmaa Mnebhi <asmaa@nvidia.com> Cc: kernel-team@lists.ubuntu.com Subject: Re: NACK: [SRU][F:linux-bluefield][PATCH v3 1/1] UBUNTU: SAUCE: i2c-mlxbf.c: support lock mechanism I'm talking about this here: if (flags & I2C_F_WRITE) { write_en = 1; write_len += operation->length; if (data_idx + operation->length > MASTER_DATA_DESC_SIZE) return -ENOBUFS; memcpy(data_desc + data_idx, operation->buffer, operation->length); data_idx += operation->length; } This is part of the for loop shortly after setting the lock. There is a conditional return that does not clear the lock before returning. On Thu, Jul 21, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>> wrote: I am not sure I understand the question. I addressed Tim's comment and released the lock in the second loop exit point. + if (WARN_ON(!mlx_smbus_master_lock(priv))) + return -EBUSY; + + /* Check whether the HW is idle */ + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) { + mlx_smbus_master_unlock(priv); return -EBUSY; + } From: Zachary Tahenakos <zachary.tahenakos@canonical.com<mailto:zachary.tahenakos@canonical.com>> Sent: Thursday, July 21, 2022 9:35 AM To: Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>> Cc: kernel-team@lists.ubuntu.com<mailto:kernel-team@lists.ubuntu.com> Subject: NACK: [SRU][F:linux-bluefield][PATCH v3 1/1] UBUNTU: SAUCE: i2c-mlxbf.c: support lock mechanism There is an exit point within the modified function after the lock has been set but the lock isn't being released if we return at this point. This return is within the for loop following the setting of the lock bit. On Wed, Jul 20, 2022 at 9:38 AM Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>> wrote: Buglink: https://bugs.launchpad.net/bugs/1981105<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1981105&data=05%7C01%7Casmaa%40nvidia.com%7C5a683fcac9bb42a6467308da6b1f5841%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940079625172240%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ist2fYst1b773Al989RgzHHi2RLg3MX0WIke7Buyr6A%3D&reserved=0> Support the I2C lock mechanism, otherwise there could be unexpected behavior when an i2c bus is accessed by several entities like the linux driver, ATF driver and UEFI driver. Make sure to pick up the ATF/UEFI image to accompany this change because at boot time ATF will ensure that the lock is released. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com<mailto:asmaa@nvidia.com>> --- drivers/i2c/busses/i2c-mlxbf.c | 40 ++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c index 0ac9e70244a7..b951443b91f8 100644 --- a/drivers/i2c/busses/i2c-mlxbf.c +++ b/drivers/i2c/busses/i2c-mlxbf.c @@ -340,7 +340,8 @@ enum { * Timeout is given in microsends. Note also that timeout handling is not * exact. */ -#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ +#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ +#define SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */ /* Encapsulates timing parameters */ struct mlx_i2c_timings { @@ -573,6 +574,25 @@ static bool mlx_smbus_master_wait_for_idle(struct mlx_i2c_priv *priv) return false; } +/* + * wait for the lock to be released before acquiring it. + */ +static bool mlx_smbus_master_lock(struct mlx_i2c_priv *priv) +{ + if (mlx_smbus_poll(priv->smbus->io, SMBUS_MASTER_GW, + 1 << MASTER_LOCK_BIT_OFF, true, + SMBUS_LOCK_POLL_TIMEOUT)) + return true; + + return false; +} + +static void mlx_smbus_master_unlock(struct mlx_i2c_priv *priv) +{ + /* Clear the gw to clear the lock */ + writel(0, priv->smbus->io + SMBUS_MASTER_GW); +} + /* * Poll SMBus master status and return transaction status, * i.e. whether succeeded or failed. I2C and SMBus fault codes @@ -744,9 +764,17 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, slave = request->slave & 0x7f; addr = slave << 1; - /* First of all, check whether the HW is idle */ - if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) + /* Try to acquire the smbus gw lock before any reads of the GW register since + * a read sets the lock. + */ + if (WARN_ON(!mlx_smbus_master_lock(priv))) + return -EBUSY; + + /* Check whether the HW is idle */ + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) { + mlx_smbus_master_unlock(priv); return -EBUSY; + } /* Set first byte */ data_desc[data_idx++] = addr; @@ -802,8 +830,10 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, if (write_en) { ret = mlx_smbus_enable(priv, slave, write_len, block_en, pec_en, 0); - if (ret != 0) + if (ret) { + mlx_smbus_master_unlock(priv); return ret; + } } if (read_en) { @@ -830,6 +860,8 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, SMBUS_MASTER_FSM_PS_STATE_MASK); } + mlx_smbus_master_unlock(priv); + return ret; } -- 2.30.1 -- kernel-team mailing list kernel-team@lists.ubuntu.com<mailto:kernel-team@lists.ubuntu.com> https://lists.ubuntu.com/mailman/listinfo/kernel-team<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.ubuntu.com%2Fmailman%2Flistinfo%2Fkernel-team&data=05%7C01%7Casmaa%40nvidia.com%7C5a683fcac9bb42a6467308da6b1f5841%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637940079625328484%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=aiAKlztcQu2Cmc07Rt72DMf6TJPDugyc84doT4osP90%3D&reserved=0>
diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c index 0ac9e70244a7..b951443b91f8 100644 --- a/drivers/i2c/busses/i2c-mlxbf.c +++ b/drivers/i2c/busses/i2c-mlxbf.c @@ -340,7 +340,8 @@ enum { * Timeout is given in microsends. Note also that timeout handling is not * exact. */ -#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ +#define SMBUS_TIMEOUT (300 * 1000) /* 300ms */ +#define SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */ /* Encapsulates timing parameters */ struct mlx_i2c_timings { @@ -573,6 +574,25 @@ static bool mlx_smbus_master_wait_for_idle(struct mlx_i2c_priv *priv) return false; } +/* + * wait for the lock to be released before acquiring it. + */ +static bool mlx_smbus_master_lock(struct mlx_i2c_priv *priv) +{ + if (mlx_smbus_poll(priv->smbus->io, SMBUS_MASTER_GW, + 1 << MASTER_LOCK_BIT_OFF, true, + SMBUS_LOCK_POLL_TIMEOUT)) + return true; + + return false; +} + +static void mlx_smbus_master_unlock(struct mlx_i2c_priv *priv) +{ + /* Clear the gw to clear the lock */ + writel(0, priv->smbus->io + SMBUS_MASTER_GW); +} + /* * Poll SMBus master status and return transaction status, * i.e. whether succeeded or failed. I2C and SMBus fault codes @@ -744,9 +764,17 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, slave = request->slave & 0x7f; addr = slave << 1; - /* First of all, check whether the HW is idle */ - if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) + /* Try to acquire the smbus gw lock before any reads of the GW register since + * a read sets the lock. + */ + if (WARN_ON(!mlx_smbus_master_lock(priv))) + return -EBUSY; + + /* Check whether the HW is idle */ + if (WARN_ON(!mlx_smbus_master_wait_for_idle(priv))) { + mlx_smbus_master_unlock(priv); return -EBUSY; + } /* Set first byte */ data_desc[data_idx++] = addr; @@ -802,8 +830,10 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, if (write_en) { ret = mlx_smbus_enable(priv, slave, write_len, block_en, pec_en, 0); - if (ret != 0) + if (ret) { + mlx_smbus_master_unlock(priv); return ret; + } } if (read_en) { @@ -830,6 +860,8 @@ static int mlx_smbus_start_transaction(struct mlx_i2c_priv *priv, SMBUS_MASTER_FSM_PS_STATE_MASK); } + mlx_smbus_master_unlock(priv); + return ret; }
Buglink: https://bugs.launchpad.net/bugs/1981105 Support the I2C lock mechanism, otherwise there could be unexpected behavior when an i2c bus is accessed by several entities like the linux driver, ATF driver and UEFI driver. Make sure to pick up the ATF/UEFI image to accompany this change because at boot time ATF will ensure that the lock is released. Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com> --- drivers/i2c/busses/i2c-mlxbf.c | 40 ++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-)