Re: iwl4965: "MAC is in deep sleep!" freezes

[Stanislaw Gruszka]

On Thu, Sep 08, 2011 at 10:49:13AM +0200, Stanislaw Gruszka wrote:
> On Mon, Sep 05, 2011 at 09:42:01AM +0000, Christoph Anton Mitterer wrote:
> > On Mon, 5 Sep 2011 11:19:18 +0200, Stanislaw Gruszka <sgruszka@xxxxxxxxxx>
> > wrote:
> > > this look like firmware hang. This happen just after module load, that's
> > > good news, because it allow to log relative small amount debug messages
> > > to see what possibly driver do wrong to crash firmware.
> > Note, that the effects of - usually but not always - frozen system while
> > the errors are printed (sometimes keyboard input seems still be possible
> > during that) ... happen over and over again... say about every  10 minutes
> > or so.
> > Not just the first time the modules is loaded.
> > Is the firmware loaded over and over again?
> I think is not, but even if is, kernel should not crash. Can you grab
> crash logs using photo camera, serial cable or netconsole, to allow to 
> see where exactly crash happen and fix it.
> 
> > > Please configure syslog to log kernel debug messages.
> > Will mail it later.
> 
> Firmware hang seems to happen when REPLY_LEDS_CMD command is send.
> No idea if attached patch could help or not, but is worth to try it.
> It prevent to send LED command asynchronously, when possibly some
> other commands are pending in firmware.
> 
> If it does not help, please provide the same way captured debug
> messages from 2.6.38 kernel, where drivers works. I'll compare
> it with broken driver logs to figure out what broken driver
> do differently.

Sorry, forgot attch the patch, do it now.

Stanislaw
diff --git a/drivers/net/wireless/iwlegacy/iwl-3945-led.c b/drivers/net/wireless/iwlegacy/iwl-3945-led.c
index abd9235..127ffd6 100644
--- a/drivers/net/wireless/iwlegacy/iwl-3945-led.c
+++ b/drivers/net/wireless/iwlegacy/iwl-3945-led.c
@@ -52,8 +52,7 @@ static int iwl3945_send_led_cmd(struct iwl_priv *priv,
 		.id = REPLY_LEDS_CMD,
 		.len = sizeof(struct iwl_led_cmd),
 		.data = led_cmd,
-		.flags = CMD_ASYNC,
-		.callback = NULL,
+		.flags = CMD_SYNC,
 	};
 
 	return iwl_legacy_send_cmd(priv, &cmd);
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-led.c b/drivers/net/wireless/iwlegacy/iwl-4965-led.c
index 26d324e..2545c19 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-led.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-led.c
@@ -51,8 +51,7 @@ iwl4965_send_led_cmd(struct iwl_priv *priv, struct iwl_led_cmd *led_cmd)
 		.id = REPLY_LEDS_CMD,
 		.len = sizeof(struct iwl_led_cmd),
 		.data = led_cmd,
-		.flags = CMD_ASYNC,
-		.callback = NULL,
+		.flags = CMD_SYNC,
 	};
 	u32 reg;
 
