[PATCH v2] pmdomain: Use str_enable_disable-like helpers

shao.mingyin@zte.com.cn posted 1 patch 4 months ago
drivers/pmdomain/renesas/rcar-gen4-sysc.c    | 3 ++-
drivers/pmdomain/renesas/rcar-sysc.c         | 3 ++-
drivers/pmdomain/rockchip/pm-domains.c       | 3 ++-
drivers/pmdomain/samsung/exynos-pm-domains.c | 6 +++---
drivers/pmdomain/starfive/jh71xx-pmu.c       | 7 ++++---
5 files changed, 13 insertions(+), 9 deletions(-)
[PATCH v2] pmdomain: Use str_enable_disable-like helpers
Posted by shao.mingyin@zte.com.cn 4 months ago
From: Shao Mingyin <shao.mingyin@zte.com.cn>

Replace ternary (condition ? "enable" : "disable") syntax and ternary
(condition ? "on" : "off") syntax with helpers from
string_choices.h because:
1. Simple function call with one argument is easier to read.  Ternary
   operator has three arguments and with wrapping might lead to quite
   long code.
2. Is slightly shorter thus also easier to read.
3. It brings uniformity in the text - same string.
4. Allows deduping by the linker, which results in a smaller binary
   file.

Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
v2:
 %sable ==> %s in jh71xx_pmu_dev()
 drivers/pmdomain/renesas/rcar-gen4-sysc.c    | 3 ++-
 drivers/pmdomain/renesas/rcar-sysc.c         | 3 ++-
 drivers/pmdomain/rockchip/pm-domains.c       | 3 ++-
 drivers/pmdomain/samsung/exynos-pm-domains.c | 6 +++---
 drivers/pmdomain/starfive/jh71xx-pmu.c       | 7 ++++---
 5 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/pmdomain/renesas/rcar-gen4-sysc.c b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
index e001b5c25bed..c8aa7538e95f 100644
--- a/drivers/pmdomain/renesas/rcar-gen4-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <linux/string_choices.h>

 #include "rcar-gen4-sysc.h"

@@ -171,7 +172,7 @@ static int rcar_gen4_sysc_power(u8 pdr, bool on)
  out:
        spin_unlock_irqrestore(&rcar_gen4_sysc_lock, flags);

-       pr_debug("sysc power %s domain %d: %08x -> %d\n", on ? "on" : "off",
+       pr_debug("sysc power %s domain %d: %08x -> %d\n", str_on_off(on),
                 pdr, ioread32(rcar_gen4_sysc_base + SYSCISCR(reg_idx)), ret);
        return ret;
 }
diff --git a/drivers/pmdomain/renesas/rcar-sysc.c b/drivers/pmdomain/renesas/rcar-sysc.c
index 047495f54e8a..dae01ca0ef6a 100644
--- a/drivers/pmdomain/renesas/rcar-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-sysc.c
@@ -17,6 +17,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/soc/renesas/rcar-sysc.h>
+#include <linux/string_choices.h>

 #include "rcar-sysc.h"

@@ -162,7 +163,7 @@ static int rcar_sysc_power(const struct rcar_sysc_pd *pd, bool on)

        spin_unlock_irqrestore(&rcar_sysc_lock, flags);

-       pr_debug("sysc power %s domain %d: %08x -> %d\n", on ? "on" : "off",
+       pr_debug("sysc power %s domain %d: %08x -> %d\n", str_on_off(on),
                 pd->isr_bit, ioread32(rcar_sysc_base + SYSCISR), ret);
        return ret;
 }
diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 4cce407bb1eb..0681c763f843 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -21,6 +21,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
+#include <linux/string_choices.h>
 #include <soc/rockchip/pm_domains.h>
 #include <soc/rockchip/rockchip_sip.h>
 #include <dt-bindings/power/px30-power.h>
