2023-12-12 21:46:15

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] 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(-)

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) |
--
2.39.2


2023-12-13 01:26:27

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH] leds: sun50i-a100: avoid division-by-zero warning

On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <[email protected]> wrote:
>
> 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(-)
>
> 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
>


--
Best Regards
Guo Ren

2023-12-13 06:32:53

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] leds: sun50i-a100: avoid division-by-zero warning

On Wed, Dec 13, 2023, at 02:26, Guo Ren wrote:
> On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <[email protected]> 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

2023-12-13 14:51:44

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH] leds: sun50i-a100: avoid division-by-zero warning

On Wed, Dec 13, 2023 at 2:32 PM Arnd Bergmann <[email protected]> wrote:
>
> On Wed, Dec 13, 2023, at 02:26, Guo Ren wrote:
> > On Wed, Dec 13, 2023 at 5:45 AM Arnd Bergmann <[email protected]> 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 <[email protected]>

>
> Arnd



--
Best Regards
Guo Ren

2023-12-13 16:16:54

by Lee Jones

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

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 [李琼斯]