[PATCH] misc: hisi_hikey_usb: Use str_enabled_disabled() in hub_power_ctrl()

Thorsten Blum posted 1 patch 1 month, 3 weeks ago
drivers/misc/hisi_hikey_usb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] misc: hisi_hikey_usb: Use str_enabled_disabled() in hub_power_ctrl()
Posted by Thorsten Blum 1 month, 3 weeks ago
Remove hard-coded strings by using the str_enabled_disabled() helper
function and silence the following Coccinelle/coccicheck warning
reported by string_choices.cocci:

  opportunity for str_enabled_disabled(value)

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/misc/hisi_hikey_usb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
index ffe7b945a298..2c6e448a47f1 100644
--- a/drivers/misc/hisi_hikey_usb.c
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -18,6 +18,7 @@
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/string_choices.h>
 #include <linux/usb/role.h>
 
 #define DEVICE_DRIVER_NAME "hisi_hikey_usb"
@@ -67,7 +68,7 @@ static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
 	if (ret)
 		dev_err(hisi_hikey_usb->dev,
 			"Can't switch regulator state to %s\n",
-			value ? "enabled" : "disabled");
+			str_enabled_disabled(value));
 }
 
 static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
-- 
2.50.1
Re: [PATCH] misc: hisi_hikey_usb: Use str_enabled_disabled() in hub_power_ctrl()
Posted by John Stultz 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 11:01 AM Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> Remove hard-coded strings by using the str_enabled_disabled() helper
> function and silence the following Coccinelle/coccicheck warning
> reported by string_choices.cocci:
>
>   opportunity for str_enabled_disabled(value)

Thanks for submitting this!

So "silence a warning" isn't really a great argument for *why* this is
being done.

I wasn't actually familiar with str_enabled_disabled, and initially
this change felt a bit arbitrary: The length of the function name
barely saves code characters - and it only seems to make the logic a
little less readable - so what is the point?

But after digging around it seems the benefit comes from allowing the
"enabled" and "disabled" string constants to be consolidated across
numerous uses. Which saves memory and makes sense.

So it might be good to add some detail in the commit message about
that, just so it's clear why this change is helpful.

Otherwise,
Acked-by: John Stultz <jstultz@google.com>