[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[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.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
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(-)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index fe364e63f8de..c0e8e1f196aa 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c
@@ -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 */
-- 
1.9.1

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[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.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
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(-)

diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c
index fe364e63f8de..c0e8e1f196aa 100644
--- a/drivers/clk/hisilicon/clk-hi6220.c
+++ b/drivers/clk/hisilicon/clk-hi6220.c
@@ -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 */
-- 
1.9.1

Re: [PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Shawn Guo]

On Sat, Oct 08, 2016 at 09:38:12PM +0800, Shawn Guo wrote:
> 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 <shawn.guo@linaro.org>
> ---
> 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

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Shawn Guo]

On Sat, Oct 08, 2016 at 09:38:12PM +0800, Shawn Guo wrote:
> 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 <shawn.guo@linaro.org>
> ---
> 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

Re: [PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[John Stultz]

On Sat, Oct 8, 2016 at 6:38 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 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 <shawn.guo@linaro.org>

Tested-by: John Stultz <john.stultz@linaro.org>

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

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[John Stultz]

On Sat, Oct 8, 2016 at 6:38 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 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 <shawn.guo@linaro.org>

Tested-by: John Stultz <john.stultz@linaro.org>

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

Re: [PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Shawn Guo]

Hi John,

On Mon, Oct 10, 2016 at 10:39:12AM -0700, John Stultz wrote:
> On Sat, Oct 8, 2016 at 6:38 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > 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 <shawn.guo@linaro.org>
> 
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> 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.

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

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Shawn Guo]

Hi John,

On Mon, Oct 10, 2016 at 10:39:12AM -0700, John Stultz wrote:
> On Sat, Oct 8, 2016 at 6:38 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > 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 <shawn.guo@linaro.org>
> 
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> 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.

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

Re: [PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Stephen Boyd]

On 10/08, Shawn Guo wrote:
> 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 <shawn.guo@linaro.org>
> ---

Applied to clk-fixes

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init

[Stephen Boyd]

On 10/08, Shawn Guo wrote:
> 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 <shawn.guo@linaro.org>
> ---

Applied to clk-fixes

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project