| Message ID | 20220810010809.2024482-1-nathan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion | expand |
On Tue, Aug 9, 2022 at 6:08 PM Nathan Chancellor <nathan@kernel.org> wrote: > > A recent change in clang strengthened its -Wbitfield-constant-conversion > to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can > only be 0 or -1, not 1: > > sound/soc/atmel/mchp-spdiftx.c:505:20: error: implicit truncation from 'int' to bit-field changes value from 1 to -1 [-Werror,-Wbitfield-constant-conversion] > dev->gclk_enabled = 1; > ^ ~ > 1 error generated. > > The actual value of the field is never checked, just that it is not > zero, so there is not a real bug here. However, it is simple enough to > silence the warning by making the bitfield unsigned, which matches the > mchp-spdifrx driver. > > Fixes: 06ca24e98e6b ("ASoC: mchp-spdiftx: add driver for S/PDIF TX Controller") > Link: https://github.com/ClangBuiltLinux/linux/issues/1686 > Link: https://github.com/llvm/llvm-project/commit/82afc9b169a67e8b8a1862fb9c41a2cd974d6691 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Ah yes, my favorite, signed one bit integers...thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> grepping for `gclk_enabled`, we see three drivers with similar duplication (single bit bitfields): sound/soc/atmel/mchp-spdifrx.c 241: unsigned int gclk_enabled:1; sound/soc/atmel/mchp-pdmc.c 118: u8 gclk_enabled:1; sound/soc/atmel/mchp-spdiftx.c 200: int gclk_enabled:1; > --- > sound/soc/atmel/mchp-spdiftx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c > index 4850a177803d..ab2d7a791f39 100644 > --- a/sound/soc/atmel/mchp-spdiftx.c > +++ b/sound/soc/atmel/mchp-spdiftx.c > @@ -196,7 +196,7 @@ struct mchp_spdiftx_dev { > struct clk *pclk; > struct clk *gclk; > unsigned int fmt; > - int gclk_enabled:1; > + unsigned int gclk_enabled:1; > }; > > static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev) > > base-commit: 15205c2829ca2cbb5ece5ceaafe1171a8470e62b > -- > 2.37.1 >
From: Nick Desaulniers > Sent: 10 August 2022 22:14 > > On Tue, Aug 9, 2022 at 6:08 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > A recent change in clang strengthened its -Wbitfield-constant-conversion > > to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can > > only be 0 or -1, not 1: Is there a -Wsigned-bitfield ? You probably don't ever want the compiler to be generating the code to sign-propagate a bitfield. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, 9 Aug 2022 18:08:09 -0700, Nathan Chancellor wrote: > A recent change in clang strengthened its -Wbitfield-constant-conversion > to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can > only be 0 or -1, not 1: > > sound/soc/atmel/mchp-spdiftx.c:505:20: error: implicit truncation from 'int' to bit-field changes value from 1 to -1 [-Werror,-Wbitfield-constant-conversion] > dev->gclk_enabled = 1; > ^ ~ > 1 error generated. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion commit: eab9100d9898cbd37882b04415b12156f8942f18 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Hi Mark, On Mon, Aug 15, 2022 at 05:23:15PM +0100, Mark Brown wrote: > On Tue, 9 Aug 2022 18:08:09 -0700, Nathan Chancellor wrote: > > A recent change in clang strengthened its -Wbitfield-constant-conversion > > to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can > > only be 0 or -1, not 1: > > > > sound/soc/atmel/mchp-spdiftx.c:505:20: error: implicit truncation from 'int' to bit-field changes value from 1 to -1 [-Werror,-Wbitfield-constant-conversion] > > dev->gclk_enabled = 1; > > ^ ~ > > 1 error generated. > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next > > Thanks! > > [1/1] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion > commit: eab9100d9898cbd37882b04415b12156f8942f18 I noticed that this was applied to for-6.1. I know you do not rebase or change your trees so this request might be rejected based on that alone but would it be possible to cherry-pick this to for-6.0 so that it can be applied to Linus's tree quicker? We have had to apply this change to our CI to keep our builds green in mainline, -tip, and 5.19/5.15 stable with clang-16 due to -Werror. If not, no worries, I should have made it clearer that is what I was looking for with the subject prefix. Cheers, Nathan
On Tue, Aug 23, 2022 at 08:39:13AM -0700, Nathan Chancellor wrote: > I noticed that this was applied to for-6.1. I know you do not rebase or > change your trees so this request might be rejected based on that alone > but would it be possible to cherry-pick this to for-6.0 so that it can > be applied to Linus's tree quicker? We have had to apply this change to > our CI to keep our builds green in mainline, -tip, and 5.19/5.15 stable > with clang-16 due to -Werror. If not, no worries, I should have made it > clearer that is what I was looking for with the subject prefix. Hrm, OK - it's a bit surprising that this didn't get fixed in -next before the clang change made it to mainline TBH, it looked like something that had just hit -next.
On Tue, Aug 23, 2022 at 05:03:58PM +0100, Mark Brown wrote: > On Tue, Aug 23, 2022 at 08:39:13AM -0700, Nathan Chancellor wrote: > > > I noticed that this was applied to for-6.1. I know you do not rebase or > > change your trees so this request might be rejected based on that alone > > but would it be possible to cherry-pick this to for-6.0 so that it can > > be applied to Linus's tree quicker? We have had to apply this change to > > our CI to keep our builds green in mainline, -tip, and 5.19/5.15 stable > > with clang-16 due to -Werror. If not, no worries, I should have made it > > clearer that is what I was looking for with the subject prefix. > > Hrm, OK - it's a bit surprising that this didn't get fixed in -next > before the clang change made it to mainline TBH, it looked like > something that had just hit -next. Right, sorry for not making that more clear in the commit message. The change in clang was made only a few hours before this patch so I did fix it quickly but we do not usually get any heads up on new warnings. They just appear then we fix them and move on. I'll make it clearer where I want the patch to go in the future, thanks for accommodating this one time. Cheers, Nathan
On Tue, 9 Aug 2022 18:08:09 -0700, Nathan Chancellor wrote: > A recent change in clang strengthened its -Wbitfield-constant-conversion > to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can > only be 0 or -1, not 1: > > sound/soc/atmel/mchp-spdiftx.c:505:20: error: implicit truncation from 'int' to bit-field changes value from 1 to -1 [-Werror,-Wbitfield-constant-conversion] > dev->gclk_enabled = 1; > ^ ~ > 1 error generated. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion commit: 5c5c2baad2b55cc0a4b190266889959642298f79 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c index 4850a177803d..ab2d7a791f39 100644 --- a/sound/soc/atmel/mchp-spdiftx.c +++ b/sound/soc/atmel/mchp-spdiftx.c @@ -196,7 +196,7 @@ struct mchp_spdiftx_dev { struct clk *pclk; struct clk *gclk; unsigned int fmt; - int gclk_enabled:1; + unsigned int gclk_enabled:1; }; static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev)
A recent change in clang strengthened its -Wbitfield-constant-conversion to warn when 1 is assigned to a 1-bit signed integer bitfield, as it can only be 0 or -1, not 1: sound/soc/atmel/mchp-spdiftx.c:505:20: error: implicit truncation from 'int' to bit-field changes value from 1 to -1 [-Werror,-Wbitfield-constant-conversion] dev->gclk_enabled = 1; ^ ~ 1 error generated. The actual value of the field is never checked, just that it is not zero, so there is not a real bug here. However, it is simple enough to silence the warning by making the bitfield unsigned, which matches the mchp-spdifrx driver. Fixes: 06ca24e98e6b ("ASoC: mchp-spdiftx: add driver for S/PDIF TX Controller") Link: https://github.com/ClangBuiltLinux/linux/issues/1686 Link: https://github.com/llvm/llvm-project/commit/82afc9b169a67e8b8a1862fb9c41a2cd974d6691 Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- sound/soc/atmel/mchp-spdiftx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 15205c2829ca2cbb5ece5ceaafe1171a8470e62b