czw., 10 sty 2019 o 08:01 Viresh Kumar <viresh.kumar@xxxxxxxxxx> napisał(a):
>
> On 08-01-19, 21:05, Paweł Chmiel wrote:
> > There is possibility, that when probing driver, regulators are not yet
> > initialized. In this case we should return EPROBE_DEFER and wait till
> > they're initialized, since they're required currently for cpufreq driver
> > to work. Also move regulator initialization code at beginning of probe,
> > so we can defer as fast as posibble.
> >
> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@xxxxxxxxx>
> > ---
> > drivers/cpufreq/s5pv210-cpufreq.c | 33 +++++++++++++++++++------------
> > 1 file changed, 20 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
> > index f51697f1e0b3..2d0e4dc7ede7 100644
> > --- a/drivers/cpufreq/s5pv210-cpufreq.c
> > +++ b/drivers/cpufreq/s5pv210-cpufreq.c
> > @@ -594,6 +594,26 @@ static int s5pv210_cpufreq_probe(struct platform_device *pdev)
> > * this whole driver as soon as S5PV210 gets migrated to use
> > * cpufreq-dt driver.
> > */
> > + arm_regulator = regulator_get(NULL, "vddarm");
> > + if (PTR_ERR(arm_regulator) == -EPROBE_DEFER) {
> > + pr_dbg("vddarm regulator not ready, defer\n");
> > + return -EPROBE_DEFER;
> > + } else if (IS_ERR(arm_regulator)) {
> > + pr_err("failed to get regulator vddarm\n");
> > + return PTR_ERR(arm_regulator);
> > + }
>
> The only difference between the two cases is pr_dbg vs pr_err, its
> ugly that we have to add special code for that :(
>
> Maybe write it as:
>
> if (IS_ERR(arm_regulator)) {
> if (PTR_ERR(arm_regulator) == -EPROBE_DEFER)
> pr_dbg...
> else
> pr_err
> return PTR_ERR(arm_regulator);
> }
>
> > +
> > + int_regulator = regulator_get(NULL, "vddint");
> > + if (PTR_ERR(int_regulator == -EPROBE_DEFER) {
>
> Does this even compile ?
Looks like i broke it after testing (where it was working fine - was
able to change freq and gov),
and didn't catch this before sending patch for review :/.
Sorry, this (and first issue) will be fixed in v2.
Thanks for review
>
> > + regulator_put(arm_regulator);
> > + pr_dbg("vddint regulator not ready, defer\n");
> > + return -EPROBE_DEFER;
> > + } else if (IS_ERR(int_regulator)) {
> > + regulator_put(arm_regulator);
> > + pr_err("failed to get regulator vddint\n");
> > + return PTR_ERR(int_regulator);
> > + }
> > +
> > np = of_find_compatible_node(NULL, NULL, "samsung,s5pv210-clock");
> > if (!np) {
> > pr_err("%s: failed to find clock controller DT node\n",
> > @@ -633,19 +653,6 @@ static int s5pv210_cpufreq_probe(struct platform_device *pdev)
> > }
> > }
> >
> > - arm_regulator = regulator_get(NULL, "vddarm");
> > - if (IS_ERR(arm_regulator)) {
> > - pr_err("failed to get regulator vddarm\n");
> > - return PTR_ERR(arm_regulator);
> > - }
> > -
> > - int_regulator = regulator_get(NULL, "vddint");
> > - if (IS_ERR(int_regulator)) {
> > - pr_err("failed to get regulator vddint\n");
> > - regulator_put(arm_regulator);
> > - return PTR_ERR(int_regulator);
> > - }
> > -
> > register_reboot_notifier(&s5pv210_cpufreq_reboot_notifier);
> >
> > return cpufreq_register_driver(&s5pv210_driver);
> > --
> > 2.17.1
>
> --
> viresh
