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

From: Michael BÃsch
Date: Fri Feb 10 2012 - 05:41:41 EST


On Thu, 09 Feb 2012 19:05:37 -0600
Larry Finger <Larry.Finger@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/