drivers/net/wireless/realtek/rtw89/phy.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 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. 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.dirtyPing-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.