[PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion
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/82afc9b169a67e8b8a1862fb9c41a2cd... Signed-off-by: Nathan Chancellor nathan@kernel.org --- 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
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/82afc9b169a67e8b8a1862fb9c41a2cd... 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
participants (4)
-
David Laight -
Mark Brown -
Nathan Chancellor -
Nick Desaulniers