The following patches provide fixes for the Intel SDSi driver based on
firmware updates and for one driver bug.
David E. Box (3):
platform/x86/intel/sdsi: Handle leaky bucket
platform/x86/intel/sdsi: Poll on ready bit for writes
platform/x86/intel/sdsi: Fix bug in multi packet reads
drivers/platform/x86/intel/sdsi.c | 44 +++++++++++++++++++++----------
1 file changed, 30 insertions(+), 14 deletions(-)
base-commit: b2d229d4ddb17db541098b83524d901257e93845
--
2.25.1
Fix bug that added an offset to the mailbox addr during multi-packet
reads. Did not affect current ABI since it doesn't support multi-packet
transactions.
Fixes: 2546c6000430 ("platform/x86: Add Intel Software Defined Silicon driver")
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel/sdsi.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 89729fed030c..c830e98dfa38 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -83,7 +83,7 @@ enum sdsi_command {
struct sdsi_mbox_info {
u64 *payload;
- u64 *buffer;
+ void *buffer;
int size;
};
@@ -165,9 +165,7 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
total = 0;
loop = 0;
do {
- int offset = SDSI_SIZE_MAILBOX * loop;
- void __iomem *addr = priv->mbox_addr + offset;
- u64 *buf = info->buffer + offset / SDSI_SIZE_CMD;
+ void *buf = info->buffer + (SDSI_SIZE_MAILBOX * loop);
u32 packet_size;
/* Poll on ready bit */
@@ -198,7 +196,7 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
break;
}
- sdsi_memcpy64_fromio(buf, addr, round_up(packet_size, SDSI_SIZE_CMD));
+ sdsi_memcpy64_fromio(buf, priv->mbox_addr, round_up(packet_size, SDSI_SIZE_CMD));
total += packet_size;
--
2.25.1
To prevent an agent from indefinitely holding the mailbox firmware has
implemented a leaky bucket algorithm. Repeated access to the mailbox may
now incur a delay of up to 2.1 seconds. Add a retry loop that tries for
up to 2.5 seconds to acquire the mailbox.
Fixes: 2546c6000430 ("platform/x86: Add Intel Software Defined Silicon driver")
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel/sdsi.c | 32 ++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 11d14cc0ff0a..11f211402479 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -51,6 +51,8 @@
#define MBOX_TIMEOUT_US 2000
#define MBOX_TIMEOUT_ACQUIRE_US 1000
#define MBOX_POLLING_PERIOD_US 100
+#define MBOX_ACQUIRE_NUM_RETRIES 5
+#define MBOX_ACQUIRE_RETRY_DELAY_MS 500
#define MBOX_MAX_PACKETS 4
#define MBOX_OWNER_NONE 0x00
@@ -263,7 +265,7 @@ static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info
{
u64 control;
u32 owner;
- int ret;
+ int ret, retries = 0;
lockdep_assert_held(&priv->mb_lock);
@@ -273,13 +275,29 @@ static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info
if (owner != MBOX_OWNER_NONE)
return -EBUSY;
- /* Write first qword of payload */
- writeq(info->payload[0], priv->mbox_addr);
+ /*
+ * If there has been no recent transaction and no one owns the mailbox,
+ * we should acquire it in under 1ms. However, if we've accessed it
+ * recently it may take up to 2.1 seconds to acquire it again.
+ */
+ do {
+ /* Write first qword of payload */
+ writeq(info->payload[0], priv->mbox_addr);
+
+ /* Check for ownership */
+ ret = readq_poll_timeout(priv->control_addr, control,
+ FIELD_GET(CTRL_OWNER, control) == MBOX_OWNER_INBAND,
+ MBOX_POLLING_PERIOD_US, MBOX_TIMEOUT_ACQUIRE_US);
+
+ if (FIELD_GET(CTRL_OWNER, control) == MBOX_OWNER_NONE &&
+ retries++ < MBOX_ACQUIRE_NUM_RETRIES) {
+ msleep(MBOX_ACQUIRE_RETRY_DELAY_MS);
+ continue;
+ }
- /* Check for ownership */
- ret = readq_poll_timeout(priv->control_addr, control,
- FIELD_GET(CTRL_OWNER, control) & MBOX_OWNER_INBAND,
- MBOX_POLLING_PERIOD_US, MBOX_TIMEOUT_ACQUIRE_US);
+ /* Either we got it or someone else did. */
+ break;
+ } while (true);
return ret;
}
--
2.25.1
Due to change in firmware flow, update mailbox writes to poll on ready bit
instead of run_busy bit. This change makes the polling method consistent
for both writes and reads, which also uses the ready bit.
Fixes: 2546c6000430 ("platform/x86: Add Intel Software Defined Silicon driver")
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel/sdsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 11f211402479..89729fed030c 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -245,8 +245,8 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
FIELD_PREP(CTRL_PACKET_SIZE, info->size);
writeq(control, priv->control_addr);
- /* Poll on run_busy bit */
- ret = readq_poll_timeout(priv->control_addr, control, !(control & CTRL_RUN_BUSY),
+ /* Poll on ready bit */
+ ret = readq_poll_timeout(priv->control_addr, control, control & CTRL_READY,
MBOX_POLLING_PERIOD_US, MBOX_TIMEOUT_US);
if (ret)
--
2.25.1
[AMD Official Use Only]
> -----Original Message-----
> From: David E. Box <[email protected]>
> Sent: Wednesday, April 20, 2022 10:56
> To: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: [PATCH 2/3] platform/x86/intel/sdsi: Poll on ready bit for writes
>
> Due to change in firmware flow, update mailbox writes to poll on ready bit
> instead of run_busy bit. This change makes the polling method consistent
> for both writes and reads, which also uses the ready bit.
Does this need some sort of guard on the behavior based on the firmware
version you are running on or are these all pre-production still?
>
> Fixes: 2546c6000430 ("platform/x86: Add Intel Software Defined Silicon driver")
> Signed-off-by: David E. Box <[email protected]>
> ---
> drivers/platform/x86/intel/sdsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 11f211402479..89729fed030c 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -245,8 +245,8 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv,
> struct sdsi_mbox_info *in
> FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> writeq(control, priv->control_addr);
>
> - /* Poll on run_busy bit */
> - ret = readq_poll_timeout(priv->control_addr, control, !(control &
> CTRL_RUN_BUSY),
> + /* Poll on ready bit */
> + ret = readq_poll_timeout(priv->control_addr, control, control &
> CTRL_READY,
> MBOX_POLLING_PERIOD_US,
> MBOX_TIMEOUT_US);
>
> if (ret)
> --
> 2.25.1
Hi,
On 4/20/22 17:56, David E. Box wrote:
> The following patches provide fixes for the Intel SDSi driver based on
> firmware updates and for one driver bug.
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
>
> David E. Box (3):
> platform/x86/intel/sdsi: Handle leaky bucket
> platform/x86/intel/sdsi: Poll on ready bit for writes
> platform/x86/intel/sdsi: Fix bug in multi packet reads
>
> drivers/platform/x86/intel/sdsi.c | 44 +++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
>
> base-commit: b2d229d4ddb17db541098b83524d901257e93845