LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
@ 2021-08-23 20:47 Matthias Kaehlcke
  2021-08-23 23:50 ` Stephen Boyd
  2021-08-24  0:08 ` Dmitry Baryshkov
  0 siblings, 2 replies; 5+ messages in thread
From: Matthias Kaehlcke @ 2021-08-23 20:47 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Amit Kucheria, Thara Gopinath,
	Zhang Rui, Daniel Lezcano, Dmitry Baryshkov
  Cc: linux-pm, Douglas Anderson, linux-arm-msm, Stephen Boyd,
	linux-kernel, Matthias Kaehlcke

adc_tm5_register_tzd() registers the thermal zone sensors for all
channels of the thermal monitor. If the registration of one channel
fails the function skips the processing of the remaining channels
and returns an error, which results in _probe() being aborted.

One of the reasons the registration could fail is that none of the
thermal zones is using the channel/sensor, which hardly is a critical
error (if it is an error at all). If this case is detected emit a
warning and continue with processing the remaining channels.

Fixes: ca66dca5eda6 ("thermal: qcom: add support for adc-tm5 PMIC thermal monitor")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---

 drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index 232fd0b33325..8494cc04aa21 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -359,6 +359,12 @@ static int adc_tm5_register_tzd(struct adc_tm5_chip *adc_tm)
 							   &adc_tm->channels[i],
 							   &adc_tm5_ops);
 		if (IS_ERR(tzd)) {
+			if (PTR_ERR(tzd) == -ENODEV) {
+				dev_warn(adc_tm->dev, "thermal sensor on channel %d is not used\n",
+					 adc_tm->channels[i].channel);
+				continue;
+			}
+
 			dev_err(adc_tm->dev, "Error registering TZ zone for channel %d: %ld\n",
 				adc_tm->channels[i].channel, PTR_ERR(tzd));
 			return PTR_ERR(tzd);
-- 
2.33.0.rc2.250.ged5fa647cd-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
  2021-08-23 20:47 [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used Matthias Kaehlcke
@ 2021-08-23 23:50 ` Stephen Boyd
  2021-08-24 18:37   ` Matthias Kaehlcke
  2021-08-24  0:08 ` Dmitry Baryshkov
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2021-08-23 23:50 UTC (permalink / raw)
  To: Amit Kucheria, Andy Gross, Bjorn Andersson, Daniel Lezcano,
	Dmitry Baryshkov, Matthias Kaehlcke, Thara Gopinath, Zhang Rui
  Cc: linux-pm, Douglas Anderson, linux-arm-msm, linux-kernel

Quoting Matthias Kaehlcke (2021-08-23 13:47:30)
> adc_tm5_register_tzd() registers the thermal zone sensors for all
> channels of the thermal monitor. If the registration of one channel
> fails the function skips the processing of the remaining channels
> and returns an error, which results in _probe() being aborted.
>
> One of the reasons the registration could fail is that none of the
> thermal zones is using the channel/sensor, which hardly is a critical
> error (if it is an error at all). If this case is detected emit a
> warning and continue with processing the remaining channels.
>
> Fixes: ca66dca5eda6 ("thermal: qcom: add support for adc-tm5 PMIC thermal monitor")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---

Reported-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Should we also disable the 'charger-thermistor@0' node in
sc7180-trogdor.dtsi on lazor boards, instead of the thermal zone, or in
addition to the thermal zone? It isn't used so there's not much value in
exposing it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
  2021-08-23 20:47 [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used Matthias Kaehlcke
  2021-08-23 23:50 ` Stephen Boyd
@ 2021-08-24  0:08 ` Dmitry Baryshkov
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Baryshkov @ 2021-08-24  0:08 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Thara Gopinath,
	Zhang Rui, Daniel Lezcano, Linux PM, Douglas Anderson,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Stephen Boyd, open list

On Mon, 23 Aug 2021 at 23:47, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> adc_tm5_register_tzd() registers the thermal zone sensors for all
> channels of the thermal monitor. If the registration of one channel
> fails the function skips the processing of the remaining channels
> and returns an error, which results in _probe() being aborted.
>
> One of the reasons the registration could fail is that none of the
> thermal zones is using the channel/sensor, which hardly is a critical
> error (if it is an error at all). If this case is detected emit a
> warning and continue with processing the remaining channels.
>
> Fixes: ca66dca5eda6 ("thermal: qcom: add support for adc-tm5 PMIC thermal monitor")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>
>  drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> index 232fd0b33325..8494cc04aa21 100644
> --- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> +++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
> @@ -359,6 +359,12 @@ static int adc_tm5_register_tzd(struct adc_tm5_chip *adc_tm)
>                                                            &adc_tm->channels[i],
>                                                            &adc_tm5_ops);
>                 if (IS_ERR(tzd)) {
> +                       if (PTR_ERR(tzd) == -ENODEV) {
> +                               dev_warn(adc_tm->dev, "thermal sensor on channel %d is not used\n",
> +                                        adc_tm->channels[i].channel);
> +                               continue;
> +                       }
> +
>                         dev_err(adc_tm->dev, "Error registering TZ zone for channel %d: %ld\n",
>                                 adc_tm->channels[i].channel, PTR_ERR(tzd));
>                         return PTR_ERR(tzd);
> --
> 2.33.0.rc2.250.ged5fa647cd-goog
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
  2021-08-23 23:50 ` Stephen Boyd
@ 2021-08-24 18:37   ` Matthias Kaehlcke
  2021-08-24 21:12     ` Stephen Boyd
  0 siblings, 1 reply; 5+ messages in thread
From: Matthias Kaehlcke @ 2021-08-24 18:37 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Amit Kucheria, Andy Gross, Bjorn Andersson, Daniel Lezcano,
	Dmitry Baryshkov, Thara Gopinath, Zhang Rui, linux-pm,
	Douglas Anderson, linux-arm-msm, linux-kernel

On Mon, Aug 23, 2021 at 06:50:04PM -0500, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2021-08-23 13:47:30)
> > adc_tm5_register_tzd() registers the thermal zone sensors for all
> > channels of the thermal monitor. If the registration of one channel
> > fails the function skips the processing of the remaining channels
> > and returns an error, which results in _probe() being aborted.
> >
> > One of the reasons the registration could fail is that none of the
> > thermal zones is using the channel/sensor, which hardly is a critical
> > error (if it is an error at all). If this case is detected emit a
> > warning and continue with processing the remaining channels.
> >
> > Fixes: ca66dca5eda6 ("thermal: qcom: add support for adc-tm5 PMIC thermal monitor")
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> 
> Reported-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Thanks for the review!

> Should we also disable the 'charger-thermistor@0' node in
> sc7180-trogdor.dtsi on lazor boards, instead of the thermal zone, or in
> addition to the thermal zone? It isn't used so there's not much value in
> exposing it.

I wouldn't do it instead of disabling the thermal zone, a TZ without a
sensor doesn't seem very useful.

IIUC the thermistor nodes would need to be deleted, not disabled.
Currently the nodes don't have labels, so either the labels would need
to be added, or the deletion would have to look something like this:

&pm6150_adc {
        /delete-node/ charger-thermistor@4f;
};

&pm6150_adc_tm {
        /delete-node/ charger-thermistor@0;
};

If the change is done for Lazor it should also be done for PomPom and
CoachZ.

I don't see a strong need to delete the ADC nodes with the clutter it
adds, but I'm open to it if you prefer :)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
  2021-08-24 18:37   ` Matthias Kaehlcke
@ 2021-08-24 21:12     ` Stephen Boyd
  0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2021-08-24 21:12 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Amit Kucheria, Andy Gross, Bjorn Andersson, Daniel Lezcano,
	Dmitry Baryshkov, Thara Gopinath, Zhang Rui, linux-pm,
	Douglas Anderson, linux-arm-msm, linux-kernel

Quoting Matthias Kaehlcke (2021-08-24 11:37:42)
> On Mon, Aug 23, 2021 at 06:50:04PM -0500, Stephen Boyd wrote:
> > Quoting Matthias Kaehlcke (2021-08-23 13:47:30)
> > > adc_tm5_register_tzd() registers the thermal zone sensors for all
> > > channels of the thermal monitor. If the registration of one channel
> > > fails the function skips the processing of the remaining channels
> > > and returns an error, which results in _probe() being aborted.
> > >
> > > One of the reasons the registration could fail is that none of the
> > > thermal zones is using the channel/sensor, which hardly is a critical
> > > error (if it is an error at all). If this case is detected emit a
> > > warning and continue with processing the remaining channels.
> > >
> > > Fixes: ca66dca5eda6 ("thermal: qcom: add support for adc-tm5 PMIC thermal monitor")
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > ---
> >
> > Reported-by: Stephen Boyd <swboyd@chromium.org>
> > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>
> Thanks for the review!
>
> > Should we also disable the 'charger-thermistor@0' node in
> > sc7180-trogdor.dtsi on lazor boards, instead of the thermal zone, or in
> > addition to the thermal zone? It isn't used so there's not much value in
> > exposing it.
>
> I wouldn't do it instead of disabling the thermal zone, a TZ without a
> sensor doesn't seem very useful.
>
> IIUC the thermistor nodes would need to be deleted, not disabled.
> Currently the nodes don't have labels, so either the labels would need
> to be added, or the deletion would have to look something like this:
>
> &pm6150_adc {
>         /delete-node/ charger-thermistor@4f;
> };
>
> &pm6150_adc_tm {
>         /delete-node/ charger-thermistor@0;
> };
>
> If the change is done for Lazor it should also be done for PomPom and
> CoachZ.
>
> I don't see a strong need to delete the ADC nodes with the clutter it
> adds, but I'm open to it if you prefer :)

The driver will be present in system memory when it is never used. I was
mostly noticing that adc_tm5_get_dt_data() calls
of_get_available_child_count() and bails probe if there aren't any child
nodes. In this case, the only child node is charger-thermistor, so probe
for this driver would fail if the thermistor was removed. So at that
point, I wonder if we should disable the thermal zone node (already
done) and then also the adc node (sadly called adc-tm). This way we
don't eat up kernel memory to describe a device that isn't used and a
driver that wastes time probing and then fails to probe anyway.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-24 21:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-23 20:47 [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used Matthias Kaehlcke
2021-08-23 23:50 ` Stephen Boyd
2021-08-24 18:37   ` Matthias Kaehlcke
2021-08-24 21:12     ` Stephen Boyd
2021-08-24  0:08 ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).