Re: [PATCH] thermal: qcom: spmi-adc-tm5: Don't abort probing if a sensor is not used

[Stephen Boyd]

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@xxxxxxxxxxxx>
> > > ---
> >
> > Reported-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
>
> 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.