[PATCH v2] leds: bcm6328: Replace divide condition with comparison for shift value

Dipendra Khadka posted 1 patch 1 month, 1 week ago
drivers/leds/leds-bcm6328.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] leds: bcm6328: Replace divide condition with comparison for shift value
Posted by Dipendra Khadka 1 month, 1 week ago
Fixes the following Smatch warnings:
drivers/leds/leds-bcm6328.c:116 bcm6328_led_mode() warn: replace divide condition 'shift / 16' with 'shift >= 16'
drivers/leds/leds-bcm6328.c:360 bcm6328_led() warn: replace divide condition 'shift / 16' with 'shift >= 16'

Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
v2:
 - Changed the patch subject as suggested
 - Changed the patch description
v1: https://lore.kernel.org/all/20240922174020.49856-1-kdipendra88@gmail.com/
 drivers/leds/leds-bcm6328.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index 29f5bad61796..592bbf4b7e35 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -113,7 +113,7 @@ static void bcm6328_led_mode(struct bcm6328_led *led, unsigned long value)
 	unsigned long val, shift;
 
 	shift = bcm6328_pin2shift(led->pin);
-	if (shift / 16)
+	if (shift >= 16)
 		mode = led->mem + BCM6328_REG_MODE_HI;
 	else
 		mode = led->mem + BCM6328_REG_MODE_LO;
@@ -357,7 +357,7 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 		break;
 	case LEDS_DEFSTATE_KEEP:
 		shift = bcm6328_pin2shift(led->pin);
-		if (shift / 16)
+		if (shift >= 16)
 			mode = mem + BCM6328_REG_MODE_HI;
 		else
 			mode = mem + BCM6328_REG_MODE_LO;
-- 
2.43.0
Re: [PATCH v2] leds: bcm6328: Replace divide condition with comparison for shift value
Posted by Andy Shevchenko 2 days, 10 hours ago
Sat, Oct 19, 2024 at 07:33:01AM +0000, Dipendra Khadka kirjoitti:
> Fixes the following Smatch warnings:
> drivers/leds/leds-bcm6328.c:116 bcm6328_led_mode() warn: replace divide condition 'shift / 16' with 'shift >= 16'
> drivers/leds/leds-bcm6328.c:360 bcm6328_led() warn: replace divide condition 'shift / 16' with 'shift >= 16'

As discussed with Dan (author of smatch), these warnings make no much sense as
"division" by power-of-two numbers in modern world (ISAs, compilers, etc) most
unlikely become a real division instruction with all bad consequences. Even
though, some ISAs can optimize cases where foo / 16; foo % 16 goes together and
this change may break that.

So, please, consider not to send such changes in the future because at least
they are just noise, at most might add a (micro-)rgressions.

https://lore.kernel.org/linux-gpio/20241112201659.16785-1-surajsonawane0215@gmail.com/
https://lore.kernel.org/all/20241121105838.4073659-4-andriy.shevchenko@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko
Re: (subset) [PATCH v2] leds: bcm6328: Replace divide condition with comparison for shift value
Posted by Lee Jones 3 weeks, 4 days ago
On Sat, 19 Oct 2024 07:33:01 +0000, Dipendra Khadka wrote:
> Fixes the following Smatch warnings:
> drivers/leds/leds-bcm6328.c:116 bcm6328_led_mode() warn: replace divide condition 'shift / 16' with 'shift >= 16'
> drivers/leds/leds-bcm6328.c:360 bcm6328_led() warn: replace divide condition 'shift / 16' with 'shift >= 16'
> 
> 

Applied, thanks!

[1/1] leds: bcm6328: Replace divide condition with comparison for shift value
      commit: 5f57672cf4629542b525cfef027e293656915368

--
Lee Jones [李琼斯]