diff --git a/drivers/net/wireless/iwlegacy/iwl-dev.h b/drivers/net/wireless/iwlegacy/iwl-dev.h
index 9c786ed..2623c79 100644
--- a/drivers/net/wireless/iwlegacy/iwl-dev.h
+++ b/drivers/net/wireless/iwlegacy/iwl-dev.h
@@ -1211,6 +1211,8 @@ struct iwl_priv {
 	struct delayed_work alive_start;
 	struct delayed_work scan_check;
 
+	struct work_struct led_work;
+
 	/* TX Power */
 	s8 tx_power_user_lmt;
 	s8 tx_power_device_lmt;
diff --git a/drivers/net/wireless/iwlegacy/iwl-led.c b/drivers/net/wireless/iwlegacy/iwl-led.c
index bda0d61..2152d87 100644
--- a/drivers/net/wireless/iwlegacy/iwl-led.c
+++ b/drivers/net/wireless/iwlegacy/iwl-led.c
@@ -106,13 +106,6 @@ static int iwl_legacy_led_cmd(struct iwl_priv *priv,
 		.id = IWL_LED_LINK,
 		.interval = IWL_DEF_LED_INTRVL
 	};
-	int ret;
-
-	if (!test_bit(STATUS_READY, &priv->status))
-		return -EBUSY;
-
-	if (priv->blink_on == on && priv->blink_off == off)
-		return 0;
 
 	if (off == 0) {
 		/* led is SOLID_ON */
@@ -126,24 +119,25 @@ static int iwl_legacy_led_cmd(struct iwl_priv *priv,
 	led_cmd.off = iwl_legacy_blink_compensation(priv, off,
 				priv->cfg->base_params->led_compensation);
 
-	ret = priv->cfg->ops->led->cmd(priv, &led_cmd);
-	if (!ret) {
-		priv->blink_on = on;
-		priv->blink_off = off;
-	}
-	return ret;
+	return priv->cfg->ops->led->cmd(priv, &led_cmd);
 }
 
 static void iwl_legacy_led_brightness_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
 	struct iwl_priv *priv = container_of(led_cdev, struct iwl_priv, led);
-	unsigned long on = 0;
+	unsigned long on = (brightness > 0) ? IWL_LED_SOLID : 0;
 
-	if (brightness > 0)
-		on = IWL_LED_SOLID;
+	if (!test_bit(STATUS_READY, &priv->status))
+		return;
+
+	if (priv->blink_on == on && priv->blink_off == 0)
+		return;
+
+	priv->blink_on = on;
+	priv->blink_off = 0;
 
-	iwl_legacy_led_cmd(priv, on, 0);
+	queue_work(priv->workqueue, &priv->led_work);
 }
 
 static int iwl_legacy_led_blink_set(struct led_classdev *led_cdev,
@@ -152,7 +146,28 @@ static int iwl_legacy_led_blink_set(struct led_classdev *led_cdev,
 {
 	struct iwl_priv *priv = container_of(led_cdev, struct iwl_priv, led);
 
-	return iwl_legacy_led_cmd(priv, *delay_on, *delay_off);
+	if (!test_bit(STATUS_READY, &priv->status))
+		return -EBUSY;
+
+	if (priv->blink_on == *delay_on && priv->blink_off == *delay_off)
+		return 0;
+
+	/* FIXME: Need spinlock to sync with possibly currently running led work? */
+	priv->blink_on = *delay_on;
+	priv->blink_off = *delay_off;
+
+	queue_work(priv->workqueue, &priv->led_work);
+	
+	return 0;
+}
+
+static void iwl_legacy_bg_led(struct work_struct *work)
+{
+	struct iwl_priv *priv = container_of(work, struct iwl_priv, led_work);
+
+	mutex_lock(&priv->mutex);
+	iwl_legacy_led_cmd(priv, priv->blink_on, priv->blink_off);
+	mutex_unlock(&priv->mutex);
 }
 
 void iwl_legacy_leds_init(struct iwl_priv *priv)
@@ -160,6 +175,8 @@ void iwl_legacy_leds_init(struct iwl_priv *priv)
 	int mode = led_mode;
 	int ret;
 
+	INIT_WORK(&priv->led_work, iwl_legacy_bg_led);
+
 	if (mode == IWL_LED_DEFAULT)
 		mode = priv->cfg->led_mode;
 
@@ -202,5 +219,7 @@ void iwl_legacy_leds_exit(struct iwl_priv *priv)
 
 	led_classdev_unregister(&priv->led);
 	kfree(priv->led.name);
+
+	cancel_work_sync(&priv->led_work);
 }
 EXPORT_SYMBOL(iwl_legacy_leds_exit);