2021-09-28 10:21:12

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 0/3] platform/x86: intel_scu_ipc: timeout fixes and cleanup

This is a short series to make some fixes and timeout value
modifications to the SCU IPC driver timeout handling.

Prashant Malani (3):
platform/x86: intel_scu_ipc: Fix busy loop expiry time
platform/x86: intel_scu_ipc: Increase virtual timeout to 10s
platform/x86: intel_scu_ipc: Update timeout value in comment

drivers/platform/x86/intel_scu_ipc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

--
2.33.0.685.g46640cef36-goog


2021-09-28 10:22:23

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 1/3] platform/x86: intel_scu_ipc: Fix busy loop expiry time

The macro IPC_TIMEOUT is already in jiffies (it is also used like that
elsewhere in the file when calling wait_for_completion_timeout()). Don’t
convert it using helper functions for the purposes of calculating the
busy loop expiry time.

Fixes: e7b7ab3847c9 (“platform/x86: intel_scu_ipc: Sleeping is fine when polling”)
Signed-off-by: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
---
drivers/platform/x86/intel_scu_ipc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index bfa0cc20750d..cfb249da2a7b 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -232,7 +232,7 @@ static inline u32 ipc_data_readl(struct intel_scu_ipc_dev *scu, u32 offset)
/* Wait till scu status is busy */
static inline int busy_loop(struct intel_scu_ipc_dev *scu)
{
- unsigned long end = jiffies + msecs_to_jiffies(IPC_TIMEOUT);
+ unsigned long end = jiffies + IPC_TIMEOUT;

do {
u32 status;
--
2.33.0.685.g46640cef36-goog

2021-09-28 10:22:44

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 3/3] platform/x86: intel_scu_ipc: Update timeout value in comment

The comment decribing the IPC timeout hadn't been updated when the
actual timeout was changed from 3 to 5 seconds in
commit a7d53dbbc70a ("platform/x86: intel_scu_ipc: Increase virtual
timeout from 3 to 5 seconds") .

Since the value is anyway updated to 10s now, take this opportunity to
update the value in the comment too.

Signed-off-by: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
---
drivers/platform/x86/intel_scu_ipc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index d71a1dce781c..7cc9089d1e14 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -247,7 +247,7 @@ static inline int busy_loop(struct intel_scu_ipc_dev *scu)
return -ETIMEDOUT;
}

-/* Wait till ipc ioc interrupt is received or timeout in 3 HZ */
+/* Wait till ipc ioc interrupt is received or timeout in 10 HZ */
static inline int ipc_wait_for_interrupt(struct intel_scu_ipc_dev *scu)
{
int status;
--
2.33.0.685.g46640cef36-goog

2021-09-28 10:23:49

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 2/3] platform/x86: intel_scu_ipc: Increase virtual timeout to 10s

Commit a7d53dbbc70a ("platform/x86: intel_scu_ipc: Increase virtual
timeout from 3 to 5 seconds") states that the recommended timeout range
is 5-10 seconds. Adjust the timeout value to the higher of those i.e 10
seconds, to account for situations where the 5 seconds is insufficient
for disconnect command success.

Signed-off-by: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
---
drivers/platform/x86/intel_scu_ipc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index cfb249da2a7b..d71a1dce781c 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -75,7 +75,7 @@ struct intel_scu_ipc_dev {
#define IPC_READ_BUFFER 0x90

/* Timeout in jiffies */
-#define IPC_TIMEOUT (5 * HZ)
+#define IPC_TIMEOUT (10 * HZ)

static struct intel_scu_ipc_dev *ipcdev; /* Only one for now */
static DEFINE_MUTEX(ipclock); /* lock used to prevent multiple call to SCU */
--
2.33.0.685.g46640cef36-goog

2021-09-28 10:56:29

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 2/3] platform/x86: intel_scu_ipc: Increase virtual timeout to 10s

On Tue, Sep 28, 2021 at 03:19:32AM -0700, Prashant Malani wrote:
> Commit a7d53dbbc70a ("platform/x86: intel_scu_ipc: Increase virtual
> timeout from 3 to 5 seconds") states that the recommended timeout range
> is 5-10 seconds. Adjust the timeout value to the higher of those i.e 10
> seconds, to account for situations where the 5 seconds is insufficient
> for disconnect command success.
>
> Signed-off-by: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2021-09-28 10:56:55

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 1/3] platform/x86: intel_scu_ipc: Fix busy loop expiry time

On Tue, Sep 28, 2021 at 03:19:30AM -0700, Prashant Malani wrote:
> The macro IPC_TIMEOUT is already in jiffies (it is also used like that
> elsewhere in the file when calling wait_for_completion_timeout()). Don’t
> convert it using helper functions for the purposes of calculating the
> busy loop expiry time.
>
> Fixes: e7b7ab3847c9 (“platform/x86: intel_scu_ipc: Sleeping is fine when polling”)
> Signed-off-by: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2021-09-28 10:57:08

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 3/3] platform/x86: intel_scu_ipc: Update timeout value in comment

On Tue, Sep 28, 2021 at 03:19:34AM -0700, Prashant Malani wrote:
> The comment decribing the IPC timeout hadn't been updated when the
> actual timeout was changed from 3 to 5 seconds in
> commit a7d53dbbc70a ("platform/x86: intel_scu_ipc: Increase virtual
> timeout from 3 to 5 seconds") .
>
> Since the value is anyway updated to 10s now, take this opportunity to
> update the value in the comment too.
>
> Signed-off-by: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2021-10-11 16:28:31

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 0/3] platform/x86: intel_scu_ipc: timeout fixes and cleanup

Hi,

On 9/28/21 12:19 PM, Prashant Malani wrote:
> This is a short series to make some fixes and timeout value
> modifications to the SCU IPC driver timeout handling.
>
> Prashant Malani (3):
> platform/x86: intel_scu_ipc: Fix busy loop expiry time
> platform/x86: intel_scu_ipc: Increase virtual timeout to 10s
> platform/x86: intel_scu_ipc: Update timeout value in comment

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

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.

Since these are clear bug fixes I will also include these
in my upcoming pdx86-fixes pull-req for 5.15 .

Regards,

Hans