@@ -595,7 +596,7 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
                                        is_on == on, 0, 10000);
        if (ret) {
                dev_err(pmu->dev, "failed to set domain '%s' %s, val=%d\n",
-                       genpd->name, on ? "on" : "off", is_on);
+                       genpd->name, str_on_off(on), is_on);
                return ret;
        }

diff --git a/drivers/pmdomain/samsung/exynos-pm-domains.c b/drivers/pmdomain/samsung/exynos-pm-domains.c
index 9b502e8751d1..1a892c611dad 100644
--- a/drivers/pmdomain/samsung/exynos-pm-domains.c
+++ b/drivers/pmdomain/samsung/exynos-pm-domains.c
@@ -13,6 +13,7 @@
 #include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/string_choices.h>
 #include <linux/pm_domain.h>
 #include <linux/delay.h>
 #include <linux/of.h>
@@ -38,7 +39,6 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
        struct exynos_pm_domain *pd;
        void __iomem *base;
        u32 timeout, pwr;
-       char *op;

        pd = container_of(domain, struct exynos_pm_domain, pd);
        base = pd->base;
@@ -51,8 +51,8 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)

        while ((readl_relaxed(base + 0x4) & pd->local_pwr_cfg) != pwr) {
                if (!timeout) {
-                       op = (power_on) ? "enable" : "disable";
-                       pr_err("Power domain %s %s failed\n", domain->name, op);
+                       pr_err("Power domain %s %s failed\n", domain->name,
+                              str_enable_disable(power_on));
                        return -ETIMEDOUT;
                }
                timeout--;
diff --git a/drivers/pmdomain/starfive/jh71xx-pmu.c b/drivers/pmdomain/starfive/jh71xx-pmu.c
index 74720c09a6e3..dc3e109e273a 100644
--- a/drivers/pmdomain/starfive/jh71xx-pmu.c
+++ b/drivers/pmdomain/starfive/jh71xx-pmu.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/string_choices.h>
 #include <dt-bindings/power/starfive,jh7110-pmu.h>

 /* register offset */
@@ -155,7 +156,7 @@ static int jh7110_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)

        if (ret) {
                dev_err(pmu->dev, "%s: failed to power %s\n",
-                       pmd->genpd.name, on ? "on" : "off");
+                       pmd->genpd.name, str_on_off(on));
                return -ETIMEDOUT;
        }

@@ -197,8 +198,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
        }

        if (is_on == on) {
-               dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n",
-                       pmd->genpd.name, on ? "en" : "dis");
+               dev_dbg(pmu->dev, "pm domain [%s] is already %s status.\n",
+                       pmd->genpd.name, str_enable_disable(on));
                return 0;
        }

-- 
2.25.1
Re: [PATCH v2] pmdomain: Use str_enable_disable-like helpers
Posted by Geert Uytterhoeven 4 months ago
Hi Mingyin,

On Tue, 10 Jun 2025 at 13:34, <shao.mingyin@zte.com.cn> wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
>
> Replace ternary (condition ? "enable" : "disable") syntax and ternary
> (condition ? "on" : "off") syntax with helpers from
> string_choices.h because:
> 1. Simple function call with one argument is easier to read.  Ternary
>    operator has three arguments and with wrapping might lead to quite
>    long code.
> 2. Is slightly shorter thus also easier to read.
> 3. It brings uniformity in the text - same string.
> 4. Allows deduping by the linker, which results in a smaller binary
>    file.
>
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> v2:
>  %sable ==> %s in jh71xx_pmu_dev()

Thanks for the update!

While I already provided my Rb-tag on Krzysztof's original [1], your
version is whitespace-damaged (TABs replaced by spaces), and thus
cannot be applied.

