> From: Brian Norris [mailto:[email protected]]
> 
> Hi,
> 
> >
> > +static inline void
> > +_rtw_write32s_mask(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32
> data)
> > +{
> > +   rtw_write32_mask(rtwdev, addr, mask, data);
> > +   rtw_write32_mask(rtwdev, addr + 0x200, mask, data);
> > +}
> > +
> > +/* 0xC00-0xCFF and 0xE00-0xEFF have the same layout */
> 
> Feels like this belongs with _rtw_write32s_mask() now, not here?

Yeah.

> 
> > +#define rtw_write32s_mask(rtwdev, addr, mask, data)                        
> >        \
> > +   do {                                                                   \
> > +           BUILD_BUG_ON(addr < 0xC00 || addr >= 0xD00);                   \
> 
> You probably want parentheses around the 'addr'. You *probably* won't
> run into trouble with this particular macro, but if the caller is doing
> the wrong kinds of comparisons or arithmetic, this might not work they
> way you want.

Should add parentheses to protect in case of some coding mistakes.
Thanks

> 
> Brian
> 
> > +                                                                          \
> > +           _rtw_write32s_mask(rtwdev, addr, mask, data);                  \
> > +   } while (0)
> > +
> >  /* phy status page0 */
> >  #define GET_PHY_STAT_P0_PWDB(phy_stat)
> \
> >     le32_get_bits(*((__le32 *)(phy_stat) + 0x00), GENMASK(15, 8))
> > --

Yan-Hsuan

Reply via email to