| Message ID | 20231212214536.175327-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | leds: sun50i-a100: avoid division-by-zero warning | expand |
On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > 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 <arnd@arndb.de> > --- > drivers/leds/leds-sun50i-a100.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/leds/leds-sun50i-a100.c b/drivers/leds/leds-sun50i-a100.c > index e4a7e692a908..171cefd1ea0d 100644 > --- a/drivers/leds/leds-sun50i-a100.c > +++ b/drivers/leds/leds-sun50i-a100.c > @@ -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; > -- > 2.39.2 >
On Wed, Dec 13, 2023, at 02:26, Guo Ren wrote: > On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <arnd@kernel.org> wrote: >> { >> 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) 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
On Wed, Dec 13, 2023 at 2:32 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Dec 13, 2023, at 02:26, Guo Ren wrote: > > On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > >> { > >> 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) > > 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 <guoren@kernel.org> > > Arnd
On Tue, 12 Dec 2023 22:45:22 +0100, Arnd Bergmann wrote: > 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 [李琼斯]
diff --git a/drivers/leds/leds-sun50i-a100.c b/drivers/leds/leds-sun50i-a100.c index e4a7e692a908..171cefd1ea0d 100644 --- a/drivers/leds/leds-sun50i-a100.c +++ b/drivers/leds/leds-sun50i-a100.c @@ -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) |