Skip to contentSkip to conversation

clk: vc5: Initialize src in vc5_mux_set_parent()

PATCH In review Stephen Boyd 3 participants 3 messages Dec 14, 2018 – Dec 16, 2018

Subsystems (2)
lore
Share & tools
lore

Stephen Boyd

author maintainer
#0 lore raw

PATCH 1/1 clk: vc5: Initialize src in vc5_mux_set_parent()

It seems that it may be possible to get to the regmap update call without ever initializing this variable, so just set it to 0 to be safe.

Cc Marek Vasut <[email protected]>
Cc Alexey Firago <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 drivers/clk/clk-versaclock5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/clk-versaclock5.c +1 -1
index 5b393e711e94..026f76276933 100644
@@ -246,7 +246,7 @@ static int vc5_mux_set_parent(struct clk_hw *hw, u8 index)
 	struct vc5_driver_data *vc5 =
 		container_of(hw, struct vc5_driver_data, clk_mux);
 	const u8 mask = VC5_PRIM_SRC_SHDN_EN_XTAL | VC5_PRIM_SRC_SHDN_EN_CLKIN;
-	u8 src;
+	u8 src = 0;
 
 	if ((index > 1) || !vc5->clk_mux_ins)
 		return -EINVAL;
Signature · 1 line
Sent by a computer through tubes
#1 lore raw
Quoted 17 lines · Stephen Boyd

It seems that it may be possible to get to the regmap update call without ever initializing this variable, so just set it to 0 to be safe.

Cc Marek Vasut <[email protected]>
Cc Alexey Firago <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 drivers/clk/clk-versaclock5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/clk-versaclock5.c +0 -0
index 5b393e711e94..026f76276933 100644

drivers/clk/clk-versaclock5.c static int vc5_mux_set_parent(struct clk_hw *hw, u8 index) · line 252

Quoted 8 lines
drivers/clk/clk-versaclock5.c +1 -1
@@ -246,7 +246,7 @@ static int vc5_mux_set_parent(struct clk_hw *hw, u8 index)
 	struct vc5_driver_data *vc5 =
 		container_of(hw, struct vc5_driver_data, clk_mux);
 	const u8 mask = VC5_PRIM_SRC_SHDN_EN_XTAL | VC5_PRIM_SRC_SHDN_EN_CLKIN;
-	u8 src;
+	u8 src = 0;
 
 	if ((index > 1) || !vc5->clk_mux_ins)
 		return -EINVAL;

Can you rather change the else { } branch in that function such that it does

if (vc5->clk_mux)ins == ...)
...
else if (...)
...
+ else
+ return -EINVAL;

The last bit would assure the function would fail in case the clk_mux_ins == 0, which can happen if someone builds a setup without any clock into connected to the VC5 (which would not work hardware-vise).

Signature · 2 lines
Best regards,
Marek Vasut
#2 lore raw
In reply to Stephen Boyd

Hi Stephen,

Quoted 2 lines · On Friday, 14 December 2018 23:49:59 EET Stephen Boyd wrote:

It seems that it may be possible to get to the regmap update call without ever initializing this variable, so just set it to 0 to be safe.

This can't happen as the probe() function would have failed first. I'd thus add a comment to explain that the goal is only to silence a compiler warning.

Quoted 22 lines
Cc Marek Vasut <[email protected]>
Cc Alexey Firago <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 drivers/clk/clk-versaclock5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/clk-versaclock5.c +0 -0
index 5b393e711e94..026f76276933 100644
@@ -246,7 +246,7 @@ static int vc5_mux_set_parent(struct clk_hw *hw, u8

index) struct vc5_driver_data *vc5 =

drivers/clk/clk-versaclock5.c +1 -1
 		container_of(hw, struct vc5_driver_data, clk_mux);
 	const u8 mask = VC5_PRIM_SRC_SHDN_EN_XTAL | VC5_PRIM_SRC_SHDN_EN_CLKIN;
-	u8 src;
+	u8 src = 0;
 
 	if ((index > 1) || !vc5->clk_mux_ins)
 		return -EINVAL;
Signature · 3 lines
Regards,

Laurent Pinchart