[1] https://lore.kernel.org/all/CAMuHMdXJ57mATWW4AnBedn+D7TQ4PadkJ642daquFtAo=wZFrQ@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH v2] pmdomain: Use str_enable_disable-like helpers
Posted by Krzysztof Kozlowski 4 months ago
On 10/06/2025 13:34, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> Replace ternary (condition ? "enable" : "disable") syntax and ternary
> (condition ? "on" : "off") syntax with helpers from
> string_choices.h because:
> 1. Simple function call with one argument is easier to read.  Ternary
>    operator has three arguments and with wrapping might lead to quite
>    long code.
> 2. Is slightly shorter thus also easier to read.
> 3. It brings uniformity in the text - same string.
> 4. Allows deduping by the linker, which results in a smaller binary
>    file.

So you just taken everything from the same my patch - even entire commit
subject and commit description - and sent it as yours?

https://lore.kernel.org/all/20250114203547.1013010-1-krzysztof.kozlowski@linaro.org/

oh my, if doing EXACTLY the same keep original authorship - the From and
Sob fields.

Best regards,
Krzysztof
Re: [PATCH v2] pmdomain: Use str_enable_disable-like helpers
Posted by shao.mingyin@zte.com.cn 4 months ago
>> From: Shao Mingyin <shao.mingyin@zte.com.cn>
>> 
>> Replace ternary (condition ? "enable" : "disable") syntax and ternary
>> (condition ? "on" : "off") syntax with helpers from
>> string_choices.h because:
>> 1. Simple function call with one argument is easier to read.  Ternary
>>    operator has three arguments and with wrapping might lead to quite
>>    long code.
>> 2. Is slightly shorter thus also easier to read.
>> 3. It brings uniformity in the text - same string.
>> 4. Allows deduping by the linker, which results in a smaller binary
>>    file.
>
>So you just taken everything from the same my patch - even entire commit
>subject and commit description - and sent it as yours?
>
>https://lore.kernel.org/all/20250114203547.1013010-1-krzysztof.kozlowski@linaro.org/
>
>oh my, if doing EXACTLY the same keep original authorship - the From and
>Sob fields.
>
>Best regards,
>Krzysztof
Dear Krzysztof,
Thank you for your suggestions. I have carefully read your advice and
made adjustments to the patches accordingly. I used your patch as a
reference standard, not just taking everything from the same your patch.

Based on your suggestion, I have consolidated the series of patches for
the pmdomain driver into a single patch. Additionally, following
@changhuang's suggestion, I have supplemented the patch for
drivers/pmdomain/starfive/jh71xx-pmu.c.

If there's anything inappropriate in this patch, I sincerely apologize.

Best regards,
Mingyin
Re: [PATCH v2] pmdomain: Use str_enable_disable-like helpers
Posted by Krzysztof Kozlowski 4 months ago
On 11/06/2025 04:10, shao.mingyin@zte.com.cn wrote:
>>> From: Shao Mingyin <shao.mingyin@zte.com.cn>
>>>
>>> Replace ternary (condition ? "enable" : "disable") syntax and ternary
>>> (condition ? "on" : "off") syntax with helpers from
>>> string_choices.h because:
>>> 1. Simple function call with one argument is easier to read.  Ternary
>>>    operator has three arguments and with wrapping might lead to quite
>>>    long code.
>>> 2. Is slightly shorter thus also easier to read.
>>> 3. It brings uniformity in the text - same string.
>>> 4. Allows deduping by the linker, which results in a smaller binary
>>>    file.
>>
>> So you just taken everything from the same my patch - even entire commit
>> subject and commit description - and sent it as yours?
>>
>> https://lore.kernel.org/all/20250114203547.1013010-1-krzysztof.kozlowski@linaro.org/
>>
>> oh my, if doing EXACTLY the same keep original authorship - the From and
>> Sob fields.
>>
>> Best regards,
>> Krzysztof
> Dear Krzysztof,
> Thank you for your suggestions. I have carefully read your advice and
> made adjustments to the patches accordingly. I used your patch as a
> reference standard, not just taking everything from the same your patch.
> 
> Based on your suggestion, I have consolidated the series of patches for

I did not suggest to take everything from my patch, add one missing
change and use as yours.



Best regards,
Krzysztof