This fixes commit 0107635e15ac
("staging: qlge: replace pr_err with netdev_err") which introduced an
build breakage of missing `struct ql_adapter *qdev` for some functions
and a warning of type mismatch with dumping enabled, i.e.,
$ make CFLAGS_MODULE="QL_ALL_DUMP=1 QL_OB_DUMP=1 QL_CB_DUMP=1 \
QL_IB_DUMP=1 QL_REG_DUMP=1 QL_DEV_DUMP=1" M=drivers/staging/qlge
qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
2051 | netdev_err(qdev->ndev, "%s\n", __func__);
| ^~~~
qlge_dbg.c: In function ‘ql_dump_routing_entries’:
qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Wformat=]
1435 | "%s: Routing Mask %d = 0x%.08x\n",
| ~^
| |
| char *
| %d
1436 | i, value);
| ~
| |
| int
qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned int’ argument [-Wformat=]
1435 | "%s: Routing Mask %d = 0x%.08x\n",
| ~~~~^
| |
| unsigned int
Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
Reported-by: Benjamin Poirier <[email protected]>
Suggested-by: Benjamin Poirier <[email protected]>
Signed-off-by: Coiby Xu <[email protected]>
---
drivers/staging/qlge/qlge.h | 20 ++++++++++----------
drivers/staging/qlge/qlge_dbg.c | 24 ++++++++++++++++++------
drivers/staging/qlge/qlge_main.c | 8 ++++----
3 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/qlge/qlge.h b/drivers/staging/qlge/qlge.h
index 483ce04789ed..7f6798b223ef 100644
--- a/drivers/staging/qlge/qlge.h
+++ b/drivers/staging/qlge/qlge.h
@@ -2338,21 +2338,21 @@ void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id);
#endif
#ifdef QL_OB_DUMP
-void ql_dump_tx_desc(struct tx_buf_desc *tbd);
-void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb);
-void ql_dump_ob_mac_rsp(struct ob_mac_iocb_rsp *ob_mac_rsp);
-#define QL_DUMP_OB_MAC_IOCB(ob_mac_iocb) ql_dump_ob_mac_iocb(ob_mac_iocb)
-#define QL_DUMP_OB_MAC_RSP(ob_mac_rsp) ql_dump_ob_mac_rsp(ob_mac_rsp)
+void ql_dump_tx_desc(struct ql_adapter *qdev, struct tx_buf_desc *tbd);
+void ql_dump_ob_mac_iocb(struct ql_adapter *qdev, struct ob_mac_iocb_req *ob_mac_iocb);
+void ql_dump_ob_mac_rsp(struct ql_adapter *qdev, struct ob_mac_iocb_rsp *ob_mac_rsp);
+#define QL_DUMP_OB_MAC_IOCB(qdev, ob_mac_iocb) ql_dump_ob_mac_iocb(qdev, ob_mac_iocb)
+#define QL_DUMP_OB_MAC_RSP(qdev, ob_mac_rsp) ql_dump_ob_mac_rsp(qdev, ob_mac_rsp)
#else
-#define QL_DUMP_OB_MAC_IOCB(ob_mac_iocb)
-#define QL_DUMP_OB_MAC_RSP(ob_mac_rsp)
+#define QL_DUMP_OB_MAC_IOCB(qdev, ob_mac_iocb)
+#define QL_DUMP_OB_MAC_RSP(qdev, ob_mac_rsp)
#endif
#ifdef QL_IB_DUMP
-void ql_dump_ib_mac_rsp(struct ib_mac_iocb_rsp *ib_mac_rsp);
-#define QL_DUMP_IB_MAC_RSP(ib_mac_rsp) ql_dump_ib_mac_rsp(ib_mac_rsp)
+void ql_dump_ib_mac_rsp(struct ql_adapter *qdev, struct ib_mac_iocb_rsp *ib_mac_rsp);
+#define QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp) ql_dump_ib_mac_rsp(qdev, ib_mac_rsp)
#else
-#define QL_DUMP_IB_MAC_RSP(ib_mac_rsp)
+#define QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp)
#endif
#ifdef QL_ALL_DUMP
diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index a55bf0b3e9dc..e62ee4e2f118 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1431,7 +1431,7 @@ void ql_dump_routing_entries(struct ql_adapter *qdev)
}
if (value)
netdev_err(qdev->ndev,
- "%s: Routing Mask %d = 0x%.08x\n",
+ "Routing Mask %d = 0x%.08x\n",
i, value);
}
ql_sem_unlock(qdev, SEM_RT_IDX_MASK);
@@ -1617,6 +1617,9 @@ void ql_dump_qdev(struct ql_adapter *qdev)
#ifdef QL_CB_DUMP
void ql_dump_wqicb(struct wqicb *wqicb)
{
+ struct tx_ring *tx_ring = container_of(wqicb, struct tx_ring, wqicb);
+ struct ql_adapter *qdev = tx_ring->qdev;
+
netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
netdev_err(qdev->ndev, "wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
netdev_err(qdev->ndev, "wqicb->flags = %x\n",
@@ -1632,6 +1635,8 @@ void ql_dump_wqicb(struct wqicb *wqicb)
void ql_dump_tx_ring(struct tx_ring *tx_ring)
{
+ struct ql_adapter *qdev = tx_ring->qdev;
+
if (!tx_ring)
return;
netdev_err(qdev->ndev, "===================== Dumping tx_ring %d ===============\n",
@@ -1657,6 +1662,8 @@ void ql_dump_tx_ring(struct tx_ring *tx_ring)
void ql_dump_ricb(struct ricb *ricb)
{
int i;
+ struct ql_adapter *qdev =
+ container_of(ricb, struct ql_adapter, ricb);
netdev_err(qdev->ndev, "===================== Dumping ricb ===============\n");
netdev_err(qdev->ndev, "Dumping ricb stuff...\n");
@@ -1686,6 +1693,9 @@ void ql_dump_ricb(struct ricb *ricb)
void ql_dump_cqicb(struct cqicb *cqicb)
{
+ struct rx_ring *rx_ring = container_of(cqicb, struct rx_ring, cqicb);
+ struct ql_adapter *qdev = rx_ring->qdev;
+
netdev_err(qdev->ndev, "Dumping cqicb stuff...\n");
netdev_err(qdev->ndev, "cqicb->msix_vect = %d\n", cqicb->msix_vect);
@@ -1725,6 +1735,8 @@ static const char *qlge_rx_ring_type_name(struct rx_ring *rx_ring)
void ql_dump_rx_ring(struct rx_ring *rx_ring)
{
+ struct ql_adapter *qdev = rx_ring->qdev;
+
if (!rx_ring)
return;
netdev_err(qdev->ndev,
@@ -1816,7 +1828,7 @@ void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id)
#endif
#ifdef QL_OB_DUMP
-void ql_dump_tx_desc(struct tx_buf_desc *tbd)
+void ql_dump_tx_desc(struct ql_adapter *qdev, struct tx_buf_desc *tbd)
{
netdev_err(qdev->ndev, "tbd->addr = 0x%llx\n",
le64_to_cpu((u64)tbd->addr));
@@ -1843,7 +1855,7 @@ void ql_dump_tx_desc(struct tx_buf_desc *tbd)
tbd->len & TX_DESC_E ? "E" : ".");
}
-void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb)
+void ql_dump_ob_mac_iocb(struct ql_adapter *qdev, struct ob_mac_iocb_req *ob_mac_iocb)
{
struct ob_mac_tso_iocb_req *ob_mac_tso_iocb =
(struct ob_mac_tso_iocb_req *)ob_mac_iocb;
@@ -1886,10 +1898,10 @@ void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb)
frame_len = le16_to_cpu(ob_mac_iocb->frame_len);
}
tbd = &ob_mac_iocb->tbd[0];
- ql_dump_tx_desc(tbd);
+ ql_dump_tx_desc(qdev, tbd);
}
-void ql_dump_ob_mac_rsp(struct ob_mac_iocb_rsp *ob_mac_rsp)
+void ql_dump_ob_mac_rsp(struct ql_adapter *qdev, struct ob_mac_iocb_rsp *ob_mac_rsp)
{
netdev_err(qdev->ndev, "%s\n", __func__);
netdev_err(qdev->ndev, "opcode = %d\n", ob_mac_rsp->opcode);
@@ -1906,7 +1918,7 @@ void ql_dump_ob_mac_rsp(struct ob_mac_iocb_rsp *ob_mac_rsp)
#endif
#ifdef QL_IB_DUMP
-void ql_dump_ib_mac_rsp(struct ib_mac_iocb_rsp *ib_mac_rsp)
+void ql_dump_ib_mac_rsp(struct ql_adapter *qdev, struct ib_mac_iocb_rsp *ib_mac_rsp)
{
netdev_err(qdev->ndev, "%s\n", __func__);
netdev_err(qdev->ndev, "opcode = 0x%x\n", ib_mac_rsp->opcode);
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 2028458bea6f..b351a7eb7a89 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -1856,7 +1856,7 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
struct net_device *ndev = qdev->ndev;
struct sk_buff *skb = NULL;
- QL_DUMP_IB_MAC_RSP(ib_mac_rsp);
+ QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp);
skb = ql_build_rx_skb(qdev, rx_ring, ib_mac_rsp);
if (unlikely(!skb)) {
@@ -1954,7 +1954,7 @@ static unsigned long ql_process_mac_rx_intr(struct ql_adapter *qdev,
((le16_to_cpu(ib_mac_rsp->vlan_id) &
IB_MAC_IOCB_RSP_VLAN_MASK)) : 0xffff;
- QL_DUMP_IB_MAC_RSP(ib_mac_rsp);
+ QL_DUMP_IB_MAC_RSP(qdev, ib_mac_rsp);
if (ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV) {
/* The data and headers are split into
@@ -2001,7 +2001,7 @@ static void ql_process_mac_tx_intr(struct ql_adapter *qdev,
struct tx_ring *tx_ring;
struct tx_ring_desc *tx_ring_desc;
- QL_DUMP_OB_MAC_RSP(mac_rsp);
+ QL_DUMP_OB_MAC_RSP(qdev, mac_rsp);
tx_ring = &qdev->tx_ring[mac_rsp->txq_idx];
tx_ring_desc = &tx_ring->q[mac_rsp->tid];
ql_unmap_send(qdev, tx_ring_desc, tx_ring_desc->map_cnt);
@@ -2593,7 +2593,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
tx_ring->tx_errors++;
return NETDEV_TX_BUSY;
}
- QL_DUMP_OB_MAC_IOCB(mac_iocb_ptr);
+ QL_DUMP_OB_MAC_IOCB(qdev, mac_iocb_ptr);
tx_ring->prod_idx++;
if (tx_ring->prod_idx == tx_ring->wq_len)
tx_ring->prod_idx = 0;
--
2.28.0
On 2020-08-27 07:27 +0800, Coiby Xu wrote:
> This fixes commit 0107635e15ac
> ("staging: qlge: replace pr_err with netdev_err") which introduced an
> build breakage of missing `struct ql_adapter *qdev` for some functions
> and a warning of type mismatch with dumping enabled, i.e.,
>
> $ make CFLAGS_MODULE="QL_ALL_DUMP=1 QL_OB_DUMP=1 QL_CB_DUMP=1 \
> QL_IB_DUMP=1 QL_REG_DUMP=1 QL_DEV_DUMP=1" M=drivers/staging/qlge
>
> qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
> qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
> 2051 | netdev_err(qdev->ndev, "%s\n", __func__);
> | ^~~~
> qlge_dbg.c: In function ‘ql_dump_routing_entries’:
> qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Wformat=]
> 1435 | "%s: Routing Mask %d = 0x%.08x\n",
> | ~^
> | |
> | char *
> | %d
> 1436 | i, value);
> | ~
> | |
> | int
> qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned int’ argument [-Wformat=]
> 1435 | "%s: Routing Mask %d = 0x%.08x\n",
> | ~~~~^
> | |
> | unsigned int
>
> Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
> Reported-by: Benjamin Poirier <[email protected]>
> Suggested-by: Benjamin Poirier <[email protected]>
> Signed-off-by: Coiby Xu <[email protected]>
> ---
> drivers/staging/qlge/qlge.h | 20 ++++++++++----------
> drivers/staging/qlge/qlge_dbg.c | 24 ++++++++++++++++++------
> drivers/staging/qlge/qlge_main.c | 8 ++++----
> 3 files changed, 32 insertions(+), 20 deletions(-)
>
[...]
> @@ -1632,6 +1635,8 @@ void ql_dump_wqicb(struct wqicb *wqicb)
>
> void ql_dump_tx_ring(struct tx_ring *tx_ring)
> {
> + struct ql_adapter *qdev = tx_ring->qdev;
> +
> if (!tx_ring)
> return;
Given the null check for tx_ring, it seems unwise to dereference tx_ring
before the check.
Looking at ql_dump_all(), I'm not sure that the check is needed at all
though. Maybe it should be removed.
Same problem in ql_dump_rx_ring().
> netdev_err(qdev->ndev, "===================== Dumping tx_ring %d ===============\n",
> @@ -1657,6 +1662,8 @@ void ql_dump_tx_ring(struct tx_ring *tx_ring)
> void ql_dump_ricb(struct ricb *ricb)
> {
> int i;
> + struct ql_adapter *qdev =
> + container_of(ricb, struct ql_adapter, ricb);
Here, davem would point out that the variables are not declared in
"reverse xmas tree" order.
On Thu, Aug 27, 2020 at 09:50:10AM +0900, Benjamin Poirier wrote:
>On 2020-08-27 07:27 +0800, Coiby Xu wrote:
>> This fixes commit 0107635e15ac
>> ("staging: qlge: replace pr_err with netdev_err") which introduced an
>> build breakage of missing `struct ql_adapter *qdev` for some functions
>> and a warning of type mismatch with dumping enabled, i.e.,
>>
>> $ make CFLAGS_MODULE="QL_ALL_DUMP=1 QL_OB_DUMP=1 QL_CB_DUMP=1 \
>> QL_IB_DUMP=1 QL_REG_DUMP=1 QL_DEV_DUMP=1" M=drivers/staging/qlge
>>
>> qlge_dbg.c: In function ‘ql_dump_ob_mac_rsp’:
>> qlge_dbg.c:2051:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
>> 2051 | netdev_err(qdev->ndev, "%s\n", __func__);
>> | ^~~~
>> qlge_dbg.c: In function ‘ql_dump_routing_entries’:
>> qlge_dbg.c:1435:10: warning: format ‘%s’ expects argument of type ‘char *’, but argument 3 has type ‘int’ [-Wformat=]
>> 1435 | "%s: Routing Mask %d = 0x%.08x\n",
>> | ~^
>> | |
>> | char *
>> | %d
>> 1436 | i, value);
>> | ~
>> | |
>> | int
>> qlge_dbg.c:1435:37: warning: format ‘%x’ expects a matching ‘unsigned int’ argument [-Wformat=]
>> 1435 | "%s: Routing Mask %d = 0x%.08x\n",
>> | ~~~~^
>> | |
>> | unsigned int
>>
>> Fixes: 0107635e15ac ("staging: qlge: replace pr_err with netdev_err")
>> Reported-by: Benjamin Poirier <[email protected]>
>> Suggested-by: Benjamin Poirier <[email protected]>
>> Signed-off-by: Coiby Xu <[email protected]>
>> ---
>> drivers/staging/qlge/qlge.h | 20 ++++++++++----------
>> drivers/staging/qlge/qlge_dbg.c | 24 ++++++++++++++++++------
>> drivers/staging/qlge/qlge_main.c | 8 ++++----
>> 3 files changed, 32 insertions(+), 20 deletions(-)
>>
>[...]
>> @@ -1632,6 +1635,8 @@ void ql_dump_wqicb(struct wqicb *wqicb)
>>
>> void ql_dump_tx_ring(struct tx_ring *tx_ring)
>> {
>> + struct ql_adapter *qdev = tx_ring->qdev;
>> +
>> if (!tx_ring)
>> return;
>
>Given the null check for tx_ring, it seems unwise to dereference tx_ring
>before the check.
>
>Looking at ql_dump_all(), I'm not sure that the check is needed at all
>though. Maybe it should be removed.
>
>Same problem in ql_dump_rx_ring().
Thank you for the spotting this issue! I'll remove the check.
>
>> netdev_err(qdev->ndev, "===================== Dumping tx_ring %d ===============\n",
>> @@ -1657,6 +1662,8 @@ void ql_dump_tx_ring(struct tx_ring *tx_ring)
>> void ql_dump_ricb(struct ricb *ricb)
>> {
>> int i;
>> + struct ql_adapter *qdev =
>> + container_of(ricb, struct ql_adapter, ricb);
>
>Here, davem would point out that the variables are not declared in
>"reverse xmas tree" order.
I'll make davem happy then:)
--
Best regards,
Coiby