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

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

[Prashant Malani]

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

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

[Prashant Malani]

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 <pmalani@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
---
 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

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

[Mika Westerberg]

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 <pmalani@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

[Prashant Malani]

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 <pmalani@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
---
 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

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

[Mika Westerberg]

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 <pmalani@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

[Prashant Malani]

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 <pmalani@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
---
 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

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

[Mika Westerberg]

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 <pmalani@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

[Hans de Goede]

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