Skip to contentSkip to conversation

clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

PATCH New Shawn Guo 4 participants 5 messages Oct 8, 2016 – Oct 17, 2016 Common clk

lore
Share & tools
lore

Shawn Guo

author
#0 lore raw

PATCH 1/1 clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but also reset controller. It worked fine that single sysctrl/mediactrl device node in DT can be used to initialize clock driver and populate platform device for reset controller. But it stops working after commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks") gets merged. The commit sets flag OF_POPULATED during clock initialization to skip the platform device populating for the same device node. On hi6220, it effectively makes hi6220-sysctrl reset driver not probe any more.

The patch changes hi6220 sysctrl and mediactrl clock init macro from CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using the same hardware block can continue working.

Signed-off-by Shawn Guo <[email protected]>

---
It fixes an issue that is seen on linux-next, i.e. the new added hi6220-sysctrl reset driver doesn't probe at all, and consequently the mmc driver fails to register.

Shawn

 drivers/clk/hisilicon/clk-hi6220.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/clk/hisilicon/clk-hi6220.c +2 -2
index fe364e63f8de..c0e8e1f196aa 100644
@@ -195,7 +195,7 @@ static void __init hi6220_clk_sys_init(struct device_node *np)
 	hi6220_clk_register_divider(hi6220_div_clks_sys,
 			ARRAY_SIZE(hi6220_div_clks_sys), clk_data);
 }
-CLK_OF_DECLARE(hi6220_clk_sys, "hisilicon,hi6220-sysctrl", hi6220_clk_sys_init);
+CLK_OF_DECLARE_DRIVER(hi6220_clk_sys, "hisilicon,hi6220-sysctrl", hi6220_clk_sys_init);
 
 
 /* clocks in media controller */

@@ -252,7 +252,7 @@ static void __init hi6220_clk_media_init(struct device_node *np)
 	hi6220_clk_register_divider(hi6220_div_clks_media,
 				ARRAY_SIZE(hi6220_div_clks_media), clk_data);
 }
-CLK_OF_DECLARE(hi6220_clk_media, "hisilicon,hi6220-mediactrl", hi6220_clk_media_init);
+CLK_OF_DECLARE_DRIVER(hi6220_clk_media, "hisilicon,hi6220-mediactrl", hi6220_clk_media_init);
 
 
 /* clocks in pmctrl */
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

Quoted 14 lines · Shawn Guo

The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but also reset controller. It worked fine that single sysctrl/mediactrl device node in DT can be used to initialize clock driver and populate platform device for reset controller. But it stops working after commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks") gets merged. The commit sets flag OF_POPULATED during clock initialization to skip the platform device populating for the same device node. On hi6220, it effectively makes hi6220-sysctrl reset driver not probe any more.

The patch changes hi6220 sysctrl and mediactrl clock init macro from CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using the same hardware block can continue working.

Quoted 5 lines
Signed-off-by Shawn Guo <[email protected]>

---
It fixes an issue that is seen on linux-next, i.e. the new added hi6220-sysctrl reset driver doesn't probe at all, and consequently the mmc driver fails to register.

Correction: the hi6220-sysctrl has been there for a while, and the issue is discovered by mmc driver which adds reset support recently. So technically, this is a regression fix.

Shawn

#2 lore raw

PATCH 1/1 clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

In reply to Shawn Guo
Quoted 10 lines · Shawn Guo

The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but also reset controller. It worked fine that single sysctrl/mediactrl device node in DT can be used to initialize clock driver and populate platform device for reset controller. But it stops working after commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks") gets merged. The commit sets flag OF_POPULATED during clock initialization to skip the platform device populating for the same device node. On hi6220, it effectively makes hi6220-sysctrl reset driver not probe any more.

Quoted 5 lines

The patch changes hi6220 sysctrl and mediactrl clock init macro from CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using the same hardware block can continue working.

Signed-off-by Shawn Guo <[email protected]>
Tested-by John Stultz <[email protected]>

I hit this as well last week when 989eafd0b609 ("clk: core: Avoid double initialization of clocks") landed, which killed graphics on my HiKey.

My workaround was a bit hackish, as I don't really know when one should use OF_DECLARE vs OF_DECLARE_DRIVER, but I also converted the hi6220_clk_ao and hi6220_clk_power to the _DRIVER side. Your patch seems to work just as well for me, but I wanted to double check with you that the ao/power clks didn't need the conversion as well.

thanks
-john

#3 lore raw

PATCH 1/1 clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

Hi John,

Quoted 23 lines · John Stultz
Quoted 10 lines · Shawn Guo

The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but also reset controller. It worked fine that single sysctrl/mediactrl device node in DT can be used to initialize clock driver and populate platform device for reset controller. But it stops working after commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks") gets merged. The commit sets flag OF_POPULATED during clock initialization to skip the platform device populating for the same device node. On hi6220, it effectively makes hi6220-sysctrl reset driver not probe any more.

Quoted 5 lines

The patch changes hi6220 sysctrl and mediactrl clock init macro from CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using the same hardware block can continue working.

Signed-off-by Shawn Guo <[email protected]>
Tested-by John Stultz <[email protected]>

I hit this as well last week when 989eafd0b609 ("clk: core: Avoid double initialization of clocks") landed, which killed graphics on my HiKey.

Quoted 5 lines

My workaround was a bit hackish, as I don't really know when one should use OF_DECLARE vs OF_DECLARE_DRIVER, but I also converted the hi6220_clk_ao and hi6220_clk_power to the _DRIVER side. Your patch seems to work just as well for me, but I wanted to double check with you that the ao/power clks didn't need the conversion as well.

For now, clock driver is the only one matching compatible "hisilicon,hi6220-aoctrl" and "hisilicon,hi6220-pmctrl". Whoever adding a platform driver probing the same compatible later will have to change it CLK_OF_DECLARE_DRIVER. Otherwise, the platform driver simply doesn't probe.

Shawn

Applied
#4 lore raw

PATCH 1/1 clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

In reply to Shawn Guo
Quoted 10 lines · Shawn Guo

The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but also reset controller. It worked fine that single sysctrl/mediactrl device node in DT can be used to initialize clock driver and populate platform device for reset controller. But it stops working after commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks") gets merged. The commit sets flag OF_POPULATED during clock initialization to skip the platform device populating for the same device node. On hi6220, it effectively makes hi6220-sysctrl reset driver not probe any more.

Quoted 6 lines

The patch changes hi6220 sysctrl and mediactrl clock init macro from CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using the same hardware block can continue working.

Signed-off-by Shawn Guo <[email protected]>

---

Applied to clk-fixes

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project