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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120210114058.771f0e27@milhouse>
Date:	Fri, 10 Feb 2012 11:40:58 +0100
From:	Michael Büsch <m@...s.ch>
To:	Larry Finger <Larry.Finger@...inger.net>
Cc:	chunkeey@....de, jcmvbkbc@...il.com, linux-kernel@...r.kernel.org,
	devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org
Subject: Re: [RFC/RFT] p54spi: Convert driver to use asynchronous firmware
 loading

On Thu, 09 Feb 2012 19:05:37 -0600
Larry Finger <Larry.Finger@...inger.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/