| Message ID | 20220808180915.446053-2-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] thermal/of: Fix error code in of_thermal_zone_find() | expand |
Am 2022-08-08 20:09, schrieb Daniel Lezcano: > The previous version of the OF code was returning -ENODEV if no > thermal zones description was found or if the lookup of the sensor in > the thermal zones was not found. > > The backend drivers are expecting this return value as an information > about skipping the sensor initialization and considered as normal. > > Fix the return value by replacing -EINVAL by -ENODEV > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Tested-by: Michael Walle <michael@walle.cc>
Am 2022-08-08 20:35, schrieb Guenter Roeck: > On 8/8/22 11:09, Daniel Lezcano wrote: >> The previous version of the OF code was returning -ENODEV if no >> thermal zones description was found or if the lookup of the sensor in >> the thermal zones was not found. >> >> The backend drivers are expecting this return value as an information >> about skipping the sensor initialization and considered as normal. >> >> Fix the return value by replacing -EINVAL by -ENODEV >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/thermal/thermal_of.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/thermal/thermal_of.c >> b/drivers/thermal/thermal_of.c >> index 368eb58e97cf..4210c18ef7b2 100644 >> --- a/drivers/thermal/thermal_of.c >> +++ b/drivers/thermal/thermal_of.c >> @@ -329,7 +329,7 @@ static struct device_node >> *of_thermal_zone_find(struct device_node *sensor, int >> np = of_find_node_by_name(NULL, "thermal-zones"); >> if (!np) { >> pr_err("Unable to find thermal zones description\n"); > > I really don't like that error message: People will see it (and > complain) > whenever a sensor registers and there is no thermal zone, even though > that > is perfectly normal (see description above). I can second that, and there actually two error messages: [ 6.156983] thermal_sys: Unable to find thermal zones description [ 6.163125] thermal_sys: Failed to find thermal zone for hwmon id=0 On the sl28 board with the qoriq_thermal driver: [ 1.917940] thermal_sys: Failed to find thermal zone for tmu id=2 [ 1.929231] thermal_sys: Failed to find thermal zone for tmu id=3 [ 1.940519] thermal_sys: Failed to find thermal zone for tmu id=4 [ 1.951814] thermal_sys: Failed to find thermal zone for tmu id=5 [ 1.963109] thermal_sys: Failed to find thermal zone for tmu id=6 [ 1.974399] thermal_sys: Failed to find thermal zone for tmu id=7 [ 1.985690] thermal_sys: Failed to find thermal zone for tmu id=8 [ 1.996980] thermal_sys: Failed to find thermal zone for tmu id=9 [ 2.008274] thermal_sys: Failed to find thermal zone for tmu id=10 [ 2.019656] thermal_sys: Failed to find thermal zone for tmu id=11 [ 2.031037] thermal_sys: Failed to find thermal zone for tmu id=12 [ 2.048942] thermal_sys: Failed to find thermal zone for tmu id=13 [ 2.060320] thermal_sys: Failed to find thermal zone for tmu id=14 [ 2.071700] thermal_sys: Failed to find thermal zone for tmu id=15 Btw. the driver seems to always register 16 sensors regardless how many the actual hardware has (or rather: are described in the DT). -michael
On 08/08/2022 20:35, Guenter Roeck wrote: > On 8/8/22 11:09, Daniel Lezcano wrote: >> The previous version of the OF code was returning -ENODEV if no >> thermal zones description was found or if the lookup of the sensor in >> the thermal zones was not found. >> >> The backend drivers are expecting this return value as an information >> about skipping the sensor initialization and considered as normal. >> >> Fix the return value by replacing -EINVAL by -ENODEV >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/thermal/thermal_of.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c >> index 368eb58e97cf..4210c18ef7b2 100644 >> --- a/drivers/thermal/thermal_of.c >> +++ b/drivers/thermal/thermal_of.c >> @@ -329,7 +329,7 @@ static struct device_node >> *of_thermal_zone_find(struct device_node *sensor, int >> np = of_find_node_by_name(NULL, "thermal-zones"); >> if (!np) { >> pr_err("Unable to find thermal zones description\n"); > > I really don't like that error message: People will see it (and complain) > whenever a sensor registers and there is no thermal zone, even though that > is perfectly normal (see description above). > I agree, I'll change the message to: pr_info("No thermal zones description\n"); sounds good ? >> - return ERR_PTR(-EINVAL); >> + return ERR_PTR(-ENODEV); >> } >> /* >> @@ -368,7 +368,7 @@ static struct device_node >> *of_thermal_zone_find(struct device_node *sensor, int >> } >> } >> } >> - tz = ERR_PTR(-EINVAL); >> + tz = ERR_PTR(-ENODEV); >> out: >> of_node_put(np); >> return tz; >
On 08/08/2022 21:05, Michael Walle wrote: > Am 2022-08-08 20:35, schrieb Guenter Roeck: >> On 8/8/22 11:09, Daniel Lezcano wrote: >>> The previous version of the OF code was returning -ENODEV if no >>> thermal zones description was found or if the lookup of the sensor in >>> the thermal zones was not found. >>> >>> The backend drivers are expecting this return value as an information >>> about skipping the sensor initialization and considered as normal. >>> >>> Fix the return value by replacing -EINVAL by -ENODEV >>> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >>> --- >>> drivers/thermal/thermal_of.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c >>> index 368eb58e97cf..4210c18ef7b2 100644 >>> --- a/drivers/thermal/thermal_of.c >>> +++ b/drivers/thermal/thermal_of.c >>> @@ -329,7 +329,7 @@ static struct device_node >>> *of_thermal_zone_find(struct device_node *sensor, int >>> np = of_find_node_by_name(NULL, "thermal-zones"); >>> if (!np) { >>> pr_err("Unable to find thermal zones description\n"); >> >> I really don't like that error message: People will see it (and complain) >> whenever a sensor registers and there is no thermal zone, even though >> that >> is perfectly normal (see description above). > > I can second that, and there actually two error messages: > > [ 6.156983] thermal_sys: Unable to find thermal zones description > [ 6.163125] thermal_sys: Failed to find thermal zone for hwmon id=0 Yeah, I can check: np = of_thermal_zone_find(sensor, id); And print the error if PTR_ERR(np) != ENODEV, otherwise silently return. > On the sl28 board with the qoriq_thermal driver: > [ 1.917940] thermal_sys: Failed to find thermal zone for tmu id=2 > [ 1.929231] thermal_sys: Failed to find thermal zone for tmu id=3 > [ 1.940519] thermal_sys: Failed to find thermal zone for tmu id=4 > [ 1.951814] thermal_sys: Failed to find thermal zone for tmu id=5 > [ 1.963109] thermal_sys: Failed to find thermal zone for tmu id=6 > [ 1.974399] thermal_sys: Failed to find thermal zone for tmu id=7 > [ 1.985690] thermal_sys: Failed to find thermal zone for tmu id=8 > [ 1.996980] thermal_sys: Failed to find thermal zone for tmu id=9 > [ 2.008274] thermal_sys: Failed to find thermal zone for tmu id=10 > [ 2.019656] thermal_sys: Failed to find thermal zone for tmu id=11 > [ 2.031037] thermal_sys: Failed to find thermal zone for tmu id=12 > [ 2.048942] thermal_sys: Failed to find thermal zone for tmu id=13 > [ 2.060320] thermal_sys: Failed to find thermal zone for tmu id=14 > [ 2.071700] thermal_sys: Failed to find thermal zone for tmu id=15 > > Btw. the driver seems to always register 16 sensors regardless how > many the actual hardware has (or rather: are described in the DT). Yes, it may be nicer to rely on the compatible string to figure out the sensors of the platform and call with full knowledge the registering function. But anyway ...
On 8/8/22 13:31, Daniel Lezcano wrote: > On 08/08/2022 20:35, Guenter Roeck wrote: >> On 8/8/22 11:09, Daniel Lezcano wrote: >>> The previous version of the OF code was returning -ENODEV if no >>> thermal zones description was found or if the lookup of the sensor in >>> the thermal zones was not found. >>> >>> The backend drivers are expecting this return value as an information >>> about skipping the sensor initialization and considered as normal. >>> >>> Fix the return value by replacing -EINVAL by -ENODEV >>> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >>> --- >>> drivers/thermal/thermal_of.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c >>> index 368eb58e97cf..4210c18ef7b2 100644 >>> --- a/drivers/thermal/thermal_of.c >>> +++ b/drivers/thermal/thermal_of.c >>> @@ -329,7 +329,7 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int >>> np = of_find_node_by_name(NULL, "thermal-zones"); >>> if (!np) { >>> pr_err("Unable to find thermal zones description\n"); >> >> I really don't like that error message: People will see it (and complain) >> whenever a sensor registers and there is no thermal zone, even though that >> is perfectly normal (see description above). >> > > I agree, I'll change the message to: > > pr_info("No thermal zones description\n"); > > sounds good ? > I think it should be silent or at best pr_debug. Guenter >>> - return ERR_PTR(-EINVAL); >>> + return ERR_PTR(-ENODEV); >>> } >>> /* >>> @@ -368,7 +368,7 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int >>> } >>> } >>> } >>> - tz = ERR_PTR(-EINVAL); >>> + tz = ERR_PTR(-ENODEV); >>> out: >>> of_node_put(np); >>> return tz; >> > >
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 368eb58e97cf..4210c18ef7b2 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -329,7 +329,7 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int np = of_find_node_by_name(NULL, "thermal-zones"); if (!np) { pr_err("Unable to find thermal zones description\n"); - return ERR_PTR(-EINVAL); + return ERR_PTR(-ENODEV); } /* @@ -368,7 +368,7 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int } } } - tz = ERR_PTR(-EINVAL); + tz = ERR_PTR(-ENODEV); out: of_node_put(np); return tz;
The previous version of the OF code was returning -ENODEV if no thermal zones description was found or if the lookup of the sensor in the thermal zones was not found. The backend drivers are expecting this return value as an information about skipping the sensor initialization and considered as normal. Fix the return value by replacing -EINVAL by -ENODEV Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/thermal_of.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)