leds: sun50i-a100: avoid division-by-zero warning

PATCH Applied Arnd Bergmann 4 participants 5 messages Dec 12, 2023 – Dec 13, 2023

Subsystems (2)

Arm/Allwinner sunXi SoC support LED

lore

Share & tools

lore

Arnd Bergmann

author

2023-12-12 21:45 UTC #0 lore raw

PATCH 1/1 leds: sun50i-a100: avoid division-by-zero warning

From: Arnd Bergmann <[email protected]>

When CONFIG_COMMON_CLK is disabled, e.g. on an x86 randconfig compile test, clang reports a field overflow from propagating the result of a division by zero:

drivers/leds/leds-sun50i-a100.c:309:12: error: call to '__compiletime_assert_265' declared with 'error' attribute: FIELD_PREP: value too large for the field

        control = FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1H, timing->t1h_ns / cycle_ns) |

Avoid the problem by adding an explicit check for the zero value here. Alternatively the assertion could be avoided with a Kconfig dependency on COMMON_CLK.

Fixes 090a25ad9798 ("leds: sun50i-a100: New driver for the A100 LED controller")

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/leds/leds-sun50i-a100.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/leds/leds-sun50i-a100.c +5 -1

index e4a7e692a908..171cefd1ea0d 100644
@@ -303,9 +303,13 @@ static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
 {
 	const struct sun50i_a100_ledc_timing *timing = &priv->timing;
 	unsigned long mod_freq = clk_get_rate(priv->mod_clk);
-	u32 cycle_ns = NSEC_PER_SEC / mod_freq;
+	u32 cycle_ns;
 	u32 control;
 
+	if (!mod_freq)
+		return;
+
+	cycle_ns = NSEC_PER_SEC / mod_freq;
 	control = FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1H, timing->t1h_ns / cycle_ns) |
 		  FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1L, timing->t1l_ns / cycle_ns) |
 		  FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T0H, timing->t0h_ns / cycle_ns) |

Signature · 1 line

2.39.2

Guo Ren

2023-12-13 01:26 UTC #1 lore raw

Quoted 24 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>
When CONFIG_COMMON_CLK is disabled, e.g. on an x86 randconfig compile test, clang reports a field overflow from propagating the result of a division by zero:
drivers/leds/leds-sun50i-a100.c:309:12: error: call to '__compiletime_assert_265' declared with 'error' attribute: FIELD_PREP: value too large for the field
        control = FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1H, timing->t1h_ns / cycle_ns) |
Avoid the problem by adding an explicit check for the zero value here. Alternatively the assertion could be avoided with a Kconfig dependency on COMMON_CLK.
Fixes 090a25ad9798 ("leds: sun50i-a100: New driver for the A100 LED controller")
Signed-off-by Arnd Bergmann <[email protected]>
---
 drivers/leds/leds-sun50i-a100.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/leds/leds-sun50i-a100.c +0 -0
index e4a7e692a908..171cefd1ea0d 100644

drivers/leds/leds-sun50i-a100.c static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv) · line 311

Quoted 10 lines

drivers/leds/leds-sun50i-a100.c +4 -1

@@ -303,9 +303,13 @@ static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
 {
        const struct sun50i_a100_ledc_timing *timing = &priv->timing;
        unsigned long mod_freq = clk_get_rate(priv->mod_clk);
-       u32 cycle_ns = NSEC_PER_SEC / mod_freq;
+       u32 cycle_ns;
        u32 control;
 
+       if (!mod_freq)
+               return;
+

Shall we need err_disable_bus_clk?

+ static int sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
- static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)

 {
         const struct sun50i_a100_ledc_timing *timing = &priv->timing;
         unsigned long mod_freq = clk_get_rate(priv->mod_clk);
 -       u32 cycle_ns = NSEC_PER_SEC / mod_freq;
 +       u32 cycle_ns;
         u32 control;

+ if (!mod_freq)
+ return -EINVAL;

+ cycle_ns = NSEC_PER_SEC / mod_freq;
control = FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1H,
timing->t1h_ns / cycle_ns) |

                   FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1L,
