drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.

PATCH kernel test robot 4 participants 6 messages Feb 14, 2023 – Feb 16, 2023

Share & tools

kernel test robot

author build bot

2023-02-14 13:08 UTC #0 lore raw

BCC: [email protected]
CC: [email protected]
CC: Linux Memory Management List <[email protected]>
TO: Bitterblue Smith <[email protected]>
CC: Kalle Valo <[email protected]>
CC: "Ping-Ke Shih" <[email protected]>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for RTL8188EU

:::::: branch date: 10 hours ago
:::::: commit date: 8 weeks ago
config: x86_64-randconfig-m001-20230213 (https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable

| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/

smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.

vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1661  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1662  void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1663  {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1664  	u32 *_rx_desc = (u32 *)(skb->data - sizeof(struct rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1665  	struct rtl8xxxu_rxdesc16 *rx_desc = (struct rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666  	struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667  	struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668  	u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669  	u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1670  	u64 macid_valid = ((u64)_rx_desc[5] << 32) | _rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671  	u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672  	u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673  	bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674  	u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676  	dev_dbg(dev, "%s: len: %d items: %d\n", __func__, tx_rpt_len, items);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1677  
8b9754b28f0f23 Bitterblue Smith 2022-12-17 @1678  	for (macid = 0; macid < items; macid++) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1679  		valid = false;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1680  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1681  		if (macid < 64)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1682  			valid = macid_valid & BIT(macid);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1683  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1684  		if (valid) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1685  			ra->retry[0] = le16_to_cpu(*(__le16 *)rpt);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1686  			ra->retry[1] = rpt[2];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1687  			ra->retry[2] = rpt[3];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1688  			ra->retry[3] = rpt[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1689  			ra->retry[4] = rpt[5];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1690  			ra->drop = rpt[6];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1691  			ra->total = ra->retry[0] + ra->retry[1] + ra->retry[2] +
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1692  				    ra->retry[3] + ra->retry[4] + ra->drop;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1693  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1694  			if (ra->total > 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1695  				if (ra->ra_stage < 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1696  					rtl8188e_rate_decision(ra);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1697  				else if (ra->ra_stage == 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1698  					rtl8188e_power_training_try_state(ra);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1699  				else /* ra->ra_stage == 6 */
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1700  					rtl8188e_power_training_decision(ra);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1701  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1702  				if (ra->ra_stage <= 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1703  					ra->ra_stage++;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1704  				else
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1705  					ra->ra_stage = 0;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1706  			}
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1707  		} else if (macid == 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1708  			dev_warn(dev, "%s: TX report item 0 not valid\n", __func__);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1709  		}
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1710  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1711  		dev_dbg(dev, "%s:  valid: %d retry: %d %d %d %d %d drop: %d\n",
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1712  			__func__, valid,
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1713  			ra->retry[0], ra->retry[1], ra->retry[2],
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1714  			ra->retry[3], ra->retry[4], ra->drop);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1715  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1716  		if (min_rpt_time > ra->rpt_time)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1717  			min_rpt_time = ra->rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1718  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1719  		rpt += TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1720  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1721  		/*
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1722  		 * We only use macid 0, so only the first item is relevant.
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1723  		 * AP mode will use more of them if it's ever implemented.
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1724  		 */
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1725  		break;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1726  	}
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1727  
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1728  	if (min_rpt_time != ra->pre_min_rpt_time) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1729  		rtl8xxxu_write16(priv, REG_TX_REPORT_TIME, min_rpt_time);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1730  		ra->pre_min_rpt_time = min_rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1731  	}
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1732  }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1733

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Ping-Ke Shih

2023-02-15 00:52 UTC #1 lore raw

Quoted 49 lines

-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, February 14, 2023 9:25 PM
To: [email protected]; Bitterblue Smith <[email protected]>
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>; Ping-Ke Shih <[email protected]>
Subject: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for
RTL8188EU
config: x86_64-randconfig-m001-20230213
(https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
ignoring unreachable code.
vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1662 void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1663 {
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1664 u32 *_rx_desc = (u32 *)(skb->data - sizeof(struct
rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1665 struct rtl8xxxu_rxdesc16 *rx_desc = (struct
rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666        struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667        struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668        u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669        u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1670        u64 macid_valid = ((u64)_rx_desc[5] << 32) |
_rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671        u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672        u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673        bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674        u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676        dev_dbg(dev, "%s: len: %d items: %d\n", __func__,

Quoted 1 line

tx_rpt_len, items);

I think an alternative way is to add a statement to replace the comment of the end of loop. Like

if (!priv->vif ||

    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;

if (priv->vif &&

    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;

Then, we can remove 'break;' of LN1725.
When this driver is going to support AP mode, this logic is still expected.

How about you, Bitterblue?

Ping-Ke

Quoted 90 lines

8b9754b28f0f23 Bitterblue Smith 2022-12-17 1677

8b9754b28f0f23 Bitterblue Smith 2022-12-17 @1678        for (macid = 0; macid < items; macid++) {
                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This loop doesn't loop. I see the comment at the end which explains why we have the dead code. I only review these checker warnings two times and this is my second review so, personally, I don't care if you leave it or not. But probably other people will review this static checker warning as well so you might want to just delete the loop.

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1679                valid = false;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1680
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1681                if (macid < 64)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1682                        valid = macid_valid & BIT(macid);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1683
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1684                if (valid) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1685                        ra->retry[0] =
le16_to_cpu(*(__le16 *)rpt);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1686                        ra->retry[1] = rpt[2];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1687                        ra->retry[2] = rpt[3];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1688                        ra->retry[3] = rpt[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1689                        ra->retry[4] = rpt[5];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1690                        ra->drop = rpt[6];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1691                        ra->total = ra->retry[0] +
ra->retry[1] + ra->retry[2] +
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1692                                    ra->retry[3] +
ra->retry[4] + ra->drop;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1693
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1694                        if (ra->total > 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1695                                if (ra->ra_stage < 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1696

rtl8188e_rate_decision(ra);
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1697 else if (ra->ra_stage ==
5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1698
rtl8188e_power_training_try_state(ra);
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1699 else /* ra->ra_stage ==
6 */
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1700
rtl8188e_power_training_decision(ra);

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1701
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1702                                if (ra->ra_stage <= 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1703                                        ra->ra_stage++;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1704                                else
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1705                                        ra->ra_stage =
0;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1706                        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1707                } else if (macid == 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1708                        dev_warn(dev, "%s: TX report item
0 not valid\n", __func__);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1709                }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1710
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1711                dev_dbg(dev, "%s:  valid: %d retry: %d %d
%d %d %d drop: %d\n",
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1712                        __func__, valid,
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1713                        ra->retry[0], ra->retry[1],
ra->retry[2],
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1714                        ra->retry[3], ra->retry[4],
ra->drop);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1715
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1716                if (min_rpt_time > ra->rpt_time)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1717                        min_rpt_time = ra->rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1718
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1719                rpt += TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1720
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1721                /*
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1722                 * We only use macid 0, so only the first

item is relevant.
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1723 * AP mode will use more of them if it's
ever implemented.
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1724 */

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1725                break;
                                                                ^^^^^

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1726        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1727
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1728        if (min_rpt_time != ra->pre_min_rpt_time) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1729                rtl8xxxu_write16(priv,
REG_TX_REPORT_TIME, min_rpt_time);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1730                ra->pre_min_rpt_time = min_rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1731        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1732  }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

------Please consider the environment before printing this e-mail.

Dan Carpenter

2023-02-15 04:12 UTC #2 lore raw

Quoted 60 lines · Ping-Ke Shih

Quoted 49 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, February 14, 2023 9:25 PM
To: [email protected]; Bitterblue Smith <[email protected]>
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>; Ping-Ke Shih <[email protected]>
Subject: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for
RTL8188EU
config: x86_64-randconfig-m001-20230213
(https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
ignoring unreachable code.
vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1662 void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1663 {
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1664 u32 *_rx_desc = (u32 *)(skb->data - sizeof(struct
rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1665 struct rtl8xxxu_rxdesc16 *rx_desc = (struct
rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666        struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667        struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668        u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669        u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1670        u64 macid_valid = ((u64)_rx_desc[5] << 32) |
_rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671        u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672        u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673        bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674        u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676        dev_dbg(dev, "%s: len: %d items: %d\n", __func__,
Quoted 1 line
tx_rpt_len, items);
I think an alternative way is to add a statement to replace the comment of the end of loop. Like
if (!priv->vif ||
    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;
or

Quoted 9 lines

if (priv->vif &&
    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;
Then, we can remove 'break;' of LN1725.
When this driver is going to support AP mode, this logic is still expected.
How about you, Bitterblue?

That seems like a good option to me.

regards,
dan carpenter

Bitterblue Smith

2023-02-15 11:30 UTC #3 lore raw

In reply to Ping-Ke Shih

Quoted 64 lines · Ping-Ke Shih

Quoted 49 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, February 14, 2023 9:25 PM
To: [email protected]; Bitterblue Smith <[email protected]>
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>; Ping-Ke Shih <[email protected]>
Subject: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for
RTL8188EU
config: x86_64-randconfig-m001-20230213
(https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
ignoring unreachable code.
vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1662 void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1663 {
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1664 u32 *_rx_desc = (u32 *)(skb->data - sizeof(struct
rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1665 struct rtl8xxxu_rxdesc16 *rx_desc = (struct
rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666        struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667        struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668        u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669        u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1670        u64 macid_valid = ((u64)_rx_desc[5] << 32) |
_rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671        u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672        u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673        bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674        u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676        dev_dbg(dev, "%s: len: %d items: %d\n", __func__,
Quoted 1 line
tx_rpt_len, items);
I think an alternative way is to add a statement to replace the comment of the end of loop. Like
if (!priv->vif ||
    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;
or
if (priv->vif &&
    priv->vif->type == NL80211_IFTYPE_STATION)
	items = 1;

Quoted 6 lines

Then, we can remove 'break;' of LN1725.
When this driver is going to support AP mode, this logic is still expected.
How about you, Bitterblue?
Ping-Ke

That sounds good to me.

Quoted 91 lines

8b9754b28f0f23 Bitterblue Smith 2022-12-17 1677

8b9754b28f0f23 Bitterblue Smith 2022-12-17 @1678        for (macid = 0; macid < items; macid++) {
                                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1679                valid = false;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1680
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1681                if (macid < 64)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1682                        valid = macid_valid & BIT(macid);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1683
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1684                if (valid) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1685                        ra->retry[0] =
le16_to_cpu(*(__le16 *)rpt);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1686                        ra->retry[1] = rpt[2];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1687                        ra->retry[2] = rpt[3];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1688                        ra->retry[3] = rpt[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1689                        ra->retry[4] = rpt[5];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1690                        ra->drop = rpt[6];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1691                        ra->total = ra->retry[0] +
ra->retry[1] + ra->retry[2] +
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1692                                    ra->retry[3] +
ra->retry[4] + ra->drop;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1693
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1694                        if (ra->total > 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1695                                if (ra->ra_stage < 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1696

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1701
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1702                                if (ra->ra_stage <= 5)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1703                                        ra->ra_stage++;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1704                                else
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1705                                        ra->ra_stage =
0;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1706                        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1707                } else if (macid == 0) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1708                        dev_warn(dev, "%s: TX report item
0 not valid\n", __func__);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1709                }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1710
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1711                dev_dbg(dev, "%s:  valid: %d retry: %d %d
%d %d %d drop: %d\n",
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1712                        __func__, valid,
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1713                        ra->retry[0], ra->retry[1],
ra->retry[2],
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1714                        ra->retry[3], ra->retry[4],
ra->drop);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1715
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1716                if (min_rpt_time > ra->rpt_time)
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1717                        min_rpt_time = ra->rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1718
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1719                rpt += TX_RPT2_ITEM_SIZE;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1720
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1721                /*
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1722                 * We only use macid 0, so only the first

item is relevant.
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1723 * AP mode will use more of them if it's
ever implemented.
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1724 */

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1725                break;
                                                                ^^^^^

8b9754b28f0f23 Bitterblue Smith 2022-12-17  1726        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1727
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1728        if (min_rpt_time != ra->pre_min_rpt_time) {
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1729                rtl8xxxu_write16(priv,
REG_TX_REPORT_TIME, min_rpt_time);
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1730                ra->pre_min_rpt_time = min_rpt_time;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1731        }
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1732  }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

------Please consider the environment before printing this e-mail.

Ping-Ke Shih

2023-02-16 00:53 UTC #4 lore raw

Quoted 82 lines

-----Original Message-----
From: Bitterblue Smith <[email protected]>
Sent: Wednesday, February 15, 2023 7:30 PM
To: Ping-Ke Shih <[email protected]>; Dan Carpenter <[email protected]>; [email protected]
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>
Subject: Re: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
External mail.
Quoted 28 lines · Ping-Ke Shih
Quoted 28 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, February 14, 2023 9:25 PM
To: [email protected]; Bitterblue Smith <[email protected]>
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>; Ping-Ke Shih <[email protected]>
Subject: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for
RTL8188EU
config: x86_64-randconfig-m001-20230213
(https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
ignoring unreachable code.
Quoted 6 lines
Quoted 6 lines
vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1662 void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1663 {
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1664 u32 *_rx_desc = (u32 *)(skb->data -
sizeof(struct
Quoted 7 lines
Quoted 7 lines
rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1665 struct rtl8xxxu_rxdesc16 *rx_desc = (struct
rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666        struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667        struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668        u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669        u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
Quoted 8 lines
Quoted 8 lines
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1670 u64 macid_valid = ((u64)_rx_desc[5] << 32) |
_rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671        u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672        u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673        bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674        u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676        dev_dbg(dev, "%s: len: %d items: %d\n",
__func__,
Quoted 15 lines
Quoted 1 line
tx_rpt_len, items);
I think an alternative way is to add a statement to replace the comment of the end of loop. Like
if (!priv->vif ||
    priv->vif->type == NL80211_IFTYPE_STATION)
      items = 1;
or
if (priv->vif &&
    priv->vif->type == NL80211_IFTYPE_STATION)
      items = 1;

Quoted 9 lines

Quoted 6 lines
Then, we can remove 'break;' of LN1725.
When this driver is going to support AP mode, this logic is still expected.
How about you, Bitterblue?
Ping-Ke
That sounds good to me.

I have sent a patch for this report:
https://lore.kernel.org/linux-wireless/[email protected]/T/#u

Could you give it a try, Bitterblue?
Then, post a Tested-by tag there.

Thank you
Ping-Ke

Bitterblue Smith

2023-02-16 14:11 UTC #5 lore raw

Quoted 92 lines · Ping-Ke Shih

Quoted 82 lines
-----Original Message-----
From: Bitterblue Smith <[email protected]>
Sent: Wednesday, February 15, 2023 7:30 PM
To: Ping-Ke Shih <[email protected]>; Dan Carpenter <[email protected]>; [email protected]
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>
Subject: Re: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
External mail.
Quoted 28 lines · Ping-Ke Shih
Quoted 28 lines
-----Original Message-----
From: Dan Carpenter <[email protected]>
Sent: Tuesday, February 14, 2023 9:25 PM
To: [email protected]; Bitterblue Smith <[email protected]>
Cc: [email protected]; [email protected]; Kalle Valo <[email protected]>; Ping-Ke Shih <[email protected]>
Subject: [linux-next:master 681/12724] drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn: ignoring unreachable code.
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   3ebb0ac55efaf1d0fb1b106f852c114e5021f7eb
commit: 8b9754b28f0f231736667e1751f1fc6f18c80ad4 [681/12724] wifi: rtl8xxxu: Add rate control code for
RTL8188EU
config: x86_64-randconfig-m001-20230213
(https://download.01.org/0day-ci/archive/20230214/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c:1678 rtl8188e_handle_ra_tx_report2() warn:
ignoring unreachable code.
Quoted 6 lines
Quoted 6 lines
vim +1678 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1662 void rtl8188e_handle_ra_tx_report2(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1663 {
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1664 u32 *_rx_desc = (u32 *)(skb->data -
sizeof(struct
Quoted 7 lines
Quoted 7 lines
rtl8xxxu_rxdesc16));
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1665 struct rtl8xxxu_rxdesc16 *rx_desc = (struct
rtl8xxxu_rxdesc16 *)_rx_desc;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1666        struct device *dev = &priv->udev->dev;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1667        struct rtl8xxxu_ra_info *ra = &priv->ra_info;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1668        u32 tx_rpt_len = rx_desc->pktlen & 0x3ff;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1669        u32 items = tx_rpt_len / TX_RPT2_ITEM_SIZE;
Quoted 8 lines
Quoted 8 lines
8b9754b28f0f23 Bitterblue Smith 2022-12-17 1670 u64 macid_valid = ((u64)_rx_desc[5] << 32) |
_rx_desc[4];
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1671        u32 macid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1672        u8 *rpt = skb->data;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1673        bool valid;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1674        u16 min_rpt_time = 0x927c;
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1675
8b9754b28f0f23 Bitterblue Smith 2022-12-17  1676        dev_dbg(dev, "%s: len: %d items: %d\n",
__func__,
Quoted 15 lines
Quoted 1 line
tx_rpt_len, items);
I think an alternative way is to add a statement to replace the comment of the end of loop. Like
if (!priv->vif ||
    priv->vif->type == NL80211_IFTYPE_STATION)
      items = 1;
or
if (priv->vif &&
    priv->vif->type == NL80211_IFTYPE_STATION)
      items = 1;
Quoted 9 lines
Quoted 6 lines
Then, we can remove 'break;' of LN1725.
When this driver is going to support AP mode, this logic is still expected.
How about you, Bitterblue?
Ping-Ke
That sounds good to me.

Quoted 9 lines

I have sent a patch for this report:
https://lore.kernel.org/linux-wireless/[email protected]/T/#u
Could you give it a try, Bitterblue?
Then, post a Tested-by tag there.
Thank you
Ping-Ke

Thank you for the patch. It works.