[bug report] clk: vc5: Add support for IDT VersaClock 5P49V5923 and 5P49V5933

[bug report] clk: vc5: Add support for IDT VersaClock 5P49V5923 and 5P49V5933

[Dan Carpenter]

Hello Marek Vasut,

The patch 3e1aec4e2c41: "clk: vc5: Add support for IDT VersaClock
5P49V5923 and 5P49V5933" from Jan 12, 2017, leads to the following
static checker warning:

	drivers/clk/clk-versaclock5.c:235 vc5_mux_set_parent()
	error: uninitialized symbol 'src'.

drivers/clk/clk-versaclock5.c
   210  static int vc5_mux_set_parent(struct clk_hw *hw, u8 index)
   211  {
   212          struct vc5_driver_data *vc5 =
   213                  container_of(hw, struct vc5_driver_data, clk_mux);
   214          const u8 mask = VC5_PRIM_SRC_SHDN_EN_XTAL | VC5_PRIM_SRC_SHDN_EN_CLKIN;
   215          u8 src;
                   ^^^
   216  
   217          if ((index > 1) || !vc5->clk_mux_ins)
   218                  return -EINVAL;
   219  
   220          if (vc5->clk_mux_ins == (VC5_MUX_IN_CLKIN | VC5_MUX_IN_XIN)) {
   221                  if (index == 0)
   222                          src = VC5_PRIM_SRC_SHDN_EN_XTAL;
   223                  if (index == 1)
   224                          src = VC5_PRIM_SRC_SHDN_EN_CLKIN;
   225          } else {
   226                  if (index != 0)
   227                          return -EINVAL;
   228  
   229                  if (vc5->clk_mux_ins == VC5_MUX_IN_XIN)
   230                          src = VC5_PRIM_SRC_SHDN_EN_XTAL;
   231                  if (vc5->clk_mux_ins == VC5_MUX_IN_CLKIN)
   232                          src = VC5_PRIM_SRC_SHDN_EN_CLKIN;

What about if both of these are false?

   233          }
   234  
   235          return regmap_update_bits(vc5->regmap, VC5_PRIM_SRC_SHDN, mask, src);
                                                                                ^^^
Potentially uninitialized.

   236  }

regards,
dan carpenter

Re: [bug report] clk: vc5: Add support for IDT VersaClock 5P49V5923 and 5P49V5933

[Geert Uytterhoeven]

Hi Dan,

On Tue, Jan 24, 2017 at 10:31 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> The patch 3e1aec4e2c41: "clk: vc5: Add support for IDT VersaClock
> 5P49V5923 and 5P49V5933" from Jan 12, 2017, leads to the following
> static checker warning:
>
>         drivers/clk/clk-versaclock5.c:235 vc5_mux_set_parent()
>         error: uninitialized symbol 'src'.
>
> drivers/clk/clk-versaclock5.c
>    210  static int vc5_mux_set_parent(struct clk_hw *hw, u8 index)
>    211  {
>    212          struct vc5_driver_data *vc5 =
>    213                  container_of(hw, struct vc5_driver_data, clk_mux);
>    214          const u8 mask = VC5_PRIM_SRC_SHDN_EN_XTAL | VC5_PRIM_SRC_SHDN_EN_CLKIN;
>    215          u8 src;
>                    ^^^
>    216
>    217          if ((index > 1) || !vc5->clk_mux_ins)
>    218                  return -EINVAL;
>    219
>    220          if (vc5->clk_mux_ins == (VC5_MUX_IN_CLKIN | VC5_MUX_IN_XIN)) {
>    221                  if (index == 0)
>    222                          src = VC5_PRIM_SRC_SHDN_EN_XTAL;
>    223                  if (index == 1)
>    224                          src = VC5_PRIM_SRC_SHDN_EN_CLKIN;
>    225          } else {
>    226                  if (index != 0)
>    227                          return -EINVAL;
>    228
>    229                  if (vc5->clk_mux_ins == VC5_MUX_IN_XIN)
>    230                          src = VC5_PRIM_SRC_SHDN_EN_XTAL;
>    231                  if (vc5->clk_mux_ins == VC5_MUX_IN_CLKIN)
>    232                          src = VC5_PRIM_SRC_SHDN_EN_CLKIN;
>
> What about if both of these are false?

That cannot happen, cfr. the initialization of clk_mux_ins.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [bug report] clk: vc5: Add support for IDT VersaClock 5P49V5923 and 5P49V5933

[Dan Carpenter]

Thanks.

I've figured out a way to fix Smatch so it doesn't complain about this
particular warning in the future.

regards,
dan carpenter