Skip to contentSkip to conversation

rsi: sdio suspend and resume support

PATCH In review Amitkumar Karwar 4 participants 6 messages Sep 21, 2017 – Sep 25, 2017 Redpine wireless driver

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 rsi: sdio suspend and resume support

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

---
v2: Replaced never ending while loop with 20msecs loop(Kalle Valo)

---
 drivers/net/wireless/rsi/rsi_91x_sdio.c | 128 +++++++++++++++++++++++++++++++-
 drivers/net/wireless/rsi/rsi_sdio.h     |   2 +
 2 files changed, 126 insertions(+), 4 deletions(-)
drivers/net/wireless/rsi/rsi_91x_sdio.c +124 -4
index 8d3a483..b3f8006 100644
@@ -1059,16 +1059,136 @@ static void rsi_disconnect(struct sdio_func *pfunction)
 }
 
 #ifdef CONFIG_PM
+static int rsi_set_sdio_pm_caps(struct rsi_hw *adapter)
+{
+	struct rsi_91x_sdiodev *dev =
+		(struct rsi_91x_sdiodev *)adapter->rsi_dev;
+	struct sdio_func *func = dev->pfunction;
+	int ret;
+
+	ret = sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
+	if (ret)
+		rsi_dbg(ERR_ZONE, "Set sdio keep pwr flag failed: %d\n", ret);
+
+	return ret;
+}
+
+static int rsi_sdio_disable_interrupts(struct sdio_func *pfunc)
+{
+	struct rsi_hw *adapter = sdio_get_drvdata(pfunc);
+	u8 isr_status = 0, data = 0;
+	int ret;
+	unsigned long t1;
+
+	rsi_dbg(INFO_ZONE, "Waiting for interrupts to be cleared..");
+	t1 = jiffies;
+	do {
+		rsi_sdio_read_register(adapter, RSI_FN1_INT_REGISTER,
+				       &isr_status);
+		rsi_dbg(INFO_ZONE, ".");
+	} while ((isr_status) && (jiffies_to_msecs(jiffies - t1) < 20));
+	rsi_dbg(INFO_ZONE, "Interrupts cleared\n");
+
+	sdio_claim_host(pfunc);
+	ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to read int enable register\n",
+			__func__);
+		goto done;
+	}
+
+	data &= RSI_INT_ENABLE_MASK;
+	ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to write to int enable register\n",
+			__func__);
+		goto done;
+	}
+	ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to read int enable register\n",
+			__func__);
+		goto done;
+	}
+	rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+	sdio_release_host(pfunc);
+	return ret;
+}
+
+static int rsi_sdio_enable_interrupts(struct sdio_func *pfunc)
+{
+	u8 data;
+	int ret;
+
+	sdio_claim_host(pfunc);
+	ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to read int enable register\n", __func__);
+		goto done;
+	}
+
+	data |= ~RSI_INT_ENABLE_MASK & 0xff;
+
+	ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to write to int enable register\n",
+			__func__);
+		goto done;
+	}
+
+	ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+	if (ret < 0) {
+		rsi_dbg(ERR_ZONE,
+			"%s: Failed to read int enable register\n", __func__);
+		goto done;
+	}
+	rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+	sdio_release_host(pfunc);
+	return ret;
+}
+
 static int rsi_suspend(struct device *dev)
 {
-	/* Not yet implemented */
-	return -ENOSYS;
+	int ret;
+	struct sdio_func *pfunction = dev_to_sdio_func(dev);
+	struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+	struct rsi_common *common;
+
+	if (!adapter) {
+		rsi_dbg(ERR_ZONE, "Device is not ready\n");
+		return -ENODEV;
+	}
+	common = adapter->priv;
+	rsi_sdio_disable_interrupts(pfunction);
+
+	ret = rsi_set_sdio_pm_caps(adapter);
+	if (ret)
+		rsi_dbg(INFO_ZONE,
+			"Setting power management caps failed\n");
+	common->fsm_state = FSM_CARD_NOT_READY;
+
+	return 0;
 }
 
 static int rsi_resume(struct device *dev)
 {
-	/* Not yet implemented */
-	return -ENOSYS;
+	struct sdio_func *pfunction = dev_to_sdio_func(dev);
+	struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+	struct rsi_common *common = adapter->priv;
+
+	common->fsm_state = FSM_MAC_INIT_DONE;
+	rsi_sdio_enable_interrupts(pfunction);
+
+	return 0;
 }
 
 static const struct dev_pm_ops rsi_pm_ops = {
drivers/net/wireless/rsi/rsi_sdio.h +2 -0
index 95e4bed..49c549b 100644
@@ -48,6 +48,8 @@ enum sdio_interrupt_type {
 
 #define RSI_DEVICE_BUFFER_STATUS_REGISTER       0xf3
 #define RSI_FN1_INT_REGISTER                    0xf9
+#define RSI_INT_ENABLE_REGISTER			0x04
+#define RSI_INT_ENABLE_MASK			0xfc
 #define RSI_SD_REQUEST_MASTER                   0x10000
 
 /* FOR SD CARD ONLY */
Signature · 1 line
2.7.4
#1 lore raw
Quoted 132 lines · Amitkumar Karwar

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

---
v2: Replaced never ending while loop with 20msecs loop(Kalle Valo)

---
 drivers/net/wireless/rsi/rsi_91x_sdio.c | 128 +++++++++++++++++++++++++++++++-
 drivers/net/wireless/rsi/rsi_sdio.h     |   2 +
 2 files changed, 126 insertions(+), 4 deletions(-)
drivers/net/wireless/rsi/rsi_91x_sdio.c +106 -2
index 8d3a483..b3f8006 100644
@@ -1059,16 +1059,136 @@ static void rsi_disconnect(struct sdio_func *pfunction)
 }
 
 #ifdef CONFIG_PM
+static int rsi_set_sdio_pm_caps(struct rsi_hw *adapter)
+{
+       struct rsi_91x_sdiodev *dev =
+               (struct rsi_91x_sdiodev *)adapter->rsi_dev;
+       struct sdio_func *func = dev->pfunction;
+       int ret;
+
+       ret = sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
+       if (ret)
+               rsi_dbg(ERR_ZONE, "Set sdio keep pwr flag failed: %d\n", ret);
+
+       return ret;
+}
+
+static int rsi_sdio_disable_interrupts(struct sdio_func *pfunc)
+{
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunc);
+       u8 isr_status = 0, data = 0;
+       int ret;
+       unsigned long t1;
+
+       rsi_dbg(INFO_ZONE, "Waiting for interrupts to be cleared..");
+       t1 = jiffies;
+       do {
+               rsi_sdio_read_register(adapter, RSI_FN1_INT_REGISTER,
+                                      &isr_status);
+               rsi_dbg(INFO_ZONE, ".");
+       } while ((isr_status) && (jiffies_to_msecs(jiffies - t1) < 20));
+       rsi_dbg(INFO_ZONE, "Interrupts cleared\n");
+
+       sdio_claim_host(pfunc);
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n",
+                       __func__);
+               goto done;
+       }
+
+       data &= RSI_INT_ENABLE_MASK;
+       ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to write to int enable register\n",
+                       __func__);
+               goto done;
+       }
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n",
+                       __func__);
+               goto done;
+       }
+       rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+       sdio_release_host(pfunc);
+       return ret;
+}
+
+static int rsi_sdio_enable_interrupts(struct sdio_func *pfunc)
+{
+       u8 data;
+       int ret;
+
+       sdio_claim_host(pfunc);
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n", __func__);
+               goto done;
+       }
+
+       data |= ~RSI_INT_ENABLE_MASK & 0xff;
+
+       ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to write to int enable register\n",
+                       __func__);
+               goto done;
+       }
+
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n", __func__);
+               goto done;
+       }
+       rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+       sdio_release_host(pfunc);
+       return ret;
+}
+
 static int rsi_suspend(struct device *dev)
 {
-       /* Not yet implemented */
-       return -ENOSYS;
+       int ret;
+       struct sdio_func *pfunction = dev_to_sdio_func(dev);
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+       struct rsi_common *common;
+
+       if (!adapter) {
+               rsi_dbg(ERR_ZONE, "Device is not ready\n");
+               return -ENODEV;
+       }

drivers/net/wireless/rsi/rsi_91x_sdio.c static void rsi_disconnect(struct sdio_func *pfunction) · line 1179

Quoted 10 lines
drivers/net/wireless/rsi/rsi_91x_sdio.c +10 -0
+       common = adapter->priv;
+       rsi_sdio_disable_interrupts(pfunction);
+
+       ret = rsi_set_sdio_pm_caps(adapter);
+       if (ret)
+               rsi_dbg(INFO_ZONE,
+                       "Setting power management caps failed\n");
+       common->fsm_state = FSM_CARD_NOT_READY;
+
+       return 0;

I think it should be return ret if rsi_set_sdio_pm_caps() fails.

Quoted 33 lines
drivers/net/wireless/rsi/rsi_91x_sdio.c +8 -2
 }
 
 static int rsi_resume(struct device *dev)
 {
-       /* Not yet implemented */
-       return -ENOSYS;
+       struct sdio_func *pfunction = dev_to_sdio_func(dev);
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+       struct rsi_common *common = adapter->priv;
+
+       common->fsm_state = FSM_MAC_INIT_DONE;
+       rsi_sdio_enable_interrupts(pfunction);
+
+       return 0;
 }
 
 static const struct dev_pm_ops rsi_pm_ops = {
drivers/net/wireless/rsi/rsi_sdio.h +2 -1
index 95e4bed..49c549b 100644
@@ -48,6 +48,8 @@ enum sdio_interrupt_type {
 
 #define RSI_DEVICE_BUFFER_STATUS_REGISTER       0xf3
 #define RSI_FN1_INT_REGISTER                    0xf9
+#define RSI_INT_ENABLE_REGISTER                        0x04
+#define RSI_INT_ENABLE_MASK                    0xfc
 #define RSI_SD_REQUEST_MASTER                   0x10000
 
 /* FOR SD CARD ONLY */
--

2.7.4

#2 lore raw
Quoted 137 lines · Souptick Joarder
Quoted 136 lines · Amitkumar Karwar

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

---
v2: Replaced never ending while loop with 20msecs loop(Kalle Valo)

---
 drivers/net/wireless/rsi/rsi_91x_sdio.c | 128 +++++++++++++++++++++++++++++++-
 drivers/net/wireless/rsi/rsi_sdio.h     |   2 +
 2 files changed, 126 insertions(+), 4 deletions(-)
drivers/net/wireless/rsi/rsi_91x_sdio.c +110 -2
index 8d3a483..b3f8006 100644
@@ -1059,16 +1059,136 @@ static void rsi_disconnect(struct sdio_func *pfunction)
 }
 
 #ifdef CONFIG_PM
+static int rsi_set_sdio_pm_caps(struct rsi_hw *adapter)
+{
+       struct rsi_91x_sdiodev *dev =
+               (struct rsi_91x_sdiodev *)adapter->rsi_dev;
+       struct sdio_func *func = dev->pfunction;
+       int ret;
+
+       ret = sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
+       if (ret)
+               rsi_dbg(ERR_ZONE, "Set sdio keep pwr flag failed: %d\n", ret);
+
+       return ret;
+}
+
+static int rsi_sdio_disable_interrupts(struct sdio_func *pfunc)
+{
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunc);
+       u8 isr_status = 0, data = 0;
+       int ret;
+       unsigned long t1;
+
+       rsi_dbg(INFO_ZONE, "Waiting for interrupts to be cleared..");
+       t1 = jiffies;
+       do {
+               rsi_sdio_read_register(adapter, RSI_FN1_INT_REGISTER,
+                                      &isr_status);
+               rsi_dbg(INFO_ZONE, ".");
+       } while ((isr_status) && (jiffies_to_msecs(jiffies - t1) < 20));
+       rsi_dbg(INFO_ZONE, "Interrupts cleared\n");
+
+       sdio_claim_host(pfunc);
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n",
+                       __func__);
+               goto done;
+       }
+
+       data &= RSI_INT_ENABLE_MASK;
+       ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to write to int enable register\n",
+                       __func__);
+               goto done;
+       }
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n",
+                       __func__);
+               goto done;
+       }
+       rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+       sdio_release_host(pfunc);
+       return ret;
+}
+
+static int rsi_sdio_enable_interrupts(struct sdio_func *pfunc)
+{
+       u8 data;
+       int ret;
+
+       sdio_claim_host(pfunc);
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n", __func__);
+               goto done;
+       }
+
+       data |= ~RSI_INT_ENABLE_MASK & 0xff;
+
+       ret = rsi_cmd52writebyte(pfunc->card, RSI_INT_ENABLE_REGISTER, data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to write to int enable register\n",
+                       __func__);
+               goto done;
+       }
+
+       ret = rsi_cmd52readbyte(pfunc->card, RSI_INT_ENABLE_REGISTER, &data);
+       if (ret < 0) {
+               rsi_dbg(ERR_ZONE,
+                       "%s: Failed to read int enable register\n", __func__);
+               goto done;
+       }
+       rsi_dbg(INFO_ZONE, "int enable reg content = %x\n", data);
+
+done:
+       sdio_release_host(pfunc);
+       return ret;
+}
+
 static int rsi_suspend(struct device *dev)
 {
-       /* Not yet implemented */
-       return -ENOSYS;
+       int ret;
+       struct sdio_func *pfunction = dev_to_sdio_func(dev);
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+       struct rsi_common *common;
+
+       if (!adapter) {
+               rsi_dbg(ERR_ZONE, "Device is not ready\n");
+               return -ENODEV;
+       }
+       common = adapter->priv;
+       rsi_sdio_disable_interrupts(pfunction);
+
+       ret = rsi_set_sdio_pm_caps(adapter);
Quoted 8 lines

drivers/net/wireless/rsi/rsi_91x_sdio.c static void rsi_disconnect(struct sdio_func *pfunction) · line 1179

Quoted 6 lines
drivers/net/wireless/rsi/rsi_91x_sdio.c +6 -0
+       if (ret)
+               rsi_dbg(INFO_ZONE,
+                       "Setting power management caps failed\n");
+       common->fsm_state = FSM_CARD_NOT_READY;
+
+       return 0;

I think it should be return ret if rsi_set_sdio_pm_caps() fails.

This is intentional. We don't want to return error and abort system suspend operation due to this.

Regards,
Amitkumar Karwar

#3 lore raw
Quoted 21 lines · Amitkumar Karwar
Quoted 20 lines · Souptick Joarder
Quoted 19 lines · Amitkumar Karwar

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

---
v2: Replaced never ending while loop with 20msecs loop(Kalle Valo)

---
  drivers/net/wireless/rsi/rsi_91x_sdio.c | 128 +++++++++++++++++++++++++++++++-
  drivers/net/wireless/rsi/rsi_sdio.h     |   2 +
  2 files changed, 126 insertions(+), 4 deletions(-)
drivers/net/wireless/rsi/rsi_91x_sdio.c +0 -0
index 8d3a483..b3f8006 100644
@@ -1059,16 +1059,136 @@ static void rsi_disconnect(struct sdio_func *pfunction)

[...]

Quoted 26 lines
Quoted 17 lines
Quoted 17 lines
  static int rsi_suspend(struct device *dev)
  {
drivers/net/wireless/rsi/rsi_91x_sdio.c +13 -2
-       /* Not yet implemented */
-       return -ENOSYS;
+       int ret;
+       struct sdio_func *pfunction = dev_to_sdio_func(dev);
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+       struct rsi_common *common;
+
+       if (!adapter) {
+               rsi_dbg(ERR_ZONE, "Device is not ready\n");
+               return -ENODEV;
+       }
+       common = adapter->priv;
+       rsi_sdio_disable_interrupts(pfunction);
+
+       ret = rsi_set_sdio_pm_caps(adapter);
Quoted 8 lines

drivers/net/wireless/rsi/rsi_91x_sdio.c static void rsi_disconnect(struct sdio_func *pfunction) · line 1179

Quoted 6 lines
drivers/net/wireless/rsi/rsi_91x_sdio.c +6 -0
+       if (ret)
+               rsi_dbg(INFO_ZONE,
+                       "Setting power management caps failed\n");
+       common->fsm_state = FSM_CARD_NOT_READY;
+
+       return 0;

I think it should be return ret if rsi_set_sdio_pm_caps() fails.

Quoted 2 lines

This is intentional. We don't want to return error and abort system suspend operation due to this.

Has it been verified that powering down the SDIO bus during the suspend works for you device. In other words does the claim in the commit message apply to a sdio host controller not supporting the KEEP_POWER flag as well?

Regards,
Arend

#4 lore raw
Quoted 63 lines · Arend van Spriel
Quoted 29 lines · Amitkumar Karwar
Quoted 26 lines · Souptick Joarder
Quoted 23 lines · Amitkumar Karwar

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

---
v2: Replaced never ending while loop with 20msecs loop(Kalle Valo)

---
  drivers/net/wireless/rsi/rsi_91x_sdio.c | 128
+++++++++++++++++++++++++++++++-
  drivers/net/wireless/rsi/rsi_sdio.h     |   2 +
  2 files changed, 126 insertions(+), 4 deletions(-)
drivers/net/wireless/rsi/rsi_91x_sdio.c +0 -0
index 8d3a483..b3f8006 100644
@@ -1059,16 +1059,136 @@ static void rsi_disconnect(struct sdio_func
 *pfunction)

[...]

Quoted 28 lines
Quoted 26 lines
Quoted 13 lines
  static int rsi_suspend(struct device *dev)
  {
drivers/net/wireless/rsi/rsi_91x_sdio.c +9 -2
-       /* Not yet implemented */
-       return -ENOSYS;
+       int ret;
+       struct sdio_func *pfunction = dev_to_sdio_func(dev);
+       struct rsi_hw *adapter = sdio_get_drvdata(pfunction);
+       struct rsi_common *common;
+
+       if (!adapter) {
+               rsi_dbg(ERR_ZONE, "Device is not ready\n");
+               return -ENODEV;
+       }

drivers/net/wireless/rsi/rsi_91x_sdio.c static void rsi_disconnect(struct sdio_func *pfunction) · line 1179

Quoted 10 lines
drivers/net/wireless/rsi/rsi_91x_sdio.c +10 -0
+       common = adapter->priv;
+       rsi_sdio_disable_interrupts(pfunction);
+
+       ret = rsi_set_sdio_pm_caps(adapter);
+       if (ret)
+               rsi_dbg(INFO_ZONE,
+                       "Setting power management caps failed\n");
+       common->fsm_state = FSM_CARD_NOT_READY;
+
+       return 0;

I think it should be return ret if rsi_set_sdio_pm_caps() fails.

Quoted 7 lines
Quoted 2 lines

This is intentional. We don't want to return error and abort system suspend operation due to this.

Has it been verified that powering down the SDIO bus during the suspend works for you device. In other words does the claim in the commit message apply to a sdio host controller not supporting the KEEP_POWER flag as well?

Yes. It is verified. In this case, chip gets re enumerated and firmware will be re-downloaded after resume.

Regards,
Amitkumar Karwar

#5 lore raw
In reply to Amitkumar Karwar
Quoted 7 lines · Amitkumar Karwar

From: Karun Eagalapati <[email protected]>

SDIO suspend and resume handlers are implemented and verified that device works after suspend/resume cycle.

Patch applied to wireless-drivers-next.git, thanks.

20db07332736 rsi: sdio suspend and resume support

Signature · 3 lines
https://patchwork.kernel.org/patch/9963895/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches