From: Rafał Miłecki <[email protected]>
This helper function is pretty trivial and we can easily do without it.
What's important though it's one of FWS (Firmware Signalling)
dependencies in core.c. The plan is to make FWS required by BCDC only so
we don't have to use/compile it when using msgbuf.
Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 10 ----------
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h | 4 ++++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++++--
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 7 +++++--
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 2f2f3a5ad86a..59831dc446d6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -283,16 +283,6 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
spin_unlock_irqrestore(&ifp->netif_stop_lock, flags);
}
-void brcmf_txflowblock(struct device *dev, bool state)
-{
- struct brcmf_bus *bus_if = dev_get_drvdata(dev);
- struct brcmf_pub *drvr = bus_if->drvr;
-
- brcmf_dbg(TRACE, "Enter\n");
-
- brcmf_fws_bus_blocked(drvr, state);
-}
-
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
{
if (skb->pkt_type == PACKET_MULTICAST)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
index 96df66073b2a..bb4591c4a14a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
@@ -18,6 +18,10 @@
#ifndef FWSIGNAL_H_
#define FWSIGNAL_H_
+struct brcmf_fws_info;
+struct brcmf_if;
+struct brcmf_pub;
+
int brcmf_fws_init(struct brcmf_pub *drvr);
void brcmf_fws_deinit(struct brcmf_pub *drvr);
bool brcmf_fws_queue_skbs(struct brcmf_fws_info *fws);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index c5744b45ec8f..10522edc9750 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -44,6 +44,7 @@
#include "firmware.h"
#include "core.h"
#include "common.h"
+#include "fwsignal.h"
#define DCMD_RESP_TIMEOUT msecs_to_jiffies(2500)
#define CTL_DONE_TIMEOUT msecs_to_jiffies(2500)
@@ -2272,6 +2273,7 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
{
+ struct brcmf_pub *pub = bus->sdiodev->bus_if->drvr;
struct sk_buff *pkt;
struct sk_buff_head pktq;
u32 intstatus = 0;
@@ -2328,7 +2330,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
if ((bus->sdiodev->state == BRCMF_SDIOD_DATA) &&
bus->txoff && (pktq_len(&bus->txq) < TXLOW)) {
bus->txoff = false;
- brcmf_txflowblock(bus->sdiodev->dev, false);
+ brcmf_fws_bus_blocked(pub, false);
}
return cnt;
@@ -2723,6 +2725,7 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
struct brcmf_sdio *bus = sdiodev->bus;
+ struct brcmf_pub *pub = bus->sdiodev->bus_if->drvr;
brcmf_dbg(TRACE, "Enter: pkt: data %p len %d\n", pkt->data, pkt->len);
if (sdiodev->state != BRCMF_SDIOD_DATA)
@@ -2753,7 +2756,7 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
if (pktq_len(&bus->txq) >= TXHI) {
bus->txoff = true;
- brcmf_txflowblock(dev, true);
+ brcmf_fws_bus_blocked(pub, true);
}
spin_unlock_bh(&bus->txq_lock);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index d93ebbdc7737..c527aa8a5f8f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -26,6 +26,7 @@
#include "bus.h"
#include "debug.h"
#include "firmware.h"
+#include "fwsignal.h"
#include "usb.h"
#include "core.h"
#include "common.h"
@@ -476,6 +477,7 @@ static void brcmf_usb_tx_complete(struct urb *urb)
{
struct brcmf_usbreq *req = (struct brcmf_usbreq *)urb->context;
struct brcmf_usbdev_info *devinfo = req->devinfo;
+ struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
unsigned long flags;
brcmf_dbg(USB, "Enter, urb->status=%d, skb=%p\n", urb->status,
@@ -488,7 +490,7 @@ static void brcmf_usb_tx_complete(struct urb *urb)
spin_lock_irqsave(&devinfo->tx_flowblock_lock, flags);
if (devinfo->tx_freecount > devinfo->tx_high_watermark &&
devinfo->tx_flowblock) {
- brcmf_txflowblock(devinfo->dev, false);
+ brcmf_fws_bus_blocked(pub, false);
devinfo->tx_flowblock = false;
}
spin_unlock_irqrestore(&devinfo->tx_flowblock_lock, flags);
@@ -598,6 +600,7 @@ brcmf_usb_state_change(struct brcmf_usbdev_info *devinfo, int state)
static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
{
struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
+ struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
struct brcmf_usbreq *req;
int ret;
unsigned long flags;
@@ -635,7 +638,7 @@ static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
spin_lock_irqsave(&devinfo->tx_flowblock_lock, flags);
if (devinfo->tx_freecount < devinfo->tx_low_watermark &&
!devinfo->tx_flowblock) {
- brcmf_txflowblock(dev, true);
+ brcmf_fws_bus_blocked(pub, true);
devinfo->tx_flowblock = true;
}
spin_unlock_irqrestore(&devinfo->tx_flowblock_lock, flags);
--
2.11.0
On 28-2-2017 11:00, Rafał Miłecki wrote:
> On 28 February 2017 at 10:50, Arend Van Spriel
> <[email protected]> wrote:
>> On 27-2-2017 13:06, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> This helper function is pretty trivial and we can easily do without it.
>>> What's important though it's one of FWS (Firmware Signalling)
>>> dependencies in core.c. The plan is to make FWS required by BCDC only so
>>> we don't have to use/compile it when using msgbuf.
>>
>> This is the same discussion as before. Our driver design really wants to
>> keep bus-specific code separated from common code. Adding more and more
>> include statements is breaking that design. Whether or not that
>> resembles the way other drivers do it is not really a consideration. So
>> I would rather like to see patches that improve that separation.
>>
>> I will see if I can publish the design summary on our wiki page.
>
> You may not like this solution, but if so, please suggest another one.
> Then we can discuss two of them & find a final one.
>
> As you see I'm trying to drop fws dependency from core.c. It's what
> was very roughly discussed in:
> brcmfmac: initialize fws(ignal) for BCDC protocol only
> https://patchwork.kernel.org/patch/9349301/
>
> My guess if you have another patch for this, but since you didn't
> manage to release it since September, I'd really like to move things
> forward somehow.
Franky has taken it on his shoulders, but it seems to have slipped under
the rug. Maybe he can chime in.
Regards,
Arend
On 28 February 2017 at 10:50, Arend Van Spriel
<[email protected]> wrote:
> On 27-2-2017 13:06, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>
>> This helper function is pretty trivial and we can easily do without it.
>> What's important though it's one of FWS (Firmware Signalling)
>> dependencies in core.c. The plan is to make FWS required by BCDC only so
>> we don't have to use/compile it when using msgbuf.
>
> This is the same discussion as before. Our driver design really wants to
> keep bus-specific code separated from common code. Adding more and more
> include statements is breaking that design. Whether or not that
> resembles the way other drivers do it is not really a consideration. So
> I would rather like to see patches that improve that separation.
>
> I will see if I can publish the design summary on our wiki page.
You may not like this solution, but if so, please suggest another one.
Then we can discuss two of them & find a final one.
As you see I'm trying to drop fws dependency from core.c. It's what
was very roughly discussed in:
brcmfmac: initialize fws(ignal) for BCDC protocol only
https://patchwork.kernel.org/patch/9349301/
My guess if you have another patch for this, but since you didn't
manage to release it since September, I'd really like to move things
forward somehow.
On 27-2-2017 13:06, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> This helper function is pretty trivial and we can easily do without it.
> What's important though it's one of FWS (Firmware Signalling)
> dependencies in core.c. The plan is to make FWS required by BCDC only so
> we don't have to use/compile it when using msgbuf.
This is the same discussion as before. Our driver design really wants to
keep bus-specific code separated from common code. Adding more and more
include statements is breaking that design. Whether or not that
resembles the way other drivers do it is not really a consideration. So
I would rather like to see patches that improve that separation.
I will see if I can publish the design summary on our wiki page.
Regards,
Arend
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 10 ----------
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h | 4 ++++
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++++--
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 7 +++++--
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 2f2f3a5ad86a..59831dc446d6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -283,16 +283,6 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
> spin_unlock_irqrestore(&ifp->netif_stop_lock, flags);
> }
>
> -void brcmf_txflowblock(struct device *dev, bool state)
> -{
> - struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> - struct brcmf_pub *drvr = bus_if->drvr;
> -
> - brcmf_dbg(TRACE, "Enter\n");
> -
> - brcmf_fws_bus_blocked(drvr, state);
> -}
> -
> void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
> {
> if (skb->pkt_type == PACKET_MULTICAST)
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
> index 96df66073b2a..bb4591c4a14a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
> @@ -18,6 +18,10 @@
> #ifndef FWSIGNAL_H_
> #define FWSIGNAL_H_
>
> +struct brcmf_fws_info;
> +struct brcmf_if;
> +struct brcmf_pub;
> +
> int brcmf_fws_init(struct brcmf_pub *drvr);
> void brcmf_fws_deinit(struct brcmf_pub *drvr);
> bool brcmf_fws_queue_skbs(struct brcmf_fws_info *fws);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index c5744b45ec8f..10522edc9750 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -44,6 +44,7 @@
> #include "firmware.h"
> #include "core.h"
> #include "common.h"
> +#include "fwsignal.h"
>
> #define DCMD_RESP_TIMEOUT msecs_to_jiffies(2500)
> #define CTL_DONE_TIMEOUT msecs_to_jiffies(2500)
> @@ -2272,6 +2273,7 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
>
> static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
> {
> + struct brcmf_pub *pub = bus->sdiodev->bus_if->drvr;
> struct sk_buff *pkt;
> struct sk_buff_head pktq;
> u32 intstatus = 0;
> @@ -2328,7 +2330,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
> if ((bus->sdiodev->state == BRCMF_SDIOD_DATA) &&
> bus->txoff && (pktq_len(&bus->txq) < TXLOW)) {
> bus->txoff = false;
> - brcmf_txflowblock(bus->sdiodev->dev, false);
> + brcmf_fws_bus_blocked(pub, false);
> }
>
> return cnt;
> @@ -2723,6 +2725,7 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
> struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> struct brcmf_sdio *bus = sdiodev->bus;
> + struct brcmf_pub *pub = bus->sdiodev->bus_if->drvr;
>
> brcmf_dbg(TRACE, "Enter: pkt: data %p len %d\n", pkt->data, pkt->len);
> if (sdiodev->state != BRCMF_SDIOD_DATA)
> @@ -2753,7 +2756,7 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
>
> if (pktq_len(&bus->txq) >= TXHI) {
> bus->txoff = true;
> - brcmf_txflowblock(dev, true);
> + brcmf_fws_bus_blocked(pub, true);
> }
> spin_unlock_bh(&bus->txq_lock);
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> index d93ebbdc7737..c527aa8a5f8f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> @@ -26,6 +26,7 @@
> #include "bus.h"
> #include "debug.h"
> #include "firmware.h"
> +#include "fwsignal.h"
> #include "usb.h"
> #include "core.h"
> #include "common.h"
> @@ -476,6 +477,7 @@ static void brcmf_usb_tx_complete(struct urb *urb)
> {
> struct brcmf_usbreq *req = (struct brcmf_usbreq *)urb->context;
> struct brcmf_usbdev_info *devinfo = req->devinfo;
> + struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
> unsigned long flags;
>
> brcmf_dbg(USB, "Enter, urb->status=%d, skb=%p\n", urb->status,
> @@ -488,7 +490,7 @@ static void brcmf_usb_tx_complete(struct urb *urb)
> spin_lock_irqsave(&devinfo->tx_flowblock_lock, flags);
> if (devinfo->tx_freecount > devinfo->tx_high_watermark &&
> devinfo->tx_flowblock) {
> - brcmf_txflowblock(devinfo->dev, false);
> + brcmf_fws_bus_blocked(pub, false);
> devinfo->tx_flowblock = false;
> }
> spin_unlock_irqrestore(&devinfo->tx_flowblock_lock, flags);
> @@ -598,6 +600,7 @@ brcmf_usb_state_change(struct brcmf_usbdev_info *devinfo, int state)
> static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
> {
> struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
> + struct brcmf_pub *pub = devinfo->bus_pub.bus->drvr;
> struct brcmf_usbreq *req;
> int ret;
> unsigned long flags;
> @@ -635,7 +638,7 @@ static int brcmf_usb_tx(struct device *dev, struct sk_buff *skb)
> spin_lock_irqsave(&devinfo->tx_flowblock_lock, flags);
> if (devinfo->tx_freecount < devinfo->tx_low_watermark &&
> !devinfo->tx_flowblock) {
> - brcmf_txflowblock(dev, true);
> + brcmf_fws_bus_blocked(pub, true);
> devinfo->tx_flowblock = true;
> }
> spin_unlock_irqrestore(&devinfo->tx_flowblock_lock, flags);
>