| 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
| ||
|
Message-ID: <20170119113857.GQ28024@kroah.com>
Date: Thu, 19 Jan 2017 12:38:57 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc: ming.lei@...onical.com, bp@...en8.de, wagi@...om.org, teg@...m.no,
mchehab@....samsung.com, zajec5@...il.com,
linux-kernel@...r.kernel.org, markivx@...eaurora.org,
stephen.boyd@...aro.org, broonie@...nel.org,
zohar@...ux.vnet.ibm.com, tiwai@...e.de, johannes@...solutions.net,
chunkeey@...glemail.com, hauke@...ke-m.de,
jwboyer@...oraproject.org, dmitry.torokhov@...il.com,
dwmw2@...radead.org, jslaby@...e.com,
torvalds@...ux-foundation.org, luto@...capital.net,
fengguang.wu@...el.com, rpurdie@...ys.net,
j.anaszewski@...sung.com, Abhay_Salunke@...l.com,
Julia.Lawall@...6.fr, Gilles.Muller@...6.fr, nicolas.palix@...g.fr,
dhowells@...hat.com, bjorn.andersson@...aro.org,
arend.vanspriel@...adcom.com, kvalo@...eaurora.org
Subject: Re: [PATCH v4 3/3] p54: convert to sysdata API
On Thu, Jan 12, 2017 at 07:02:44AM -0800, Luis R. Rodriguez wrote:
> The Coccinelle sysdata patches were used to help with
> this transition. The changes have been carefully manually
> vetted for. With the conversion we modify the cases that do
> not need the firmware to be kept so that the sysdata API
> can release it for us. Using the new sysdata API also means
> we can get rid of our own completions.
>
> v2: was not present
> v3: initial release
> v4: small cosmetic fixes
> v5: bike shed changes
> v6: forgot to change one piece of code during the bikeshed name change
>
> Generated-by: Coccinelle SmPL
What is this tag for?
Also, meta-comment, put your vN: lines below the --- line like the
kernel documentation says to do.
> Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
> ---
> drivers/net/wireless/intersil/p54/eeprom.c | 2 +-
> drivers/net/wireless/intersil/p54/fwio.c | 5 +-
> drivers/net/wireless/intersil/p54/led.c | 2 +-
> drivers/net/wireless/intersil/p54/main.c | 2 +-
> drivers/net/wireless/intersil/p54/p54.h | 3 +-
> drivers/net/wireless/intersil/p54/p54pci.c | 26 ++++++----
> drivers/net/wireless/intersil/p54/p54pci.h | 4 +-
> drivers/net/wireless/intersil/p54/p54spi.c | 80 +++++++++++++++++++-----------
> drivers/net/wireless/intersil/p54/p54spi.h | 2 +-
> drivers/net/wireless/intersil/p54/p54usb.c | 18 +++----
> drivers/net/wireless/intersil/p54/p54usb.h | 4 +-
> drivers/net/wireless/intersil/p54/txrx.c | 2 +-
> 12 files changed, 89 insertions(+), 61 deletions(-)
why does the "new" api require more lines?
>
> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
> index d4c73d39336f..b8184cbc6770 100644
> --- a/drivers/net/wireless/intersil/p54/eeprom.c
> +++ b/drivers/net/wireless/intersil/p54/eeprom.c
> @@ -16,7 +16,7 @@
> * published by the Free Software Foundation.
> */
>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/etherdevice.h>
> #include <linux/sort.h>
> #include <linux/slab.h>
> diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
> index 4ac6764f4897..dc27049e4533 100644
> --- a/drivers/net/wireless/intersil/p54/fwio.c
> +++ b/drivers/net/wireless/intersil/p54/fwio.c
> @@ -17,7 +17,7 @@
> */
>
> #include <linux/slab.h>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/etherdevice.h>
> #include <linux/export.h>
>
> @@ -27,7 +27,8 @@
> #include "eeprom.h"
> #include "lmac.h"
>
> -int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
> +int p54_parse_firmware(struct ieee80211_hw *dev,
> + const struct drvdata *fw)
> {
> struct p54_common *priv = dev->priv;
> struct exp_if *exp_if;
> diff --git a/drivers/net/wireless/intersil/p54/led.c b/drivers/net/wireless/intersil/p54/led.c
> index 9a8fedd3c0f5..4d13598d3968 100644
> --- a/drivers/net/wireless/intersil/p54/led.c
> +++ b/drivers/net/wireless/intersil/p54/led.c
> @@ -16,7 +16,7 @@
> * published by the Free Software Foundation.
> */
>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/etherdevice.h>
>
> #include <net/mac80211.h>
> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
> index d5a3bf91a03e..a1c546cd232c 100644
> --- a/drivers/net/wireless/intersil/p54/main.c
> +++ b/drivers/net/wireless/intersil/p54/main.c
> @@ -17,7 +17,7 @@
> */
>
> #include <linux/slab.h>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/etherdevice.h>
> #include <linux/module.h>
>
> diff --git a/drivers/net/wireless/intersil/p54/p54.h b/drivers/net/wireless/intersil/p54/p54.h
> index 529939e611cd..5bbe9d77e5fc 100644
> --- a/drivers/net/wireless/intersil/p54/p54.h
> +++ b/drivers/net/wireless/intersil/p54/p54.h
> @@ -268,7 +268,8 @@ struct p54_common {
> /* interfaces for the drivers */
> int p54_rx(struct ieee80211_hw *dev, struct sk_buff *skb);
> void p54_free_skb(struct ieee80211_hw *dev, struct sk_buff *skb);
> -int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw);
> +int p54_parse_firmware(struct ieee80211_hw *dev,
> + const struct drvdata *fw);
> int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len);
> int p54_read_eeprom(struct ieee80211_hw *dev);
>
> diff --git a/drivers/net/wireless/intersil/p54/p54pci.c b/drivers/net/wireless/intersil/p54/p54pci.c
> index 27a49068d32d..0e7fd9ba7186 100644
> --- a/drivers/net/wireless/intersil/p54/p54pci.c
> +++ b/drivers/net/wireless/intersil/p54/p54pci.c
> @@ -15,7 +15,7 @@
>
> #include <linux/pci.h>
> #include <linux/slab.h>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/etherdevice.h>
> #include <linux/delay.h>
> #include <linux/completion.h>
> @@ -490,7 +490,7 @@ static int p54p_open(struct ieee80211_hw *dev)
> return 0;
> }
>
> -static void p54p_firmware_step2(const struct firmware *fw,
> +static void p54p_firmware_step2(const struct drvdata *fw,
> void *context)
> {
> struct p54p_priv *priv = context;
> @@ -520,8 +520,6 @@ static void p54p_firmware_step2(const struct firmware *fw,
>
> out:
>
> - complete(&priv->fw_loaded);
> -
> if (err) {
> struct device *parent = pdev->dev.parent;
>
> @@ -542,6 +540,17 @@ static void p54p_firmware_step2(const struct firmware *fw,
> pci_dev_put(pdev);
> }
>
> +static int p54p_load_firmware(struct p54p_priv *priv)
> +{
> + const struct drvdata_req_params req_params = {
> + DRVDATA_KEEP_ASYNC(p54p_firmware_step2, priv),
> + };
> +
> + return drvdata_request_async("isl3886pci", &req_params,
> + &priv->pdev->dev,
> + &priv->fw_async_cookie);
> +}
> +
> static int p54p_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> @@ -595,7 +604,6 @@ static int p54p_probe(struct pci_dev *pdev,
> priv = dev->priv;
> priv->pdev = pdev;
>
> - init_completion(&priv->fw_loaded);
> SET_IEEE80211_DEV(dev, &pdev->dev);
> pci_set_drvdata(pdev, dev);
>
> @@ -620,9 +628,7 @@ static int p54p_probe(struct pci_dev *pdev,
> spin_lock_init(&priv->lock);
> tasklet_init(&priv->tasklet, p54p_tasklet, (unsigned long)dev);
>
> - err = request_firmware_nowait(THIS_MODULE, 1, "isl3886pci",
> - &priv->pdev->dev, GFP_KERNEL,
> - priv, p54p_firmware_step2);
> + err = p54p_load_firmware(priv);
> if (!err)
> return 0;
>
> @@ -652,9 +658,9 @@ static void p54p_remove(struct pci_dev *pdev)
> return;
>
> priv = dev->priv;
> - wait_for_completion(&priv->fw_loaded);
> + drvdata_synchronize_request(priv->fw_async_cookie);
> p54_unregister_common(dev);
> - release_firmware(priv->firmware);
> + release_drvdata(priv->firmware);
> pci_free_consistent(pdev, sizeof(*priv->ring_control),
> priv->ring_control, priv->ring_control_dma);
> iounmap(priv->map);
> diff --git a/drivers/net/wireless/intersil/p54/p54pci.h b/drivers/net/wireless/intersil/p54/p54pci.h
> index 68405c142f97..00c30e1fc60b 100644
> --- a/drivers/net/wireless/intersil/p54/p54pci.h
> +++ b/drivers/net/wireless/intersil/p54/p54pci.h
> @@ -94,7 +94,7 @@ struct p54p_priv {
> struct pci_dev *pdev;
> struct p54p_csr __iomem *map;
> struct tasklet_struct tasklet;
> - const struct firmware *firmware;
> + const struct drvdata *firmware;
> spinlock_t lock;
> struct p54p_ring_control *ring_control;
> dma_addr_t ring_control_dma;
> @@ -105,7 +105,7 @@ struct p54p_priv {
> struct sk_buff *tx_buf_data[32];
> struct sk_buff *tx_buf_mgmt[4];
> struct completion boot_comp;
> - struct completion fw_loaded;
> + async_cookie_t fw_async_cookie;
> };
>
> #endif /* P54USB_H */
> diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
> index 7ab2f43ab425..c0118048c01f 100644
> --- a/drivers/net/wireless/intersil/p54/p54spi.c
> +++ b/drivers/net/wireless/intersil/p54/p54spi.c
> @@ -23,7 +23,7 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/interrupt.h>
> -#include <linux/firmware.h>
> +#include <linux/drvdata.h>
> #include <linux/delay.h>
> #include <linux/irq.h>
> #include <linux/spi/spi.h>
> @@ -162,53 +162,73 @@ static int p54spi_spi_write_dma(struct p54s_priv *priv, __le32 base,
> return 0;
> }
>
> +static int p54spi_request_firmware_found_cb(void *context,
> + const struct drvdata *drvdata)
> +{
> + int ret;
> + struct p54s_priv *priv = context;
> +
> + priv->firmware = drvdata;
> + ret = p54_parse_firmware(priv->hw, priv->firmware);
> + if (ret)
> + release_drvdata(priv->firmware);
> +
> + return ret;
> +}
> +
> static int p54spi_request_firmware(struct ieee80211_hw *dev)
> {
> struct p54s_priv *priv = dev->priv;
> + const struct drvdata_req_params req_params = {
> + DRVDATA_KEEP_SYNC(p54spi_request_firmware_found_cb, priv),
> + };
> int ret;
>
> /* FIXME: should driver use it's own struct device? */
> - ret = request_firmware(&priv->firmware, "3826.arm", &priv->spi->dev);
> -
> + ret = drvdata_request("3826.arm", &req_params, &priv->spi->dev);
> if (ret < 0) {
> - dev_err(&priv->spi->dev, "request_firmware() failed: %d", ret);
> - return ret;
> + dev_err(&priv->spi->dev,
> + "firmware request failed: %d", ret);
shouldn't the call report this error to the kernel log? Why must each
user print it out themselves again?
thanks,
greg k-h
Powered by blists - more mailing lists