LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: Amit Kucheria <amitk@kernel.org>, Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Thara Gopinath <thara.gopinath@linaro.org>,
	Zhang Rui <rui.zhang@intel.com>,
	linux-pm@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used
Date: Tue, 24 Aug 2021 14:12:29 -0700	[thread overview]
Message-ID: <CAE-0n52SDVCqyp64cWd0uw1F4NRO3N8dT5LNOO+eh-zvWpffoQ@mail.gmail.com> (raw)
In-Reply-To: <YSU8djt765o81YiQ@google.com>

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.

  reply	other threads:[~2021-08-24 21:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-23 20:47 Matthias Kaehlcke
2021-08-23 23:50 ` Stephen Boyd
2021-08-24 18:37   ` Matthias Kaehlcke
2021-08-24 21:12     ` Stephen Boyd [this message]
2021-08-24  0:08 ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAE-0n52SDVCqyp64cWd0uw1F4NRO3N8dT5LNOO+eh-zvWpffoQ@mail.gmail.com \
    --to=swboyd@chromium.org \
    --cc=agross@kernel.org \
    --cc=amitk@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=rui.zhang@intel.com \
    --cc=thara.gopinath@linaro.org \
    --subject='Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don'\''t abort probing if a sensor is not used' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).