RE: [PATCH rtw-next 2/2] wifi: rtw89: avoid to use not consecutive mask in __write_ctrl()

[Ping-Ke Shih <pkshih@realtek.com>]

Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ping-Ke,
> 
> On Mon, 17 Nov 2025 at 04:30, Ping-Ke Shih <pkshih@realtek.com> wrote:
> > The power value and enable bit fields can be not consecutive mask, but
> > normally we expect mask argument of rtw89_mac_txpwr_write32_mask() is
> > consecutive bit mask. Therefore, change the code accordingly.
> >
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/drivers/net/wireless/realtek/rtw89/mac.h
> > +++ b/drivers/net/wireless/realtek/rtw89/mac.h
> > @@ -1456,6 +1456,26 @@ static inline int rtw89_mac_txpwr_write32_mask(struct rtw89_dev *rtwdev,
> >         return 0;
> >  }
> >
> > +static inline
> > +void rtw89_mac_write_txpwr_ctrl(struct rtw89_dev *rtwdev, u32 reg, u32 mask, u32 val,
> > +                               u32 mask_en, bool cond)
> > +{
> > +       u32 wrt = u32_encode_bits(val, mask);
> 
> Nit: you could do without this variable...
> 
> > +       u32 val32;
> > +       int ret;
> > +
> > +       if (cond)
> > +               wrt |= mask_en;
> > +
> > +       ret = rtw89_mac_txpwr_read32(rtwdev, RTW89_PHY_0, reg, &val32);
> > +       if (ret)
> > +               return;
> > +
> > +       val32 &= ~(mask | mask_en);
> > +       val32 |= wrt;
> 
> val32 |= u32_encode_bits(val, mask);
> if (cond)
>         cal32 |= mask_en;

With this change, ARCH arm is failed to build (x86 is well):

In file included from /build/rtw89/core.h:9,
                 from /build/rtw89/coex.h:8,
                 from /build/rtw89/rtw8851b.c:5:
In function 'field_multiplier',
    inlined from 'field_mask' at ./include/linux/bitfield.h:170:17,
    inlined from 'u32_encode_bits' at ./include/linux/bitfield.h:200:1,
    inlined from 'rtw89_mac_write_txpwr_ctrl' at /build/rtw89/mac.h:1468:11:
./include/linux/bitfield.h:165:17: error: call to '__bad_mask' declared with attribute error: bad bitfield mask
  165 |                 __bad_mask();
      |                 ^~~~~~~~~~~~
In function 'field_multiplier',

> 
> > +       rtw89_mac_txpwr_write32(rtwdev, RTW89_PHY_0, reg, val32);
> 
> As this calls mac->get_txpwr_cr() a second time, perhaps it is better to
> open-code rtw89_mac_txpwr_read32() and rtw89_mac_txpwr_write32()
> in this function?

This function isn't in hot path, and using rtw89_mac_txpwr_read32() and
rtw89_mac_txpwr_write32() is clearer than open-code, so I'd keep it as was.

Then I don't plan to send v2.