drivers/net/wireless/realtek/rtw89/phy.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
There are more unsupported functions than just LOWRT_RTY. Improve on
commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid
warning message") by printing a message just once when we first
encounter an unsupported class. This prevents messages like
rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support
from filling up dmesg.
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
drivers/net/wireless/realtek/rtw89/phy.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index f4eee642e5ce..5280ffd77b39 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -3535,6 +3535,7 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb,
{
void (*handler)(struct rtw89_dev *rtwdev,
struct sk_buff *c2h, u32 len) = NULL;
+ static DECLARE_BITMAP(printed_support, U8_MAX);
switch (class) {
case RTW89_PHY_C2H_CLASS_RA:
@@ -3549,17 +3550,15 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb,
if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler))
handler = rtw89_phy_c2h_rfk_report_handler[func];
break;
- case RTW89_PHY_C2H_CLASS_DM:
- if (func == RTW89_PHY_C2H_DM_FUNC_LOWRT_RTY)
- return;
- fallthrough;
default:
- rtw89_info(rtwdev, "PHY c2h class %d not support\n", class);
+ if (!test_and_set_bit(class, printed_support))
+ rtw89_info(rtwdev, "PHY c2h class %d not supported\n",
+ class);
return;
}
if (!handler) {
- rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class,
- func);
+ rtw89_info(rtwdev, "PHY c2h class %d func %d not supported\n",
+ class, func);
return;
}
handler(rtwdev, skb, len);
--
2.35.1.1320.gc452695387.dirty
On 29/07/2025 21:27, Sean Anderson wrote: > There are more unsupported functions than just LOWRT_RTY. Improve on > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > warning message") by printing a message just once when we first > encounter an unsupported class. This prevents messages like > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > from filling up dmesg. > I also get "MAC c2h class 1 func 3 not support" and "MAC c2h class 0 func 6 not support" with RTL8832CU. The second one appears a lot in AP mode. > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- > > drivers/net/wireless/realtek/rtw89/phy.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c > index f4eee642e5ce..5280ffd77b39 100644 > --- a/drivers/net/wireless/realtek/rtw89/phy.c > +++ b/drivers/net/wireless/realtek/rtw89/phy.c > @@ -3535,6 +3535,7 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > { > void (*handler)(struct rtw89_dev *rtwdev, > struct sk_buff *c2h, u32 len) = NULL; > + static DECLARE_BITMAP(printed_support, U8_MAX); > > switch (class) { > case RTW89_PHY_C2H_CLASS_RA: > @@ -3549,17 +3550,15 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler)) > handler = rtw89_phy_c2h_rfk_report_handler[func]; > break; > - case RTW89_PHY_C2H_CLASS_DM: > - if (func == RTW89_PHY_C2H_DM_FUNC_LOWRT_RTY) > - return; > - fallthrough; > default: > - rtw89_info(rtwdev, "PHY c2h class %d not support\n", class); > + if (!test_and_set_bit(class, printed_support)) > + rtw89_info(rtwdev, "PHY c2h class %d not supported\n", > + class); > return; > } > if (!handler) { > - rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class, > - func); > + rtw89_info(rtwdev, "PHY c2h class %d func %d not supported\n", > + class, func); > return; > } > handler(rtwdev, skb, len);
Ping-Ke Shih <pkshih@realtek.com> wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > > On 29/07/2025 21:27, Sean Anderson wrote: > > > There are more unsupported functions than just LOWRT_RTY. Improve on > > > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > > > warning message") by printing a message just once when we first > > > encounter an unsupported class. This prevents messages like > > > > > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > > > > > from filling up dmesg. > > > > > > > I also get "MAC c2h class 1 func 3 not support" and "MAC c2h class > > 0 func 6 not support" with RTL8832CU. > > These two C2H events were defined as: > > // CLASS 1 - FW_OFLD > #define FWCMD_C2H_FUNC_BEACON_RESEND 0x3 > > // CLASS 0 - FW_INFO > #define FWCMD_C2H_FUNC_BCN_UPD_DONE 0x06 > > The implementation of handlers in vendor driver looks like does nothing > needed to be rewritten. Just add a dummy to represent that we have reviewed > the C2H events. > I added two dummy functions for these to C2H events by [1]. Please help to test if this can resolve the messages in your side. [1] https://lore.kernel.org/linux-wireless/20250804012234.8913-1-pkshih@realtek.com/T/#t
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > On 29/07/2025 21:27, Sean Anderson wrote: > > There are more unsupported functions than just LOWRT_RTY. Improve on > > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > > warning message") by printing a message just once when we first > > encounter an unsupported class. This prevents messages like > > > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > > > from filling up dmesg. > > > > I also get "MAC c2h class 1 func 3 not support" and "MAC c2h class > 0 func 6 not support" with RTL8832CU. These two C2H events were defined as: // CLASS 1 - FW_OFLD #define FWCMD_C2H_FUNC_BEACON_RESEND 0x3 // CLASS 0 - FW_INFO #define FWCMD_C2H_FUNC_BCN_UPD_DONE 0x06 The implementation of handlers in vendor driver looks like does nothing needed to be rewritten. Just add a dummy to represent that we have reviewed the C2H events.
There are more unsupported functions than just LOWRT_RTY. Improve on
commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid
warning message") by printing a message just once when we first
encounter an unsupported class. Do the same for each unsupported func of
the supported classes. This prevents messages like
rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support
from filling up dmesg.
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---
Changes in v2:
- Also suppress unsupported func messages
drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c
index f4eee642e5ce..9484d80eea9b 100644
--- a/drivers/net/wireless/realtek/rtw89/phy.c
+++ b/drivers/net/wireless/realtek/rtw89/phy.c
@@ -3535,17 +3535,25 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb,
{
void (*handler)(struct rtw89_dev *rtwdev,
struct sk_buff *c2h, u32 len) = NULL;
+ static DECLARE_BITMAP(printed_ra, U8_MAX);
+ static DECLARE_BITMAP(printed_rfk_log, U8_MAX);
+ static DECLARE_BITMAP(printed_rfk_report, U8_MAX);
+ static DECLARE_BITMAP(printed_class, U8_MAX);
+ unsigned long *printed;
switch (class) {
case RTW89_PHY_C2H_CLASS_RA:
+ printed = printed_ra;
if (func < RTW89_PHY_C2H_FUNC_RA_MAX)
handler = rtw89_phy_c2h_ra_handler[func];
break;
case RTW89_PHY_C2H_RFK_LOG:
+ printed = printed_rfk_log;
if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_log_handler))
handler = rtw89_phy_c2h_rfk_log_handler[func];
break;
case RTW89_PHY_C2H_RFK_REPORT:
+ printed = printed_rfk_report;
if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler))
handler = rtw89_phy_c2h_rfk_report_handler[func];
break;
@@ -3554,12 +3562,16 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb,
return;
fallthrough;
default:
- rtw89_info(rtwdev, "PHY c2h class %d not support\n", class);
+ if (!test_and_set_bit(class, printed_class))
+ rtw89_info(rtwdev, "PHY c2h class %d not supported\n",
+ class);
return;
}
if (!handler) {
- rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class,
- func);
+ if (!test_and_set_bit(func, printed))
+ rtw89_info(rtwdev,
+ "PHY c2h class %d func %d not supported\n",
+ class, func);
return;
}
handler(rtwdev, skb, len);
--
2.35.1.1320.gc452695387.dirty
Ping-Ke Shih <pkshih@realtek.com> wrote: > Sean Anderson <sean.anderson@linux.dev> wrote: > > There are more unsupported functions than just LOWRT_RTY. Improve on > > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > > warning message") by printing a message just once when we first > > encounter an unsupported class. > > Once I encounter an unsupported class/func, I'll check firmware team if the > C2H events can be ignored. If so, I add a dummy function to avoid the message. > If not, I should add code to handle the event. > > Do you want to see the message even though it only appears once? > > > Do the same for each unsupported func of > > the supported classes. This prevents messages like > > > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support Is this a real example? We have handled class 2 (RTW89_PHY_C2H_CLASS_DM), no? Please point out the class / func you encountered. Then I can look up vendor driver or contact internal firmware team to know if we should implement or just add a dummy function. If we defer it, I don't know when we can do it. > > > > from filling up dmesg. > > > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > > --- > > > > Changes in v2: > > - Also suppress unsupported func messages > > > > drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c > > index f4eee642e5ce..9484d80eea9b 100644 > > --- a/drivers/net/wireless/realtek/rtw89/phy.c > > +++ b/drivers/net/wireless/realtek/rtw89/phy.c > > @@ -3535,17 +3535,25 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > > { > > void (*handler)(struct rtw89_dev *rtwdev, > > struct sk_buff *c2h, u32 len) = NULL; > > + static DECLARE_BITMAP(printed_ra, U8_MAX); > > + static DECLARE_BITMAP(printed_rfk_log, U8_MAX); > > + static DECLARE_BITMAP(printed_rfk_report, U8_MAX); > > + static DECLARE_BITMAP(printed_class, U8_MAX); > > + unsigned long *printed; > > > > switch (class) { > > case RTW89_PHY_C2H_CLASS_RA: > > + printed = printed_ra; > > if (func < RTW89_PHY_C2H_FUNC_RA_MAX) > > handler = rtw89_phy_c2h_ra_handler[func]; > > break; > > case RTW89_PHY_C2H_RFK_LOG: > > + printed = printed_rfk_log; > > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_log_handler)) > > handler = rtw89_phy_c2h_rfk_log_handler[func]; > > break; > > case RTW89_PHY_C2H_RFK_REPORT: > > + printed = printed_rfk_report; > > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler)) > > handler = rtw89_phy_c2h_rfk_report_handler[func]; > > break; > > @@ -3554,12 +3562,16 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > > return; > > fallthrough; > > default: > > - rtw89_info(rtwdev, "PHY c2h class %d not support\n", class); > > + if (!test_and_set_bit(class, printed_class)) > > + rtw89_info(rtwdev, "PHY c2h class %d not supported\n", > > + class); > > return; > > } > > if (!handler) { > > - rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class, > > - func); > > + if (!test_and_set_bit(func, printed)) > > + rtw89_info(rtwdev, > > + "PHY c2h class %d func %d not supported\n", > > + class, func); > > return; > > } > > handler(rtwdev, skb, len); > > -- > > 2.35.1.1320.gc452695387.dirty
On 7/29/25 23:42, Ping-Ke Shih wrote: > Ping-Ke Shih <pkshih@realtek.com> wrote: >> Sean Anderson <sean.anderson@linux.dev> wrote: >> > There are more unsupported functions than just LOWRT_RTY. Improve on >> > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid >> > warning message") by printing a message just once when we first >> > encounter an unsupported class. >> >> Once I encounter an unsupported class/func, I'll check firmware team if the >> C2H events can be ignored. If so, I add a dummy function to avoid the message. >> If not, I should add code to handle the event. >> >> Do you want to see the message even though it only appears once? >> >> > Do the same for each unsupported func of >> > the supported classes. This prevents messages like >> > >> > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > Is this a real example? This is a real example. > We have handled class 2 (RTW89_PHY_C2H_CLASS_DM), no? If func != RTW89_PHY_C2H_DM_FUNC_LOWRT_RTY then we fall through to the default case. > Please point out the class / func you encountered. Then I can look up vendor > driver or contact internal firmware team to know if we should implement or > just add a dummy function. > > If we defer it, I don't know when we can do it. rtw89_8922ae 0000:81:00.0: PHY c2h class 2 func 12 not supported But the func was not printed in the base kernel, just the class. --Sean >> > >> > from filling up dmesg. >> > >> > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> > --- >> > >> > Changes in v2: >> > - Also suppress unsupported func messages >> > >> > drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++--- >> > 1 file changed, 15 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c >> > index f4eee642e5ce..9484d80eea9b 100644 >> > --- a/drivers/net/wireless/realtek/rtw89/phy.c >> > +++ b/drivers/net/wireless/realtek/rtw89/phy.c >> > @@ -3535,17 +3535,25 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, >> > { >> > void (*handler)(struct rtw89_dev *rtwdev, >> > struct sk_buff *c2h, u32 len) = NULL; >> > + static DECLARE_BITMAP(printed_ra, U8_MAX); >> > + static DECLARE_BITMAP(printed_rfk_log, U8_MAX); >> > + static DECLARE_BITMAP(printed_rfk_report, U8_MAX); >> > + static DECLARE_BITMAP(printed_class, U8_MAX); >> > + unsigned long *printed; >> > >> > switch (class) { >> > case RTW89_PHY_C2H_CLASS_RA: >> > + printed = printed_ra; >> > if (func < RTW89_PHY_C2H_FUNC_RA_MAX) >> > handler = rtw89_phy_c2h_ra_handler[func]; >> > break; >> > case RTW89_PHY_C2H_RFK_LOG: >> > + printed = printed_rfk_log; >> > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_log_handler)) >> > handler = rtw89_phy_c2h_rfk_log_handler[func]; >> > break; >> > case RTW89_PHY_C2H_RFK_REPORT: >> > + printed = printed_rfk_report; >> > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler)) >> > handler = rtw89_phy_c2h_rfk_report_handler[func]; >> > break; >> > @@ -3554,12 +3562,16 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, >> > return; >> > fallthrough; >> > default: >> > - rtw89_info(rtwdev, "PHY c2h class %d not support\n", class); >> > + if (!test_and_set_bit(class, printed_class)) >> > + rtw89_info(rtwdev, "PHY c2h class %d not supported\n", >> > + class); >> > return; >> > } >> > if (!handler) { >> > - rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class, >> > - func); >> > + if (!test_and_set_bit(func, printed)) >> > + rtw89_info(rtwdev, >> > + "PHY c2h class %d func %d not supported\n", >> > + class, func); >> > return; >> > } >> > handler(rtwdev, skb, len); >> > -- >> > 2.35.1.1320.gc452695387.dirty >
Sean Anderson <sean.anderson@linux.dev> wrote: > On 7/29/25 23:42, Ping-Ke Shih wrote: > > Ping-Ke Shih <pkshih@realtek.com> wrote: > >> Sean Anderson <sean.anderson@linux.dev> wrote: > >> > There are more unsupported functions than just LOWRT_RTY. Improve on > >> > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > >> > warning message") by printing a message just once when we first > >> > encounter an unsupported class. > >> > >> Once I encounter an unsupported class/func, I'll check firmware team if the > >> C2H events can be ignored. If so, I add a dummy function to avoid the message. > >> If not, I should add code to handle the event. > >> > >> Do you want to see the message even though it only appears once? > >> > >> > Do the same for each unsupported func of > >> > the supported classes. This prevents messages like > >> > > >> > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > > > Is this a real example? > > This is a real example. > > > We have handled class 2 (RTW89_PHY_C2H_CLASS_DM), no? > > If func != RTW89_PHY_C2H_DM_FUNC_LOWRT_RTY then we fall through to the > default case. Oh. I see. > > > Please point out the class / func you encountered. Then I can look up vendor > > driver or contact internal firmware team to know if we should implement or > > just add a dummy function. > > > > If we defer it, I don't know when we can do it. > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 func 12 not supported > The C2H event handler has been added by [1]. [1] d31c42466b1a ("wifi: rtw89: phy: add C2H event handler for report of FW scan")
On 7/31/25 20:36, Ping-Ke Shih wrote: > Sean Anderson <sean.anderson@linux.dev> wrote: >> On 7/29/25 23:42, Ping-Ke Shih wrote: >> > Ping-Ke Shih <pkshih@realtek.com> wrote: >> >> Sean Anderson <sean.anderson@linux.dev> wrote: >> >> > There are more unsupported functions than just LOWRT_RTY. Improve on >> >> > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid >> >> > warning message") by printing a message just once when we first >> >> > encounter an unsupported class. >> >> >> >> Once I encounter an unsupported class/func, I'll check firmware team if the >> >> C2H events can be ignored. If so, I add a dummy function to avoid the message. >> >> If not, I should add code to handle the event. >> >> >> >> Do you want to see the message even though it only appears once? >> >> >> >> > Do the same for each unsupported func of >> >> > the supported classes. This prevents messages like >> >> > >> >> > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support >> > >> > Is this a real example? >> >> This is a real example. >> >> > We have handled class 2 (RTW89_PHY_C2H_CLASS_DM), no? >> >> If func != RTW89_PHY_C2H_DM_FUNC_LOWRT_RTY then we fall through to the >> default case. > > Oh. I see. > >> >> > Please point out the class / func you encountered. Then I can look up vendor >> > driver or contact internal firmware team to know if we should implement or >> > just add a dummy function. >> > >> > If we defer it, I don't know when we can do it. >> >> rtw89_8922ae 0000:81:00.0: PHY c2h class 2 func 12 not supported >> > > The C2H event handler has been added by [1]. > > [1] d31c42466b1a ("wifi: rtw89: phy: add C2H event handler for report of FW scan") OK, but func 12 is still not handled by that commit. --Sean
Sean Anderson <sean.anderson@linux.dev> wrote: > There are more unsupported functions than just LOWRT_RTY. Improve on > commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > warning message") by printing a message just once when we first > encounter an unsupported class. Once I encounter an unsupported class/func, I'll check firmware team if the C2H events can be ignored. If so, I add a dummy function to avoid the message. If not, I should add code to handle the event. Do you want to see the message even though it only appears once? > Do the same for each unsupported func of > the supported classes. This prevents messages like > > rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support > > from filling up dmesg. > > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- > > Changes in v2: > - Also suppress unsupported func messages > > drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c > index f4eee642e5ce..9484d80eea9b 100644 > --- a/drivers/net/wireless/realtek/rtw89/phy.c > +++ b/drivers/net/wireless/realtek/rtw89/phy.c > @@ -3535,17 +3535,25 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > { > void (*handler)(struct rtw89_dev *rtwdev, > struct sk_buff *c2h, u32 len) = NULL; > + static DECLARE_BITMAP(printed_ra, U8_MAX); > + static DECLARE_BITMAP(printed_rfk_log, U8_MAX); > + static DECLARE_BITMAP(printed_rfk_report, U8_MAX); > + static DECLARE_BITMAP(printed_class, U8_MAX); > + unsigned long *printed; > > switch (class) { > case RTW89_PHY_C2H_CLASS_RA: > + printed = printed_ra; > if (func < RTW89_PHY_C2H_FUNC_RA_MAX) > handler = rtw89_phy_c2h_ra_handler[func]; > break; > case RTW89_PHY_C2H_RFK_LOG: > + printed = printed_rfk_log; > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_log_handler)) > handler = rtw89_phy_c2h_rfk_log_handler[func]; > break; > case RTW89_PHY_C2H_RFK_REPORT: > + printed = printed_rfk_report; > if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler)) > handler = rtw89_phy_c2h_rfk_report_handler[func]; > break; > @@ -3554,12 +3562,16 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, > return; > fallthrough; > default: > - rtw89_info(rtwdev, "PHY c2h class %d not support\n", class); > + if (!test_and_set_bit(class, printed_class)) > + rtw89_info(rtwdev, "PHY c2h class %d not supported\n", > + class); > return; > } > if (!handler) { > - rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class, > - func); > + if (!test_and_set_bit(func, printed)) > + rtw89_info(rtwdev, > + "PHY c2h class %d func %d not supported\n", > + class, func); > return; > } > handler(rtwdev, skb, len); > -- > 2.35.1.1320.gc452695387.dirty
On 7/29/25 20:36, Ping-Ke Shih wrote: > Sean Anderson <sean.anderson@linux.dev> wrote: >> There are more unsupported functions than just LOWRT_RTY. Improve on >> commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid >> warning message") by printing a message just once when we first >> encounter an unsupported class. > > Once I encounter an unsupported class/func, I'll check firmware team if the > C2H events can be ignored. If so, I add a dummy function to avoid the message. > If not, I should add code to handle the event. > > Do you want to see the message even though it only appears once? I mean, maybe it should just be a debug? Are these messages useful for anyone other than the developers? Maybe we should just print only the very first unsupported message at info level and print the rest at debug. --Sean >> Do the same for each unsupported func of >> the supported classes. This prevents messages like >> >> rtw89_8922ae 0000:81:00.0: PHY c2h class 2 not support >> >> from filling up dmesg. >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> Changes in v2: >> - Also suppress unsupported func messages >> >> drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtw89/phy.c b/drivers/net/wireless/realtek/rtw89/phy.c >> index f4eee642e5ce..9484d80eea9b 100644 >> --- a/drivers/net/wireless/realtek/rtw89/phy.c >> +++ b/drivers/net/wireless/realtek/rtw89/phy.c >> @@ -3535,17 +3535,25 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, >> { >> void (*handler)(struct rtw89_dev *rtwdev, >> struct sk_buff *c2h, u32 len) = NULL; >> + static DECLARE_BITMAP(printed_ra, U8_MAX); >> + static DECLARE_BITMAP(printed_rfk_log, U8_MAX); >> + static DECLARE_BITMAP(printed_rfk_report, U8_MAX); >> + static DECLARE_BITMAP(printed_class, U8_MAX); >> + unsigned long *printed; >> >> switch (class) { >> case RTW89_PHY_C2H_CLASS_RA: >> + printed = printed_ra; >> if (func < RTW89_PHY_C2H_FUNC_RA_MAX) >> handler = rtw89_phy_c2h_ra_handler[func]; >> break; >> case RTW89_PHY_C2H_RFK_LOG: >> + printed = printed_rfk_log; >> if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_log_handler)) >> handler = rtw89_phy_c2h_rfk_log_handler[func]; >> break; >> case RTW89_PHY_C2H_RFK_REPORT: >> + printed = printed_rfk_report; >> if (func < ARRAY_SIZE(rtw89_phy_c2h_rfk_report_handler)) >> handler = rtw89_phy_c2h_rfk_report_handler[func]; >> break; >> @@ -3554,12 +3562,16 @@ void rtw89_phy_c2h_handle(struct rtw89_dev *rtwdev, struct sk_buff *skb, >> return; >> fallthrough; >> default: >> - rtw89_info(rtwdev, "PHY c2h class %d not support\n", class); >> + if (!test_and_set_bit(class, printed_class)) >> + rtw89_info(rtwdev, "PHY c2h class %d not supported\n", >> + class); >> return; >> } >> if (!handler) { >> - rtw89_info(rtwdev, "PHY c2h class %d func %d not support\n", class, >> - func); >> + if (!test_and_set_bit(func, printed)) >> + rtw89_info(rtwdev, >> + "PHY c2h class %d func %d not supported\n", >> + class, func); >> return; >> } >> handler(rtwdev, skb, len); >> -- >> 2.35.1.1320.gc452695387.dirty >
Sean Anderson <sean.anderson@linux.dev> wrote: > On 7/29/25 20:36, Ping-Ke Shih wrote: > > Sean Anderson <sean.anderson@linux.dev> wrote: > >> There are more unsupported functions than just LOWRT_RTY. Improve on > >> commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > >> warning message") by printing a message just once when we first > >> encounter an unsupported class. > > > > Once I encounter an unsupported class/func, I'll check firmware team if the > > C2H events can be ignored. If so, I add a dummy function to avoid the message. > > If not, I should add code to handle the event. > > > > Do you want to see the message even though it only appears once? > > I mean, maybe it should just be a debug? Are these messages useful for anyone > other than the developers? Yes, this could just be a debug. However, developers normally don't turn on debug mask, so using rtw89_info is to clearly remind developers to pay attention on this lack of C2H handler. And, I suppose developers must handle this when they see flooding messages. > > Maybe we should just print only the very first unsupported message at info level > and print the rest at debug. I'm afraid developers will ignore or miss the messages. To reduce messages is fine to me , but more important is to look up vendor driver to see if the C2H handler is necessary.
On 7/31/25 20:30, Ping-Ke Shih wrote: > Sean Anderson <sean.anderson@linux.dev> wrote: >> On 7/29/25 20:36, Ping-Ke Shih wrote: >> > Sean Anderson <sean.anderson@linux.dev> wrote: >> >> There are more unsupported functions than just LOWRT_RTY. Improve on >> >> commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid >> >> warning message") by printing a message just once when we first >> >> encounter an unsupported class. >> > >> > Once I encounter an unsupported class/func, I'll check firmware team if the >> > C2H events can be ignored. If so, I add a dummy function to avoid the message. >> > If not, I should add code to handle the event. >> > >> > Do you want to see the message even though it only appears once? >> >> I mean, maybe it should just be a debug? Are these messages useful for anyone >> other than the developers? > > Yes, this could just be a debug. However, developers normally don't turn on > debug mask, so using rtw89_info is to clearly remind developers to pay > attention on this lack of C2H handler. And, I suppose developers must handle > this when they see flooding messages. Well, regular users get this too. It is really unnecessary to print thousands of messages when they are completely benign. >> >> Maybe we should just print only the very first unsupported message at info level >> and print the rest at debug. > > I'm afraid developers will ignore or miss the messages. To reduce messages > is fine to me , but more important is to look up vendor driver to see if > the C2H handler is necessary. OK, so we should print exactly once for each class/func. --Sean
Sean Anderson <sean.anderson@linux.dev> wrote: > On 7/31/25 20:30, Ping-Ke Shih wrote: > > Sean Anderson <sean.anderson@linux.dev> wrote: > >> On 7/29/25 20:36, Ping-Ke Shih wrote: > >> > Sean Anderson <sean.anderson@linux.dev> wrote: > >> >> There are more unsupported functions than just LOWRT_RTY. Improve on > >> >> commit 3b66519b023b ("wifi: rtw89: phy: add dummy c2h handler to avoid > >> >> warning message") by printing a message just once when we first > >> >> encounter an unsupported class. > >> > > >> > Once I encounter an unsupported class/func, I'll check firmware team if the > >> > C2H events can be ignored. If so, I add a dummy function to avoid the message. > >> > If not, I should add code to handle the event. > >> > > >> > Do you want to see the message even though it only appears once? > >> > >> I mean, maybe it should just be a debug? Are these messages useful for anyone > >> other than the developers? > > > > Yes, this could just be a debug. However, developers normally don't turn on > > debug mask, so using rtw89_info is to clearly remind developers to pay > > attention on this lack of C2H handler. And, I suppose developers must handle > > this when they see flooding messages. > > Well, regular users get this too. It is really unnecessary to print > thousands of messages when they are completely benign. > > >> > >> Maybe we should just print only the very first unsupported message at info level > >> and print the rest at debug. > > > > I'm afraid developers will ignore or miss the messages. To reduce messages > > is fine to me , but more important is to look up vendor driver to see if > > the C2H handler is necessary. > > OK, so we should print exactly once for each class/func. > I sent a patchset [1] to print exactly once, but I don't add bitmap for every printed class/func, because it seems to be unnecessary to add much code to handle the case which should be handled during development. Please help to try if this patchset can resolve thousands of message in your side. [1] https://lore.kernel.org/linux-wireless/20250804012234.8913-1-pkshih@realtek.com/T/#t
© 2016 - 2025 Red Hat, Inc.