Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: Nathan Chancellor <natechancellor@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Pavel Machek <pavel@ucw.cz>, Bryan Wu <bryan.wu@canonical.com>,
	"G.Shark Jeong" <gshark.jeong@gmail.com>,
	Dan Murphy <dmurphy@ti.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org,
	clang-built-linux@googlegroups.com
Subject: Re: [PATCH] leds: lm355x: avoid enum conversion warning
Date: Tue, 5 May 2020 19:44:16 -0700
Message-ID: <20200506024416.GB415100@ubuntu-s3-xlarge-x86> (raw)
In-Reply-To: <20200505141928.923428-1-arnd@arndb.de>

On Tue, May 05, 2020 at 04:19:17PM +0200, Arnd Bergmann wrote:
> clang points out that doing arithmetic between diffent enums is usually
                                                 ^ different
> a mistake:
> 
> drivers/leds/leds-lm355x.c:167:28: warning: bitwise operation between different enumeration types ('enum lm355x_tx2' and 'enum lm355x_ntc') [-Wenum-enum-conversion]
>                 reg_val = pdata->pin_tx2 | pdata->ntc_pin;
>                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> drivers/leds/leds-lm355x.c:178:28: warning: bitwise operation between different enumeration types ('enum lm355x_tx2' and 'enum lm355x_ntc') [-Wenum-enum-conversion]
>                 reg_val = pdata->pin_tx2 | pdata->ntc_pin | pdata->pass_mode;
>                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
> 
> In this driver, it is intentional, so add a cast to hide the false-positive

Not sure that I would call this a false positive. The warning is correct
that there is a bitwise operation between different enumeration types
but we do not care since we are just using the enumerated type for its
integer value in lieu of a #define VAR value.

> warning. It appears to be the only instance of this warning at the moment.
> 
> Fixes: b98d13c72592 ("leds: Add new LED driver for lm355x chips")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/leds/leds-lm355x.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
> index 11ce05249751..b2eb2e1e9c04 100644
> --- a/drivers/leds/leds-lm355x.c
> +++ b/drivers/leds/leds-lm355x.c
> @@ -164,18 +164,19 @@ static int lm355x_chip_init(struct lm355x_chip_data *chip)
>  	/* input and output pins configuration */
>  	switch (chip->type) {
>  	case CHIP_LM3554:
> -		reg_val = pdata->pin_tx2 | pdata->ntc_pin;
> +		reg_val = (u32)pdata->pin_tx2 | (u32)pdata->ntc_pin;
>  		ret = regmap_update_bits(chip->regmap, 0xE0, 0x28, reg_val);
>  		if (ret < 0)
>  			goto out;
> -		reg_val = pdata->pass_mode;
> +		reg_val = (u32)pdata->pass_mode;

Is this cast needed? I don't think there should be warning from going
from an enumerated type to unsigned int.

>  		ret = regmap_update_bits(chip->regmap, 0xA0, 0x04, reg_val);
>  		if (ret < 0)
>  			goto out;
>  		break;
>  
>  	case CHIP_LM3556:
> -		reg_val = pdata->pin_tx2 | pdata->ntc_pin | pdata->pass_mode;
> +		reg_val = (u32)pdata->pin_tx2 | (u32)pdata->ntc_pin |
> +		          (u32)pdata->pass_mode;
>  		ret = regmap_update_bits(chip->regmap, 0x0A, 0xC4, reg_val);
>  		if (ret < 0)
>  			goto out;
> -- 
> 2.26.0
> 

With those comments addressed, feel free to add:

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 14:19 Arnd Bergmann
2020-05-06  2:44 ` Nathan Chancellor [this message]
2020-05-06 14:19   ` Arnd Bergmann
2020-05-06 15:35     ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200506024416.GB415100@ubuntu-s3-xlarge-x86 \
    --to=natechancellor@gmail.com \
    --cc=arnd@arndb.de \
    --cc=bryan.wu@canonical.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dmurphy@ti.com \
    --cc=gshark.jeong@gmail.com \
    --cc=jacek.anaszewski@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=pavel@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git