timing->t1l_ns / cycle_ns) |
                   FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T0H,
timing->t0h_ns / cycle_ns) |
...
       return 0;

----------

+ ret = sun50i_a100_ledc_set_timing(priv);
+ if (ret)
+ goto err_disable_bus_clk;

Quoted 3 lines

drivers/leds/leds-sun50i-a100.c +0 -1
--
2.39.2

Signature · 2 lines

Best Regards
 Guo Ren

Arnd Bergmann

2023-12-13 06:32 UTC #2 lore raw

Quoted 8 lines · Guo Ren

Quoted 7 lines · Arnd Bergmann

 {
        const struct sun50i_a100_ledc_timing *timing = &priv->timing;
        unsigned long mod_freq = clk_get_rate(priv->mod_clk);

drivers/leds/leds-sun50i-a100.c +1 -1

-       u32 cycle_ns = NSEC_PER_SEC / mod_freq;
+       u32 cycle_ns;
        u32 control;

Quoted 7 lines

drivers/leds/leds-sun50i-a100.c static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv) · line 311
Quoted 3 lines
drivers/leds/leds-sun50i-a100.c +3 -0
+       if (!mod_freq)
+               return;
+
Shall we need err_disable_bus_clk?
+ static int sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
- static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)

I'm not worried about it too much, as there is already an error check on priv->mod_clk being unavailable during initialization. The case that the warning is about is just for build-testing on architectures that don't even use COMMON_CLK.

Arnd

Guo Ren

Reviewed-by

2023-12-13 14:51 UTC #3 lore raw

Quoted 15 lines · Arnd Bergmann

Quoted 13 lines · Guo Ren
drivers/leds/leds-sun50i-a100.c static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv) · line 311
Quoted 10 lines · Arnd Bergmann
 {
        const struct sun50i_a100_ledc_timing *timing = &priv->timing;
        unsigned long mod_freq = clk_get_rate(priv->mod_clk);
drivers/leds/leds-sun50i-a100.c +4 -1
-       u32 cycle_ns = NSEC_PER_SEC / mod_freq;
+       u32 cycle_ns;
        u32 control;
 
+       if (!mod_freq)
+               return;
+
Shall we need err_disable_bus_clk?

Quoted 7 lines

Quoted 2 lines
+ static int sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
- static void sun50i_a100_ledc_set_timing(struct sun50i_a100_ledc *priv)
I'm not worried about it too much, as there is already an error check on priv->mod_clk being unavailable during initialization. The case that the warning is about is just for build-testing on architectures that don't even use COMMON_CLK.

Okay

Reviewed-by Guo Ren <[email protected]>

Quoted 2 lines

Arnd

Signature · 2 lines

Best Regards
 Guo Ren

Lee Jones

maintainer Applied

2023-12-13 16:16 UTC #4 lore raw

Re: (subset) [PATCH] leds: sun50i-a100: avoid division-by-zero warning

In reply to Arnd Bergmann

Quoted 8 lines · Arnd Bergmann

When CONFIG_COMMON_CLK is disabled, e.g. on an x86 randconfig compile test, clang reports a field overflow from propagating the result of a division by zero:
drivers/leds/leds-sun50i-a100.c:309:12: error: call to '__compiletime_assert_265' declared with 'error' attribute: FIELD_PREP: value too large for the field
        control = FIELD_PREP(LEDC_T01_TIMING_CTRL_REG_T1H, timing->t1h_ns / cycle_ns) |
[...]

Applied, thanks!

[1/1] leds: sun50i-a100: avoid division-by-zero warning

      commit: f969d75a0218da32c40dd4940bd430b0530433cf

--
Lee Jones [李琼斯]