| Message ID | 20190210195217.18817-2-lkundrak@v3.sk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | libertas_tf: fix setting the hardware address | expand |
On Sun, Feb 10, 2019 at 11:52 AM Lubomir Rintel <lkundrak@v3.sk> wrote: > > We'll need to talk to the firmware to get a hardware address before > device is registered with ieee80211 subsystem at the end of > lbtf_add_card(). Hooking the callbacks after that is too late. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > --- > drivers/net/wireless/marvell/libertas_tf/cmd.c | 2 +- > .../net/wireless/marvell/libertas_tf/if_usb.c | 12 +++++++----- > .../wireless/marvell/libertas_tf/libertas_tf.h | 17 +++++++++++------ > drivers/net/wireless/marvell/libertas_tf/main.c | 10 ++++++---- > 4 files changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/wireless/marvell/libertas_tf/cmd.c b/drivers/net/wireless/marvell/libertas_tf/cmd.c > index 64b147dd2432..130f578daafd 100644 > --- a/drivers/net/wireless/marvell/libertas_tf/cmd.c > +++ b/drivers/net/wireless/marvell/libertas_tf/cmd.c > @@ -256,7 +256,7 @@ static void lbtf_submit_command(struct lbtf_private *priv, > command, le16_to_cpu(cmd->seqnum), cmdsize); > lbtf_deb_hex(LBTF_DEB_CMD, "DNLD_CMD", (void *) cmdnode->cmdbuf, cmdsize); > > - ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) cmd, cmdsize); > + ret = priv->ops->hw_host_to_card(priv, MVMS_CMD, (u8 *)cmd, cmdsize); > spin_unlock_irqrestore(&priv->driver_lock, flags); > > if (ret) { > diff --git a/drivers/net/wireless/marvell/libertas_tf/if_usb.c b/drivers/net/wireless/marvell/libertas_tf/if_usb.c > index 6ede6168bd85..7a5a1a85dcf7 100644 > --- a/drivers/net/wireless/marvell/libertas_tf/if_usb.c > +++ b/drivers/net/wireless/marvell/libertas_tf/if_usb.c > @@ -131,6 +131,12 @@ static void if_usb_fw_timeo(struct timer_list *t) > lbtf_deb_leave(LBTF_DEB_USB); > } > > +static const struct lbtf_ops if_usb_ops = { > + .hw_host_to_card = if_usb_host_to_card, > + .hw_prog_firmware = if_usb_prog_firmware, > + .hw_reset_device = if_usb_reset_device, > +}; > + > /** > * if_usb_probe - sets the configuration values > * > @@ -216,15 +222,11 @@ static int if_usb_probe(struct usb_interface *intf, > goto dealloc; > } > > - priv = lbtf_add_card(cardp, &udev->dev); > + priv = lbtf_add_card(cardp, &udev->dev, &if_usb_ops); > if (!priv) > goto dealloc; > > cardp->priv = priv; > - > - priv->hw_host_to_card = if_usb_host_to_card; > - priv->hw_prog_firmware = if_usb_prog_firmware; > - priv->hw_reset_device = if_usb_reset_device; > cardp->boot2_version = udev->descriptor.bcdDevice; > > usb_get_dev(udev); > diff --git a/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h b/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h > index ad77b92d0b41..11d5ff68bc5e 100644 > --- a/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h > +++ b/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h > @@ -173,10 +173,19 @@ struct channel_range { > > struct if_usb_card; > > +struct lbtf_ops { > + /** Hardware access */ > + int (*hw_host_to_card)(struct lbtf_private *priv, u8 type, > + u8 *payload, u16 nb); > + int (*hw_prog_firmware)(struct if_usb_card *cardp); > + int (*hw_reset_device)(struct if_usb_card *cardp); > +}; > + > /** Private structure for the MV device */ > struct lbtf_private { > void *card; > struct ieee80211_hw *hw; > + const struct lbtf_ops *ops; > > /* Command response buffer */ > u8 cmd_resp_buff[LBS_UPLD_SIZE]; > @@ -188,11 +197,6 @@ struct lbtf_private { > > struct work_struct cmd_work; > struct work_struct tx_work; > - /** Hardware access */ > - int (*hw_host_to_card) (struct lbtf_private *priv, u8 type, u8 *payload, u16 nb); > - int (*hw_prog_firmware) (struct if_usb_card *cardp); > - int (*hw_reset_device) (struct if_usb_card *cardp); > - > > /** Wlan adapter data structure*/ > /** STATUS variables */ > @@ -486,7 +490,8 @@ void lbtf_cmd_response_rx(struct lbtf_private *priv); > /* main.c */ > struct chan_freq_power *lbtf_get_region_cfp_table(u8 region, > int *cfp_no); > -struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev); > +struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev, > + const struct lbtf_ops *ops); > int lbtf_remove_card(struct lbtf_private *priv); > int lbtf_start_card(struct lbtf_private *priv); > int lbtf_rx(struct lbtf_private *priv, struct sk_buff *skb); > diff --git a/drivers/net/wireless/marvell/libertas_tf/main.c b/drivers/net/wireless/marvell/libertas_tf/main.c > index f93b400db949..8ed3cd158cf5 100644 > --- a/drivers/net/wireless/marvell/libertas_tf/main.c > +++ b/drivers/net/wireless/marvell/libertas_tf/main.c > @@ -281,7 +281,7 @@ static void lbtf_tx_work(struct work_struct *work) > BUG_ON(priv->tx_skb); > spin_lock_irq(&priv->driver_lock); > priv->tx_skb = skb; > - err = priv->hw_host_to_card(priv, MVMS_DAT, skb->data, skb->len); > + err = priv->ops->hw_host_to_card(priv, MVMS_DAT, skb->data, skb->len); > spin_unlock_irq(&priv->driver_lock); > if (err) { > dev_kfree_skb_any(skb); > @@ -301,7 +301,7 @@ static int lbtf_op_start(struct ieee80211_hw *hw) > > if (!priv->fw_ready) > /* Upload firmware */ > - if (priv->hw_prog_firmware(card)) > + if (priv->ops->hw_prog_firmware(card)) > goto err_prog_firmware; > > /* poke the firmware */ > @@ -322,7 +322,7 @@ static int lbtf_op_start(struct ieee80211_hw *hw) > return 0; > > err_prog_firmware: > - priv->hw_reset_device(card); > + priv->ops->hw_reset_device(card); > lbtf_deb_leave_args(LBTF_DEB_MACOPS, "error programming fw; ret=%d", ret); > return ret; > } > @@ -605,7 +605,8 @@ EXPORT_SYMBOL_GPL(lbtf_rx); > * > * Returns: pointer to struct lbtf_priv. > */ > -struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev) > +struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev, > + const struct lbtf_ops *ops) > { > struct ieee80211_hw *hw; > struct lbtf_private *priv = NULL; > @@ -622,6 +623,7 @@ struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev) > > priv->hw = hw; > priv->card = card; > + priv->ops = ops; > priv->tx_skb = NULL; > > hw->queues = 1; > -- > 2.20.1 > Reviewed-by: Steve deRosier <derosier@cal-sierra.com>
Lubomir Rintel <lkundrak@v3.sk> wrote: > We'll need to talk to the firmware to get a hardware address before > device is registered with ieee80211 subsystem at the end of > lbtf_add_card(). Hooking the callbacks after that is too late. > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> > Reviewed-by: Steve deRosier <derosier@cal-sierra.com> Failed to compile: drivers/net/wireless/marvell/libertas_tf/main.c: In function 'lbtf_add_card': drivers/net/wireless/marvell/libertas_tf/main.c:603:309: error: 'udev' undeclared (first use in this function); did you mean 'cdev'? lbtf_deb_usbd(&udev->dev, "Error programming the firmware\n"); ^ cdev drivers/net/wireless/marvell/libertas_tf/main.c:603:309: note: each undeclared identifier is reported only once for each function it appears in make[5]: *** [drivers/net/wireless/marvell/libertas_tf/main.o] Error 1 make[5]: *** Waiting for unfinished jobs.... make[4]: *** [drivers/net/wireless/marvell/libertas_tf] Error 2 make[3]: *** [drivers/net/wireless/marvell] Error 2 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [drivers/net/wireless] Error 2 make[1]: *** [drivers/net] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [drivers] Error 2 3 patches set to Changes Requested. 10804885 [1/3] libertas_tf: move hardware callbacks to a separate structure 10804881 [2/3] libertas_tf: don't defer firmware loading until start() 10804883 [3/3] libertas_tf: get the MAC address before registering the device
diff --git a/drivers/net/wireless/marvell/libertas_tf/cmd.c b/drivers/net/wireless/marvell/libertas_tf/cmd.c index 64b147dd2432..130f578daafd 100644 --- a/drivers/net/wireless/marvell/libertas_tf/cmd.c +++ b/drivers/net/wireless/marvell/libertas_tf/cmd.c @@ -256,7 +256,7 @@ static void lbtf_submit_command(struct lbtf_private *priv, command, le16_to_cpu(cmd->seqnum), cmdsize); lbtf_deb_hex(LBTF_DEB_CMD, "DNLD_CMD", (void *) cmdnode->cmdbuf, cmdsize); - ret = priv->hw_host_to_card(priv, MVMS_CMD, (u8 *) cmd, cmdsize); + ret = priv->ops->hw_host_to_card(priv, MVMS_CMD, (u8 *)cmd, cmdsize); spin_unlock_irqrestore(&priv->driver_lock, flags); if (ret) { diff --git a/drivers/net/wireless/marvell/libertas_tf/if_usb.c b/drivers/net/wireless/marvell/libertas_tf/if_usb.c index 6ede6168bd85..7a5a1a85dcf7 100644 --- a/drivers/net/wireless/marvell/libertas_tf/if_usb.c +++ b/drivers/net/wireless/marvell/libertas_tf/if_usb.c @@ -131,6 +131,12 @@ static void if_usb_fw_timeo(struct timer_list *t) lbtf_deb_leave(LBTF_DEB_USB); } +static const struct lbtf_ops if_usb_ops = { + .hw_host_to_card = if_usb_host_to_card, + .hw_prog_firmware = if_usb_prog_firmware, + .hw_reset_device = if_usb_reset_device, +}; + /** * if_usb_probe - sets the configuration values * @@ -216,15 +222,11 @@ static int if_usb_probe(struct usb_interface *intf, goto dealloc; } - priv = lbtf_add_card(cardp, &udev->dev); + priv = lbtf_add_card(cardp, &udev->dev, &if_usb_ops); if (!priv) goto dealloc; cardp->priv = priv; - - priv->hw_host_to_card = if_usb_host_to_card; - priv->hw_prog_firmware = if_usb_prog_firmware; - priv->hw_reset_device = if_usb_reset_device; cardp->boot2_version = udev->descriptor.bcdDevice; usb_get_dev(udev); diff --git a/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h b/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h index ad77b92d0b41..11d5ff68bc5e 100644 --- a/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h +++ b/drivers/net/wireless/marvell/libertas_tf/libertas_tf.h @@ -173,10 +173,19 @@ struct channel_range { struct if_usb_card; +struct lbtf_ops { + /** Hardware access */ + int (*hw_host_to_card)(struct lbtf_private *priv, u8 type, + u8 *payload, u16 nb); + int (*hw_prog_firmware)(struct if_usb_card *cardp); + int (*hw_reset_device)(struct if_usb_card *cardp); +}; + /** Private structure for the MV device */ struct lbtf_private { void *card; struct ieee80211_hw *hw; + const struct lbtf_ops *ops; /* Command response buffer */ u8 cmd_resp_buff[LBS_UPLD_SIZE]; @@ -188,11 +197,6 @@ struct lbtf_private { struct work_struct cmd_work; struct work_struct tx_work; - /** Hardware access */ - int (*hw_host_to_card) (struct lbtf_private *priv, u8 type, u8 *payload, u16 nb); - int (*hw_prog_firmware) (struct if_usb_card *cardp); - int (*hw_reset_device) (struct if_usb_card *cardp); - /** Wlan adapter data structure*/ /** STATUS variables */ @@ -486,7 +490,8 @@ void lbtf_cmd_response_rx(struct lbtf_private *priv); /* main.c */ struct chan_freq_power *lbtf_get_region_cfp_table(u8 region, int *cfp_no); -struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev); +struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev, + const struct lbtf_ops *ops); int lbtf_remove_card(struct lbtf_private *priv); int lbtf_start_card(struct lbtf_private *priv); int lbtf_rx(struct lbtf_private *priv, struct sk_buff *skb); diff --git a/drivers/net/wireless/marvell/libertas_tf/main.c b/drivers/net/wireless/marvell/libertas_tf/main.c index f93b400db949..8ed3cd158cf5 100644 --- a/drivers/net/wireless/marvell/libertas_tf/main.c +++ b/drivers/net/wireless/marvell/libertas_tf/main.c @@ -281,7 +281,7 @@ static void lbtf_tx_work(struct work_struct *work) BUG_ON(priv->tx_skb); spin_lock_irq(&priv->driver_lock); priv->tx_skb = skb; - err = priv->hw_host_to_card(priv, MVMS_DAT, skb->data, skb->len); + err = priv->ops->hw_host_to_card(priv, MVMS_DAT, skb->data, skb->len); spin_unlock_irq(&priv->driver_lock); if (err) { dev_kfree_skb_any(skb); @@ -301,7 +301,7 @@ static int lbtf_op_start(struct ieee80211_hw *hw) if (!priv->fw_ready) /* Upload firmware */ - if (priv->hw_prog_firmware(card)) + if (priv->ops->hw_prog_firmware(card)) goto err_prog_firmware; /* poke the firmware */ @@ -322,7 +322,7 @@ static int lbtf_op_start(struct ieee80211_hw *hw) return 0; err_prog_firmware: - priv->hw_reset_device(card); + priv->ops->hw_reset_device(card); lbtf_deb_leave_args(LBTF_DEB_MACOPS, "error programming fw; ret=%d", ret); return ret; } @@ -605,7 +605,8 @@ EXPORT_SYMBOL_GPL(lbtf_rx); * * Returns: pointer to struct lbtf_priv. */ -struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev) +struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev, + const struct lbtf_ops *ops) { struct ieee80211_hw *hw; struct lbtf_private *priv = NULL; @@ -622,6 +623,7 @@ struct lbtf_private *lbtf_add_card(void *card, struct device *dmdev) priv->hw = hw; priv->card = card; + priv->ops = ops; priv->tx_skb = NULL; hw->queues = 1;
We'll need to talk to the firmware to get a hardware address before device is registered with ieee80211 subsystem at the end of lbtf_add_card(). Hooking the callbacks after that is too late. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/net/wireless/marvell/libertas_tf/cmd.c | 2 +- .../net/wireless/marvell/libertas_tf/if_usb.c | 12 +++++++----- .../wireless/marvell/libertas_tf/libertas_tf.h | 17 +++++++++++------ drivers/net/wireless/marvell/libertas_tf/main.c | 10 ++++++---- 4 files changed, 25 insertions(+), 16 deletions(-)