Patchwork [v2,3/4] cpufreq: s5pv210: Defer probe if getting regulators fail

login
register
mail settings
Submitter Paweł Chmiel
Date Jan. 10, 2019, 8:52 p.m.
Message ID <20190110205215.22030-4-pawel.mikolaj.chmiel@gmail.com>
Download mbox | patch
Permalink /patch/697261/
State New
Headers show

Comments

Paweł Chmiel - Jan. 10, 2019, 8:52 p.m.
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@gmail.com>
---
Changes from v1:
  - Fix compilation error
  - Reorganize code so it's smaller
---
 drivers/cpufreq/s5pv210-cpufreq.c | 32 ++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)
Krzysztof Kozlowski - Jan. 11, 2019, 9:46 a.m.
On Thu, 10 Jan 2019 at 21:53, Paweł Chmiel
<pawel.mikolaj.chmiel@gmail.com> 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@gmail.com>
> ---
> Changes from v1:
>   - Fix compilation error
>   - Reorganize code so it's smaller
> ---
>  drivers/cpufreq/s5pv210-cpufreq.c | 32 ++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
> index f51697f1e0b3..6df95941ba96 100644
> --- a/drivers/cpufreq/s5pv210-cpufreq.c
> +++ b/drivers/cpufreq/s5pv210-cpufreq.c
> @@ -594,6 +594,25 @@ 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 (IS_ERR(arm_regulator)) {
> +               if (PTR_ERR(arm_regulator) == -EPROBE_DEFER)
> +                       pr_debug("vddarm regulator not ready, defer\n");
> +               else
> +                       pr_err("failed to get regulator vddarm\n");
> +               return PTR_ERR(arm_regulator);
> +       }
> +
> +       int_regulator = regulator_get(NULL, "vddint");
> +       if (IS_ERR(int_regulator)) {
> +               if (PTR_ERR(int_regulator) == -EPROBE_DEFER)
> +                       pr_debug("vddint regulator not ready, defer\n");
> +               else
> +                       pr_err("failed to get regulator vddint\n");
> +               regulator_put(arm_regulator);
> +               return PTR_ERR(int_regulator);
> +       }
> +

Error paths are now not correct. You should put the regulators when
returning later. The previous code had these error paths wrong as well
- not iounmapping - but if you move things around, maybe let's fix
things.

Best regards,
Krzysztof
Paweł Chmiel - Jan. 11, 2019, 9:53 a.m.
pt., 11 sty 2019 o 10:46 Krzysztof Kozlowski <krzk@kernel.org> napisał(a):
>
> On Thu, 10 Jan 2019 at 21:53, Paweł Chmiel
> <pawel.mikolaj.chmiel@gmail.com> 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@gmail.com>
> > ---
> > Changes from v1:
> >   - Fix compilation error
> >   - Reorganize code so it's smaller
> > ---
> >  drivers/cpufreq/s5pv210-cpufreq.c | 32 ++++++++++++++++++-------------
> >  1 file changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
> > index f51697f1e0b3..6df95941ba96 100644
> > --- a/drivers/cpufreq/s5pv210-cpufreq.c
> > +++ b/drivers/cpufreq/s5pv210-cpufreq.c
> > @@ -594,6 +594,25 @@ 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 (IS_ERR(arm_regulator)) {
> > +               if (PTR_ERR(arm_regulator) == -EPROBE_DEFER)
> > +                       pr_debug("vddarm regulator not ready, defer\n");
> > +               else
> > +                       pr_err("failed to get regulator vddarm\n");
> > +               return PTR_ERR(arm_regulator);
> > +       }
> > +
> > +       int_regulator = regulator_get(NULL, "vddint");
> > +       if (IS_ERR(int_regulator)) {
> > +               if (PTR_ERR(int_regulator) == -EPROBE_DEFER)
> > +                       pr_debug("vddint regulator not ready, defer\n");
> > +               else
> > +                       pr_err("failed to get regulator vddint\n");
> > +               regulator_put(arm_regulator);
> > +               return PTR_ERR(int_regulator);
> > +       }
> > +
>
> Error paths are now not correct. You should put the regulators when
> returning later. The previous code had these error paths wrong as well
> - not iounmapping - but if you move things around, maybe let's fix
> things.
Ok, will fix this in new version and will resend it (only this one
patch, since all other looks ok/applied).
Also will add missing iounmapping/etc to fix all other error paths.

Thanks
>
> Best regards,
> Krzysztof

Patch

diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
index f51697f1e0b3..6df95941ba96 100644
--- a/drivers/cpufreq/s5pv210-cpufreq.c
+++ b/drivers/cpufreq/s5pv210-cpufreq.c
@@ -594,6 +594,25 @@  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 (IS_ERR(arm_regulator)) {
+		if (PTR_ERR(arm_regulator) == -EPROBE_DEFER)
+			pr_debug("vddarm regulator not ready, defer\n");
+		else
+			pr_err("failed to get regulator vddarm\n");
+		return PTR_ERR(arm_regulator);
+	}
+
+	int_regulator = regulator_get(NULL, "vddint");
+	if (IS_ERR(int_regulator)) {
+		if (PTR_ERR(int_regulator) == -EPROBE_DEFER)
+			pr_debug("vddint regulator not ready, defer\n");
+		else
+			pr_err("failed to get regulator vddint\n");
+		regulator_put(arm_regulator);
+		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 +652,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);