Hi Biju,
On Sun, Jun 25, 2023 at 8:09 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> Change the scope of the variables {clkin_name, xin_name} from global->local
> to fix the below warning.
>
> drivers/regulator/raa215300.c:42:12: sparse: sparse: symbol 'xin_name' was
> not declared. Should it be static?
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-kbuild-all/202306250552.Fan9WTiN-lkp@xxxxxxxxx/
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Thanks for your patch, which is correct, so:
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> --- a/drivers/regulator/raa215300.c
> +++ b/drivers/regulator/raa215300.c
> @@ -38,8 +38,6 @@
> #define RAA215300_REG_BLOCK_EN_RTC_EN BIT(6)
> #define RAA215300_RTC_DEFAULT_ADDR 0x6f
>
> -const char *clkin_name = "clkin";
> -const char *xin_name = "xin";
> static struct clk *clk;
>
> static const struct regmap_config raa215300_regmap_config = {
> @@ -71,9 +69,11 @@ static int raa215300_clk_present(struct i2c_client *client, const char *name)
> static int raa215300_i2c_probe(struct i2c_client *client)
> {
> struct device *dev = &client->dev;
> - const char *clk_name = xin_name;
> + const char *clkin_name = "clkin";
> unsigned int pmic_version, val;
> + const char *xin_name = "xin";
> struct regmap *regmap;
> + const char *clk_name;
> int ret;
>
> regmap = devm_regmap_init_i2c(client, &raa215300_regmap_config);
> @@ -120,6 +120,8 @@ static int raa215300_i2c_probe(struct i2c_client *client)
> return ret;
>
> clk_name = clkin_name;
> + } else {
> + clk_name = xin_name;
I'd rather invert the second if-condition and exchange the two branches,
to make the code flow easier to follow for the casual reader.
ret = raa215300_clk_present(client, xin_name);
if (ret < 0) {
return ret;
} else if (ret) {
clk_name = xin_name;
} else {
ret = raa215300_clk_present(client, clkin_name);
if (ret < 0)
return ret;
clk_name = clkin_name;
}
> }
>
> if (ret) {
Not introduced by this patch: the check above really checks if there is
an external clock present. A casual reader might not notice that detail,
and add more code in between the assignment to ret and the check.
So it might be prudent to pre-initialize clk_name to NULL, and set it
to clkin_name only if ret > 0. Then the above check can become a check
for clk_name.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
