| Message ID | 20210727035144.298277-1-seanga2@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Andes |
| Headers | show |
| Series | [1/4] clk: k210: Fix checking if ulongs are less than 0 | expand |
On 2021/07/27 12:51, Sean Anderson wrote: > Some clock functions return ulong but still have "negative" errors. To deal > with this, cast the relevant arguments to long. > > Fixes: 609bd60b94 ("clk: k210: Rewrite to remove CCF") > Reported-by: Coverity Scan <scan-admin@coverity.com> > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > drivers/clk/clk_kendryte.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/clk_kendryte.c b/drivers/clk/clk_kendryte.c > index 3148756968..37bd624eca 100644 > --- a/drivers/clk/clk_kendryte.c > +++ b/drivers/clk/clk_kendryte.c > @@ -439,7 +439,7 @@ static const struct k210_clk_params k210_clks[] = { > #ifdef CONFIG_CLK_K210_SET_RATE > static int k210_pll_enable(struct k210_clk_priv *priv, int id); > static int k210_pll_disable(struct k210_clk_priv *priv, int id); > -static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, ulong rate_in); > +static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, long rate_in); > > /* > * The PLL included with the Kendryte K210 appears to be a True Circuits, Inc. > @@ -841,7 +841,7 @@ TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in, > } > > static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, Shouldn't this one return a long, in case of error ? It seems that the commit messages hints at such a change, but you are changing the argument type instead. A little confusing. What am I missing ? > - ulong rate_in) > + long rate_in) > { > int err; > const struct k210_pll_params *pll = &k210_plls[id]; > @@ -890,7 +890,7 @@ static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, > #endif /* CONFIG_CLK_K210_SET_RATE */ > > static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, Same here ? > - ulong rate_in) > + long rate_in) I would assume that these functions are called if the rate_in argument is correct, so I do not really understand why the argument type needs to be changed... > { > u64 r, f, od; > u32 reg = readl(priv->base + k210_plls[id].off); >
On 7/27/21 4:15 AM, Damien Le Moal wrote: > On 2021/07/27 12:51, Sean Anderson wrote: >> Some clock functions return ulong but still have "negative" errors. To deal >> with this, cast the relevant arguments to long. >> >> Fixes: 609bd60b94 ("clk: k210: Rewrite to remove CCF") >> Reported-by: Coverity Scan <scan-admin@coverity.com> >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> drivers/clk/clk_kendryte.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/clk/clk_kendryte.c b/drivers/clk/clk_kendryte.c >> index 3148756968..37bd624eca 100644 >> --- a/drivers/clk/clk_kendryte.c >> +++ b/drivers/clk/clk_kendryte.c >> @@ -439,7 +439,7 @@ static const struct k210_clk_params k210_clks[] = { >> #ifdef CONFIG_CLK_K210_SET_RATE >> static int k210_pll_enable(struct k210_clk_priv *priv, int id); >> static int k210_pll_disable(struct k210_clk_priv *priv, int id); >> -static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, ulong rate_in); >> +static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, long rate_in); >> >> /* >> * The PLL included with the Kendryte K210 appears to be a True Circuits, Inc. >> @@ -841,7 +841,7 @@ TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in, >> } >> >> static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, > > Shouldn't this one return a long, in case of error ? It seems that the commit > messages hints at such a change, but you are changing the argument type instead. > A little confusing. What am I missing ? Perhaps they should return long, but these are basically matching the prototypes in include/clk-uclass.h. And there, get_rate and set_rate take ulong arguments and return ulongs. > >> - ulong rate_in) >> + long rate_in) >> { >> int err; >> const struct k210_pll_params *pll = &k210_plls[id]; >> @@ -890,7 +890,7 @@ static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, >> #endif /* CONFIG_CLK_K210_SET_RATE */ >> >> static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, > > Same here ? > >> - ulong rate_in) >> + long rate_in) > > I would assume that these functions are called if the rate_in argument is > correct, so I do not really understand why the argument type needs to be changed... Hm, I suppose the better patch would be to check the return of get_rate and set_rate when we call them. I think my intent here was to allow subsequent functions to be no-ops in case of error, but it looks like I act on these values directly. Will fix in v2. --Sean > >> { >> u64 r, f, od; >> u32 reg = readl(priv->base + k210_plls[id].off); >> > >
diff --git a/drivers/clk/clk_kendryte.c b/drivers/clk/clk_kendryte.c index 3148756968..37bd624eca 100644 --- a/drivers/clk/clk_kendryte.c +++ b/drivers/clk/clk_kendryte.c @@ -439,7 +439,7 @@ static const struct k210_clk_params k210_clks[] = { #ifdef CONFIG_CLK_K210_SET_RATE static int k210_pll_enable(struct k210_clk_priv *priv, int id); static int k210_pll_disable(struct k210_clk_priv *priv, int id); -static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, ulong rate_in); +static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, long rate_in); /* * The PLL included with the Kendryte K210 appears to be a True Circuits, Inc. @@ -841,7 +841,7 @@ TEST_STATIC int k210_pll_calc_config(u32 rate, u32 rate_in, } static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, - ulong rate_in) + long rate_in) { int err; const struct k210_pll_params *pll = &k210_plls[id]; @@ -890,7 +890,7 @@ static ulong k210_pll_set_rate(struct k210_clk_priv *priv, int id, ulong rate, #endif /* CONFIG_CLK_K210_SET_RATE */ static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int id, - ulong rate_in) + long rate_in) { u64 r, f, od; u32 reg = readl(priv->base + k210_plls[id].off);
Some clock functions return ulong but still have "negative" errors. To deal with this, cast the relevant arguments to long. Fixes: 609bd60b94 ("clk: k210: Rewrite to remove CCF") Reported-by: Coverity Scan <scan-admin@coverity.com> Signed-off-by: Sean Anderson <seanga2@gmail.com> --- drivers/clk/clk_kendryte.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)