Re: [RFC/RFT] p54spi: Convert driver to use asynchronous firmware loading

["Michael Büsch" <m@bues.ch>]

On Thu, 09 Feb 2012 19:05:37 -0600
Larry Finger <Larry.Finger@lwfinger.net> wrote:

> Index: wireless-testing-new/drivers/net/wireless/p54/p54spi.c
> ===================================================================
> --- wireless-testing-new.orig/drivers/net/wireless/p54/p54spi.c
> +++ wireless-testing-new/drivers/net/wireless/p54/p54spi.c
> @@ -163,25 +163,41 @@ static int p54spi_spi_write_dma(struct p
>  	return 0;
>  }
>  
> -static int p54spi_request_firmware(struct ieee80211_hw *dev)
> +
> +static void p54s_load_firmware_cb(const struct firmware *firmware,
> +				  void *context)
>  {
> +	struct ieee80211_hw *dev = context;
>  	struct p54s_priv *priv = dev->priv;
>  	int ret;
>  
> -	/* FIXME: should driver use it's own struct device? */
> -	ret = request_firmware(&priv->firmware, "3826.arm", &priv->spi->dev);
> -
> -	if (ret < 0) {
> -		dev_err(&priv->spi->dev, "request_firmware() failed: %d", ret);
> -		return ret;
> +	if (!firmware) {
> +		/* alternate firmware not found */
> +		dev_err(&priv->spi->dev, "Firmware loading failed\n");
> +		return;
>  	}
> +	complete(&priv->fw_loaded);
> +	priv->firmware = firmware;
>  
>  	ret = p54_parse_firmware(dev, priv->firmware);
> -	if (ret) {
> +	if (ret)
>  		release_firmware(priv->firmware);
> +}
> +
> +static int p54spi_request_firmware(struct ieee80211_hw *dev)
> +{
> +	struct p54s_priv *priv = dev->priv;
> +	int ret;
> +
> +	init_completion(&priv->fw_loaded);
> +	/* FIXME: should driver use it's own struct device? */
> +	ret = request_firmware_nowait(THIS_MODULE, 1, "3826.arm",
> +				      &priv->spi->dev, GFP_KERNEL, dev,
> +				      p54s_load_firmware_cb);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "request_firmware() failed: %d", ret);
>  		return ret;
>  	}
> -
>  	return 0;
>  }

I think we need a wait_for_completion in op_start. We need to make sure firmware load is finished.
It might currently work, though, because we have the other fw_comp completion.
Please also rename your new completion to fw_requested or something like this to avoid
confusion with the real fw-loaded completion.

Also, what about eeprom request?

> @@ -681,6 +697,7 @@ static int __devexit p54spi_remove(struc
>  {
>  	struct p54s_priv *priv = dev_get_drvdata(&spi->dev);
>  
> +	wait_for_completion(&priv->fw_loaded);
>  	p54_unregister_common(priv->hw);
>  
>  	free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
> Index: wireless-testing-new/drivers/net/wireless/p54/p54spi.h
> ===================================================================
> --- wireless-testing-new.orig/drivers/net/wireless/p54/p54spi.h
> +++ wireless-testing-new/drivers/net/wireless/p54/p54spi.h
> @@ -120,6 +120,7 @@ struct p54s_priv {
>  
>  	enum fw_state fw_state;
>  	const struct firmware *firmware;
> +	struct completion fw_loaded;
>  };
>  
>  #endif /* P54SPI_H */
> 



-- 
Greetings, Michael.