[PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Nathan Chancellor]

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(-)

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

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Nick Desaulniers]

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
>


-- 
Thanks,
~Nick Desaulniers

RE: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[David Laight]

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)

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Mark Brown]

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

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Nathan Chancellor]

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

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Nathan Chancellor]

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

Re: [PATCH] ASoC: mchp-spdiftx: Fix clang -Wbitfield-constant-conversion

[Mark Brown]

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