clk: cs2000: Fix the setting of saved_rate during the resume process

Commit Message

Yoshihiro Kaneko April 19, 2017, 5:46 p.m. UTC

From: Gaku Inami <[e-mail]>

In the resume process, there is the case that other drivers call
cs2000_enable before cs2000_resume is called. Since the order of
resume process is not guaranteed, it is needed to reset the clk
rate in any cases before cs2000 is used.

Also, the current cs2000 driver is using resume_early function.
The using .resume_early is bad idea because the dependency with
other drivers is to be complicated.

This patch adds to reset the clk rate in cs2000_enable and
changes to use from .resume_early to .resume.

Signed-off-by: Gaku Inami <[e-mail]>
Signed-off-by: Hiroyuki Yokoyama <[e-mail]>
Signed-off-by: Yoshihiro Kaneko <[e-mail]>
---
This patch is based on the clk-next branch of linux-clk tree.

 drivers/clk/clk-cs2000-cp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Kuninori Morimoto April 19, 2017, 11:57 p.m. UTC | #1

Hi

> From: Gaku Inami <[e-mail]>
> 
> In the resume process, there is the case that other drivers call
> cs2000_enable before cs2000_resume is called. Since the order of
> resume process is not guaranteed, it is needed to reset the clk
> rate in any cases before cs2000 is used.
> 
> Also, the current cs2000 driver is using resume_early function.
> The using .resume_early is bad idea because the dependency with
> other drivers is to be complicated.
> 
> This patch adds to reset the clk rate in cs2000_enable and
> changes to use from .resume_early to .resume.
> 
> Signed-off-by: Gaku Inami <[e-mail]>
> Signed-off-by: Hiroyuki Yokoyama <[e-mail]>
> Signed-off-by: Yoshihiro Kaneko <[e-mail]>
> ---

Acked-by: Kuninori Morimoto <[e-mail]>

Stephen Boyd April 22, 2017, 2:10 a.m. UTC | #2

On 04/20, Yoshihiro Kaneko wrote:
> From: Gaku Inami <[e-mail]>
> 
> In the resume process, there is the case that other drivers call
> cs2000_enable before cs2000_resume is called. Since the order of
> resume process is not guaranteed, it is needed to reset the clk
> rate in any cases before cs2000 is used.
> 
> Also, the current cs2000 driver is using resume_early function.
> The using .resume_early is bad idea because the dependency with
> other drivers is to be complicated.
> 
> This patch adds to reset the clk rate in cs2000_enable and
> changes to use from .resume_early to .resume.
> 
> Signed-off-by: Gaku Inami <[e-mail]>
> Signed-off-by: Hiroyuki Yokoyama <[e-mail]>
> Signed-off-by: Yoshihiro Kaneko <[e-mail]>
> ---

Was this supposed to be sent as a series? This is the second
patch in the series? 

In what situation does a driver call clk_prepare() on the clocks
in here before the device's resume callback is called? I would
think that any devices that use the clks provided by this driver
would be probe defered until the clks are registered, so they
would be later in the suspend/resume list than this clk driver?

Geert Uytterhoeven April 23, 2017, 8:16 p.m. UTC | #3

Hi Stephen,

On Sat, Apr 22, 2017 at 4:10 AM, Stephen Boyd <[e-mail]> wrote:
> On 04/20, Yoshihiro Kaneko wrote:
>> From: Gaku Inami <[e-mail]>
>>
>> In the resume process, there is the case that other drivers call
>> cs2000_enable before cs2000_resume is called. Since the order of
>> resume process is not guaranteed, it is needed to reset the clk
>> rate in any cases before cs2000 is used.
>>
>> Also, the current cs2000 driver is using resume_early function.
>> The using .resume_early is bad idea because the dependency with
>> other drivers is to be complicated.
>>
>> This patch adds to reset the clk rate in cs2000_enable and
>> changes to use from .resume_early to .resume.
>>
>> Signed-off-by: Gaku Inami <[e-mail]>
>> Signed-off-by: Hiroyuki Yokoyama <[e-mail]>
>> Signed-off-by: Yoshihiro Kaneko <[e-mail]>
>> ---
>
> Was this supposed to be sent as a series? This is the second
> patch in the series?
>
> In what situation does a driver call clk_prepare() on the clocks
> in here before the device's resume callback is called? I would
> think that any devices that use the clks provided by this driver
> would be probe defered until the clks are registered, so they
> would be later in the suspend/resume list than this clk driver?

Deferred probing is used during driver initialization only.
This patch is about clock users calling into the clock API during the resume
procedure, before the clock provider has been resumed.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

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

Stephen Boyd April 24, 2017, 5:53 p.m. UTC | #4

On 04/23, Geert Uytterhoeven wrote:
> Hi Stephen,
> 
> On Sat, Apr 22, 2017 at 4:10 AM, Stephen Boyd <[e-mail]> wrote:
> > On 04/20, Yoshihiro Kaneko wrote:
> >> From: Gaku Inami <[e-mail]>
> >>
> >> In the resume process, there is the case that other drivers call
> >> cs2000_enable before cs2000_resume is called. Since the order of
> >> resume process is not guaranteed, it is needed to reset the clk
> >> rate in any cases before cs2000 is used.
> >>
> >> Also, the current cs2000 driver is using resume_early function.
> >> The using .resume_early is bad idea because the dependency with
> >> other drivers is to be complicated.
> >>
> >> This patch adds to reset the clk rate in cs2000_enable and
> >> changes to use from .resume_early to .resume.
> >>
> >> Signed-off-by: Gaku Inami <[e-mail]>
> >> Signed-off-by: Hiroyuki Yokoyama <[e-mail]>
> >> Signed-off-by: Yoshihiro Kaneko <[e-mail]>
> >> ---
> >
> > Was this supposed to be sent as a series? This is the second
> > patch in the series?
> >
> > In what situation does a driver call clk_prepare() on the clocks
> > in here before the device's resume callback is called? I would
> > think that any devices that use the clks provided by this driver
> > would be probe defered until the clks are registered, so they
> > would be later in the suspend/resume list than this clk driver?
> 
> Deferred probing is used during driver initialization only.
> This patch is about clock users calling into the clock API during the resume
> procedure, before the clock provider has been resumed.
> 

Right, and which driver is calling into the clk API in their
resume function before the clk provider is resume? Presumably the
clk provider had probed "first", or at least was ordered in the
dpm_list before the consumer drivers so that this problem
wouldn't happen.

Patch

diff --git a/drivers/clk/clk-cs2000-cp.c b/drivers/clk/clk-cs2000-cp.c
index 5b3516d..de6e8c6 100644
--- a/drivers/clk/clk-cs2000-cp.c
+++ b/drivers/clk/clk-cs2000-cp.c
@@ -338,6 +338,10 @@  static int cs2000_enable(struct clk_hw *hw)
 	struct cs2000_priv *priv = hw_to_priv(hw);
 	int ret;
 
+	ret = cs2000_set_saved_rate(priv);
+	if (ret < 0)
+		return ret;
+
 	ret = cs2000_enable_dev_config(priv, true);
 	if (ret < 0)
 		return ret;
@@ -530,7 +534,7 @@  static int cs2000_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops cs2000_pm_ops = {
-	.resume_early	= cs2000_resume,
+	.resume		= cs2000_resume,
 };
 
 static struct i2c_driver cs2000_driver = {