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

[Stanislaw Gruszka]

On Thu, Sep 08, 2011 at 06:19:56PM +0200, Stanislaw Gruszka wrote:
> 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.

Any feedback here?

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);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/