diff mbox series

[for,v5.17,6/8] iwlwifi: mei: retry mapping the shared area

Message ID iwlwifi.20220128142706.cc51e6a6d635.I4b74a082eb8d89f9e4f556a27c4339c15444dc6c@changeid
State New
Headers show
Series iwlwifi: fixes intended for v5.17 2022-01-28 | expand

Commit Message

Luca Coelho Jan. 28, 2022, 12:30 p.m. UTC
From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

The shared area is a DMA memory allocated in the host and
mapped so that the host and the CSME firmware can
exchange data. It is mapped through a dedicated PCI device
that is driven by the mei bus driver.

The bus driver is in charge of allocating and mapping this
memory. It also needs to configure the CSME firmware with
a specific set of commands, so that the CSME firmware will
know that this memory is meant to be used by its internal
WLAN module.

For this, the CSME firmware first needs to completely
initialize its WLAN module and only then get the mapping
request.

The problem is that the mei bus enumeration completes
before the WLAN is completely ready. This means that
the WLAN module's initialization is racing with iwlmei's
allocation and mapping flow.

Testing showed a problem in resume flows where iwlmei
was too fast and the DMA mapping failed.

Add a retry mechanism to make sure that we will succeed
to map the memory.

Fixes: 2da4366f9e2c ("iwlwifi: mei: add the driver to allow cooperation with CSME")
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Fixes: bcbddc4f9d02 ("iwlwifi: mei: wait before mapping the shared area")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mei/main.c | 35 ++++++++++++++-----
 1 file changed, 26 insertions(+), 9 deletions(-)

Comments

Kalle Valo Feb. 3, 2022, 8:16 a.m. UTC | #1
Luca Coelho <luca@coelho.fi> writes:

> From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
>
> The shared area is a DMA memory allocated in the host and
> mapped so that the host and the CSME firmware can
> exchange data. It is mapped through a dedicated PCI device
> that is driven by the mei bus driver.
>
> The bus driver is in charge of allocating and mapping this
> memory. It also needs to configure the CSME firmware with
> a specific set of commands, so that the CSME firmware will
> know that this memory is meant to be used by its internal
> WLAN module.
>
> For this, the CSME firmware first needs to completely
> initialize its WLAN module and only then get the mapping
> request.
>
> The problem is that the mei bus enumeration completes
> before the WLAN is completely ready. This means that
> the WLAN module's initialization is racing with iwlmei's
> allocation and mapping flow.
>
> Testing showed a problem in resume flows where iwlmei
> was too fast and the DMA mapping failed.
>
> Add a retry mechanism to make sure that we will succeed
> to map the memory.
>
> Fixes: 2da4366f9e2c ("iwlwifi: mei: add the driver to allow cooperation with CSME")
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Fixes: bcbddc4f9d02 ("iwlwifi: mei: wait before mapping the shared area")

I'll move the latter Fixes before s-o-b tag.

> -	/*
> -	 * The CSME firmware needs to boot the internal WLAN client. Wait here
> -	 * so that the DMA map request will succeed.
> -	 */
> -	msleep(20);
> +	do {
> +		ret = iwl_mei_alloc_shared_mem(cldev);
> +		if (!ret)
> +			break;
> +		/*
> +		 * The CSME firmware needs to boot the internal WLAN client.
> +		 * This can take time in certain configurations (usually
> +		 * upon resume and when the whole CSME firmware is shut down
> +		 * during suspend).
> +		 *
> +		 * Wait a bit before retrying and hope we'll succeed next time.
> +		 */
>  
> -	ret = iwl_mei_alloc_shared_mem(cldev);
> -	if (ret)
> +		dev_dbg(&cldev->dev,
> +			"Couldn't allocate the shared memory: %d, attempt %d / %d\n",
> +			ret, alloc_retry, ALLOC_SHARED_MEM_RETRY_MAX_NUM);
> +		msleep(100);
> +		alloc_retry--;
> +	} while (alloc_retry);

Nitpicking, but this could have been:

while (alloc_retry--);

But no need to resend because of this.
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/mei/main.c b/drivers/net/wireless/intel/iwlwifi/mei/main.c
index d9733aaf6f6e..6cc5553027a0 100644
--- a/drivers/net/wireless/intel/iwlwifi/mei/main.c
+++ b/drivers/net/wireless/intel/iwlwifi/mei/main.c
@@ -229,8 +229,6 @@  static int iwl_mei_alloc_shared_mem(struct mei_cl_device *cldev)
 	if (IS_ERR(mem->ctrl)) {
 		int ret = PTR_ERR(mem->ctrl);
 
-		dev_err(&cldev->dev, "Couldn't allocate the shared memory: %d\n",
-			ret);
 		mem->ctrl = NULL;
 
 		return ret;
@@ -1784,6 +1782,8 @@  static void iwl_mei_dbgfs_unregister(struct iwl_mei *mei) {}
 
 #endif /* CONFIG_DEBUG_FS */
 
+#define ALLOC_SHARED_MEM_RETRY_MAX_NUM	3
+
 /*
  * iwl_mei_probe - the probe function called by the mei bus enumeration
  *
@@ -1795,6 +1795,7 @@  static void iwl_mei_dbgfs_unregister(struct iwl_mei *mei) {}
 static int iwl_mei_probe(struct mei_cl_device *cldev,
 			 const struct mei_cl_device_id *id)
 {
+	int alloc_retry = ALLOC_SHARED_MEM_RETRY_MAX_NUM;
 	struct iwl_mei *mei;
 	int ret;
 
@@ -1812,15 +1813,31 @@  static int iwl_mei_probe(struct mei_cl_device *cldev,
 	mei_cldev_set_drvdata(cldev, mei);
 	mei->cldev = cldev;
 
-	/*
-	 * The CSME firmware needs to boot the internal WLAN client. Wait here
-	 * so that the DMA map request will succeed.
-	 */
-	msleep(20);
+	do {
+		ret = iwl_mei_alloc_shared_mem(cldev);
+		if (!ret)
+			break;
+		/*
+		 * The CSME firmware needs to boot the internal WLAN client.
+		 * This can take time in certain configurations (usually
+		 * upon resume and when the whole CSME firmware is shut down
+		 * during suspend).
+		 *
+		 * Wait a bit before retrying and hope we'll succeed next time.
+		 */
 
-	ret = iwl_mei_alloc_shared_mem(cldev);
-	if (ret)
+		dev_dbg(&cldev->dev,
+			"Couldn't allocate the shared memory: %d, attempt %d / %d\n",
+			ret, alloc_retry, ALLOC_SHARED_MEM_RETRY_MAX_NUM);
+		msleep(100);
+		alloc_retry--;
+	} while (alloc_retry);
+
+	if (ret) {
+		dev_err(&cldev->dev, "Couldn't allocate the shared memory: %d\n",
+			ret);
 		goto free;
+	}
 
 	iwl_mei_init_shared_mem(mei);