Search Linux Wireless

RE: [PATCH v4 09/13] rtw88: chip files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Brian Norris [mailto:briannorris@xxxxxxxxxxxx]
> 
> > +static inline void
> > +rtw_write32s_mask(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32 data)
> > +{
> > +	BUILD_BUG_ON(addr < 0xC00 || addr >= 0xD00);
> 
> This seems to be a non-traditional use of BUILD_BUG_ON(). Normally, I
> see this used for stuff that's guaranteed to be known at compile time --
> structure offsets, constants, etc. This is usually (always?) a constant,
> but it passes through a function parameter, so I'm not sure if that's
> really guaranteed.
> 
> Anyway...this is failing confusingly for me when I try to build:
> 
> drivers/net/wireless/realtek/rtw88/rtw8822b.c: In function
> ‘rtw_write32s_mask’:
> drivers/net/wireless/realtek/rtw88/rtw8822b.c:230:176: error: call to
> ‘__compiletime_assert_230’ declared with attribute error: BUILD_BUG_ON
> failed: addr < 0xC00 || addr >= 0xD00
>   BUILD_BUG_ON(addr < 0xC00 || addr >= 0xD00);
> 
> ^
> 
> I tried to pinpoint which call yielded this, and I think once I deleted
> enough calls to rtw_write32s_mask() it came down to this one:
> 
>         rtw_write32s_mask(rtwdev, REG_RFEINV, BIT(11) | BIT(10) | 0x3f,
> 0x0);
> 
> which doesn't really make sense, as that's a value of 0xcbc.
> 
> What I really think it comes down to is that you can't guarantee
> rtw_write32s_mask() will get inlined, and so BUILD_BUG_ON() may not know
> what to do with it.

Yeah, you're right. I think we should turn it into macro.
I will send the fix patch in the next patch set.
Thanks!

> 
> I think you should either drop the BUILD_BUG_ON(), or else turn
> rtw_write32s_mask() into a macro.
> 
> BTW, I'm using a variant of gcc version 4.9.2. I normally use clang for
> kernel builds (as that's the current Chromium OS toolchain), but clang
> still regularly has breakage upstream, so I'm still building with an old
> GCC toolchain. It's technically still supported in
> Documentation/process/changes.rst though!

Like you said, should support it anyway :)

> 
> Brian
> 

Yan-Hsuan




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux