| Message ID | 1455819551-4666-4-git-send-email-javier@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19.02.2016 03:19, Javier Martinez Canillas wrote: > The driver doesn't check if the regulator_get_optional return value is > -EPROBE_DEFER so it will wrongly assume that the regulator couldn't be > found just because the regulator driver wasn't registered yet, i.e: > > exynos-tmu 10060000.tmu: Regulator node (vtmu) not found > > In this case the return value should be propagated to allow the driver > probe function to be deferred until the regulator driver is registered. > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > > --- > > drivers/thermal/samsung/exynos_tmu.c | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof
Hi Javier, Just a question... > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index f4f36bba7be9..f3ce94ec73b5 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -1318,6 +1318,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) > return ret; > } > } else { > + if (PTR_ERR(data->regulator) == -EPROBE_DEFER) > + return -EPROBE_DEFER; shouldn't we return also in case of -ENOMEM? -ENOMEM is a Kernel failure, not depending on the regulator itself. Andi > dev_info(&pdev->dev, "Regulator node (vtmu) not found\n"); > }
On 19.02.2016 13:20, Andi Shyti wrote: > Hi Javier, > > Just a question... > >> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c >> index f4f36bba7be9..f3ce94ec73b5 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> @@ -1318,6 +1318,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> return ret; >> } >> } else { >> + if (PTR_ERR(data->regulator) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; > > shouldn't we return also in case of -ENOMEM? -ENOMEM is a Kernel > failure, not depending on the regulator itself. Usually not because that would make this error path quite complicated and difficult to keep consistent. If you choose this way then you will have to probably extend the black list (e.g. EINVAL, some other)... or use a white list. Additionally the error codes can come from deeper layers. These layer now can return ESOMETHING but later it might change to EDIFFERENT... you cannot predict that. Whatever the reason was (except defer), just ignore the regulator. Best regards, Krzysztof
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index f4f36bba7be9..f3ce94ec73b5 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -1318,6 +1318,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) return ret; } } else { + if (PTR_ERR(data->regulator) == -EPROBE_DEFER) + return -EPROBE_DEFER; dev_info(&pdev->dev, "Regulator node (vtmu) not found\n"); }
The driver doesn't check if the regulator_get_optional return value is -EPROBE_DEFER so it will wrongly assume that the regulator couldn't be found just because the regulator driver wasn't registered yet, i.e: exynos-tmu 10060000.tmu: Regulator node (vtmu) not found In this case the return value should be propagated to allow the driver probe function to be deferred until the regulator driver is registered. Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- drivers/thermal/samsung/exynos_tmu.c | 2 ++ 1 file changed, 2 insertions(+)