| Message ID | 20250114203547.1013010-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pmdomain: Use str_enable_disable-like helpers | expand |
Hi, Krzysztof Thanks for your patch. > Replace ternary (condition ? "enable" : "disable") 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: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/pmdomain/renesas/rcar-gen4-sysc.c | 3 ++- > drivers/pmdomain/renesas/rcar-sysc.c | 3 ++- > drivers/pmdomain/samsung/exynos-pm-domains.c | 6 +++--- > drivers/pmdomain/starfive/jh71xx-pmu.c | 3 ++- > 4 files changed, 9 insertions(+), 6 deletions(-) [...] > diff --git a/drivers/pmdomain/starfive/jh71xx-pmu.c > b/drivers/pmdomain/starfive/jh71xx-pmu.c > index 74720c09a6e3..30c29ac9391f 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; > } In jh71xx-pmu.c jh71xx_pmu_set_state(), maybe you can also change this line: if (is_on == on) { dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n", pmd->genpd.name, on ? "en" : "dis"); return 0; } ====> if (is_on == on) { dev_dbg(pmu->dev, "pm domain [%s] is already %s status.\n", pmd->genpd.name, str_enable_disable(on)); return 0; } Best Regards, Changhuang
On Tue, Jan 14, 2025 at 9:36 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > Replace ternary (condition ? "enable" : "disable") 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: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On 15/01/2025 02:55, Changhuang Liang wrote: > > In jh71xx-pmu.c jh71xx_pmu_set_state(), maybe you can also change this line: > > if (is_on == on) { > dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n", > pmd->genpd.name, on ? "en" : "dis"); > return 0; > } > ====> > if (is_on == on) { > dev_dbg(pmu->dev, "pm domain [%s] is already %s status.\n", > pmd->genpd.name, str_enable_disable(on)); Sure Best regards, Krzysztof
diff --git a/drivers/pmdomain/renesas/rcar-gen4-sysc.c b/drivers/pmdomain/renesas/rcar-gen4-sysc.c index 66409cff2083..fe9a0c1deaa3 100644 --- a/drivers/pmdomain/renesas/rcar-gen4-sysc.c +++ b/drivers/pmdomain/renesas/rcar-gen4-sysc.c @@ -17,6 +17,7 @@ #include <linux/pm_domain.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/string_choices.h> #include <linux/types.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 b99326917330..e3f2c7edf22a 100644 --- a/drivers/pmdomain/renesas/rcar-sysc.c +++ b/drivers/pmdomain/renesas/rcar-sysc.c @@ -14,6 +14,7 @@ #include <linux/pm_domain.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/string_choices.h> #include <linux/io.h> #include <linux/iopoll.h> #include <linux/soc/renesas/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/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..30c29ac9391f 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; }
Replace ternary (condition ? "enable" : "disable") 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: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/pmdomain/renesas/rcar-gen4-sysc.c | 3 ++- drivers/pmdomain/renesas/rcar-sysc.c | 3 ++- drivers/pmdomain/samsung/exynos-pm-domains.c | 6 +++--- drivers/pmdomain/starfive/jh71xx-pmu.c | 3 ++- 4 files changed, 9 insertions(+), 6 deletions(-)