diff mbox series

hwmon: (aspeed-g6-pwm-tacho): Drop cpp define only used once

Message ID 20240411160136.247138-2-u.kleine-koenig@pengutronix.de (mailing list archive)
State Rejected
Headers show
Series hwmon: (aspeed-g6-pwm-tacho): Drop cpp define only used once | expand

Commit Message

Uwe Kleine-König April 11, 2024, 4:01 p.m. UTC
The macro PWM_ASPEED_NR_PWMS is only used once, just use it's value in
this single code line.

Having the number of PWM lines explictly in the call to
devm_pwmchip_alloc() also has the advantage to be easily greppable.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

note this patch depends on commit 2c56fedef8c9 (hwmon: (aspeed-g6-pwm-tacho):
Make use of devm_pwmchip_alloc() function) that currently sits in pwm/for-next.

Best regards
Uwe

 drivers/hwmon/aspeed-g6-pwm-tach.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

base-commit: 4118d9533ff3a5d16efb476a0d00afceecd92cf5

Comments

Guenter Roeck April 11, 2024, 6:15 p.m. UTC | #1
On Thu, Apr 11, 2024 at 06:01:36PM +0200, Uwe Kleine-König wrote:
> The macro PWM_ASPEED_NR_PWMS is only used once, just use it's value in
> this single code line.
> 

I am not part of the thou-shalt-not-use-defines-if-only-used-once
crowd, so I won't take this patch, sorry.

Guenter
Uwe Kleine-König April 12, 2024, 7:15 a.m. UTC | #2
On Thu, Apr 11, 2024 at 11:15:05AM -0700, Guenter Roeck wrote:
> On Thu, Apr 11, 2024 at 06:01:36PM +0200, Uwe Kleine-König wrote:
> > The macro PWM_ASPEED_NR_PWMS is only used once, just use it's value in
> > this single code line.
> 
> I am not part of the thou-shalt-not-use-defines-if-only-used-once
> crowd, so I won't take this patch, sorry.

My patch wasn't about religion. It's more that I was annoyed that

	git grep pwmchip_alloc next/master drivers/hwmon

doesn't give me the number of PWM channels. That PWM_ASPEED_NR_PWMS is
only used once then only the detail that makes it easy to actually
change that.

So in my eyes there is no advantage in this define and the only effect
is that it hides information.

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/hwmon/aspeed-g6-pwm-tach.c b/drivers/hwmon/aspeed-g6-pwm-tach.c
index 08a2ded95e45..706c344b181b 100644
--- a/drivers/hwmon/aspeed-g6-pwm-tach.c
+++ b/drivers/hwmon/aspeed-g6-pwm-tach.c
@@ -59,8 +59,6 @@ 
 #include <linux/reset.h>
 #include <linux/sysfs.h>
 
-/* The channel number of Aspeed pwm controller */
-#define PWM_ASPEED_NR_PWMS			16
 /* PWM Control Register */
 #define PWM_ASPEED_CTRL(ch)			((ch) * 0x10 + 0x00)
 #define PWM_ASPEED_CTRL_LOAD_SEL_RISING_AS_WDT	BIT(19)
@@ -487,7 +485,7 @@  static int aspeed_pwm_tach_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	chip = devm_pwmchip_alloc(dev, PWM_ASPEED_NR_PWMS, 0);
+	chip = devm_pwmchip_alloc(dev, 16, 0);
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);