diff mbox series

[3/3] arm64: dts: qcom: sc7180: Delete charger thermal zone and ADC channel for lazor <= rev3

Message ID 20210219181032.3.Ia4c1022191d09fe8c56a16486b77796b83ffcae4@changeid
State New
Headers show
Series [1/3] arm64: dts: qcom: sc7180: Add lazor rev4 | expand

Commit Message

Matthias Kaehlcke Feb. 20, 2021, 2:10 a.m. UTC
Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
the charger temperature which currently isn't supported by the
PM6150 ADC driver. Delete the charger thermal zone and ADC channel
to avoid the use of bogus temperature values.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---

 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++
 3 files changed, 27 insertions(+)

Comments

Stephen Boyd Feb. 22, 2021, 8:20 p.m. UTC | #1
Quoting Matthias Kaehlcke (2021-02-19 18:10:59)
> Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
> the charger temperature which currently isn't supported by the
> PM6150 ADC driver. Delete the charger thermal zone and ADC channel
> to avoid the use of bogus temperature values.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> 
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> index 30e3e769d2b4..0974dbd424e1 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> @@ -14,6 +14,15 @@ / {
>         compatible = "google,lazor-rev0", "qcom,sc7180";
>  };
>  
> +/*
> + * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
> + * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
> + * channel to avoid the use of bogus temperature values.
> + */
> +/delete-node/ &charger_thermal;
> +/delete-node/ &pm6150_adc_charger_thm;
> +/delete-node/ &pm6150_adc_tm_charger_thm;

Can we disable pm6150_adc_tm instead on <= rev3 boards? It would be the
same number of lines, but is simpler to reason about disabled nodes vs.
deleted nodes usually.

> +
>  &pp3300_hub {
>         /* pp3300_l7c is used to power the USB hub */
>         /delete-property/regulator-always-on;
Matthias Kaehlcke Feb. 22, 2021, 8:38 p.m. UTC | #2
On Mon, Feb 22, 2021 at 12:20:04PM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2021-02-19 18:10:59)
> > Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
> > the charger temperature which currently isn't supported by the
> > PM6150 ADC driver. Delete the charger thermal zone and ADC channel
> > to avoid the use of bogus temperature values.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > 
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++
> >  3 files changed, 27 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > index 30e3e769d2b4..0974dbd424e1 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > @@ -14,6 +14,15 @@ / {
> >         compatible = "google,lazor-rev0", "qcom,sc7180";
> >  };
> >  
> > +/*
> > + * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
> > + * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
> > + * channel to avoid the use of bogus temperature values.
> > + */
> > +/delete-node/ &charger_thermal;
> > +/delete-node/ &pm6150_adc_charger_thm;
> > +/delete-node/ &pm6150_adc_tm_charger_thm;
> 
> Can we disable pm6150_adc_tm instead on <= rev3 boards? It would be the
> same number of lines, but is simpler to reason about disabled nodes vs.
> deleted nodes usually.

For Lazor theoretically this could be done since it doesn't use other ADC
channels, however it won't work for other trogdor devices that will be
upstreamed eventually. Some of these boards have the same problem, however
they have other thermistors connected to the ADC. One could argue that it's
preferable to do things in a uniform way, but I'm open to do it either way
for Lazor.
Stephen Boyd Feb. 22, 2021, 8:45 p.m. UTC | #3
Quoting Matthias Kaehlcke (2021-02-22 12:38:46)
> On Mon, Feb 22, 2021 at 12:20:04PM -0800, Stephen Boyd wrote:

> > Quoting Matthias Kaehlcke (2021-02-19 18:10:59)

> > > Lazor rev3 and older are stuffed with a 47k NTC as thermistor for

> > > the charger temperature which currently isn't supported by the

> > > PM6150 ADC driver. Delete the charger thermal zone and ADC channel

> > > to avoid the use of bogus temperature values.

> > > 

> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

> > > ---

> > > 

> > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++

> > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++

> > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++

> > >  3 files changed, 27 insertions(+)

> > > 

> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > index 30e3e769d2b4..0974dbd424e1 100644

> > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > @@ -14,6 +14,15 @@ / {

> > >         compatible = "google,lazor-rev0", "qcom,sc7180";

> > >  };

> > >  

> > > +/*

> > > + * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently

> > > + * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC

> > > + * channel to avoid the use of bogus temperature values.

> > > + */

> > > +/delete-node/ &charger_thermal;

> > > +/delete-node/ &pm6150_adc_charger_thm;

> > > +/delete-node/ &pm6150_adc_tm_charger_thm;

> > 

> > Can we disable pm6150_adc_tm instead on <= rev3 boards? It would be the

> > same number of lines, but is simpler to reason about disabled nodes vs.

> > deleted nodes usually.

> 

> For Lazor theoretically this could be done since it doesn't use other ADC

> channels, however it won't work for other trogdor devices that will be

> upstreamed eventually. Some of these boards have the same problem, however

> they have other thermistors connected to the ADC. One could argue that it's

> preferable to do things in a uniform way, but I'm open to do it either way

> for Lazor.

> 


I see. Can the thermal-zone be disabled then vs. deleting three nodes? I
think the thermal driver uses for_each_available_child_of_node() so that
would work?
Doug Anderson Feb. 22, 2021, 11:25 p.m. UTC | #4
Hi,

On Mon, Feb 22, 2021 at 12:45 PM Stephen Boyd <swboyd@chromium.org> wrote:
>

> Quoting Matthias Kaehlcke (2021-02-22 12:38:46)

> > On Mon, Feb 22, 2021 at 12:20:04PM -0800, Stephen Boyd wrote:

> > > Quoting Matthias Kaehlcke (2021-02-19 18:10:59)

> > > > Lazor rev3 and older are stuffed with a 47k NTC as thermistor for

> > > > the charger temperature which currently isn't supported by the

> > > > PM6150 ADC driver. Delete the charger thermal zone and ADC channel

> > > > to avoid the use of bogus temperature values.

> > > >

> > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

> > > > ---

> > > >

> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++

> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++

> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++

> > > >  3 files changed, 27 insertions(+)

> > > >

> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > > index 30e3e769d2b4..0974dbd424e1 100644

> > > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts

> > > > @@ -14,6 +14,15 @@ / {

> > > >         compatible = "google,lazor-rev0", "qcom,sc7180";

> > > >  };

> > > >

> > > > +/*

> > > > + * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently

> > > > + * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC

> > > > + * channel to avoid the use of bogus temperature values.

> > > > + */

> > > > +/delete-node/ &charger_thermal;

> > > > +/delete-node/ &pm6150_adc_charger_thm;

> > > > +/delete-node/ &pm6150_adc_tm_charger_thm;

> > >

> > > Can we disable pm6150_adc_tm instead on <= rev3 boards? It would be the

> > > same number of lines, but is simpler to reason about disabled nodes vs.

> > > deleted nodes usually.

> >

> > For Lazor theoretically this could be done since it doesn't use other ADC

> > channels, however it won't work for other trogdor devices that will be

> > upstreamed eventually. Some of these boards have the same problem, however

> > they have other thermistors connected to the ADC. One could argue that it's

> > preferable to do things in a uniform way, but I'm open to do it either way

> > for Lazor.

> >

>

> I see. Can the thermal-zone be disabled then vs. deleting three nodes? I

> think the thermal driver uses for_each_available_child_of_node() so that

> would work?


FWIW: +1 to what Stephen suggests assuming it works.

-Doug
Dmitry Baryshkov Feb. 23, 2021, 11:12 a.m. UTC | #5
Hi,

On Sat, 20 Feb 2021 at 05:13, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
> the charger temperature which currently isn't supported by the
> PM6150 ADC driver. Delete the charger thermal zone and ADC channel
> to avoid the use of bogus temperature values.

Should we just expand the adc/adc-tm drivers with additional calibration tables?
Matthias Kaehlcke Feb. 24, 2021, 4:54 p.m. UTC | #6
On Tue, Feb 23, 2021 at 02:12:30PM +0300, Dmitry Baryshkov wrote:
> Hi,
> 
> On Sat, 20 Feb 2021 at 05:13, Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
> > the charger temperature which currently isn't supported by the
> > PM6150 ADC driver. Delete the charger thermal zone and ADC channel
> > to avoid the use of bogus temperature values.
> 
> Should we just expand the adc/adc-tm drivers with additional calibration tables?

Generally that seems desirable, I'm not sure about the process, I guess
someone with access to a climate chamber would have to create these tables?

I think it would also require an extension of the DT bindings, currently
the ADC driver assumes that a 100k NTC is connected, something in the DT
would have to indicate the thermistor type.

We want to remove the bogus temperatures from the system now, if support for
47k NTCs is added at some point we can consider changing the DT again.
Matthias Kaehlcke Feb. 24, 2021, 5:12 p.m. UTC | #7
On Mon, Feb 22, 2021 at 12:45:23PM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2021-02-22 12:38:46)
> > On Mon, Feb 22, 2021 at 12:20:04PM -0800, Stephen Boyd wrote:
> > > Quoting Matthias Kaehlcke (2021-02-19 18:10:59)
> > > > Lazor rev3 and older are stuffed with a 47k NTC as thermistor for
> > > > the charger temperature which currently isn't supported by the
> > > > PM6150 ADC driver. Delete the charger thermal zone and ADC channel
> > > > to avoid the use of bogus temperature values.
> > > > 
> > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > > ---
> > > > 
> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts | 9 +++++++++
> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts | 9 +++++++++
> > > >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts | 9 +++++++++
> > > >  3 files changed, 27 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > > > index 30e3e769d2b4..0974dbd424e1 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > > > @@ -14,6 +14,15 @@ / {
> > > >         compatible = "google,lazor-rev0", "qcom,sc7180";
> > > >  };
> > > >  
> > > > +/*
> > > > + * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
> > > > + * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
> > > > + * channel to avoid the use of bogus temperature values.
> > > > + */
> > > > +/delete-node/ &charger_thermal;
> > > > +/delete-node/ &pm6150_adc_charger_thm;
> > > > +/delete-node/ &pm6150_adc_tm_charger_thm;
> > > 
> > > Can we disable pm6150_adc_tm instead on <= rev3 boards? It would be the
> > > same number of lines, but is simpler to reason about disabled nodes vs.
> > > deleted nodes usually.
> > 
> > For Lazor theoretically this could be done since it doesn't use other ADC
> > channels, however it won't work for other trogdor devices that will be
> > upstreamed eventually. Some of these boards have the same problem, however
> > they have other thermistors connected to the ADC. One could argue that it's
> > preferable to do things in a uniform way, but I'm open to do it either way
> > for Lazor.
> > 
> 
> I see. Can the thermal-zone be disabled then vs. deleting three nodes? I
> think the thermal driver uses for_each_available_child_of_node() so that
> would work?

Yes, that would work. I also deleted the ADC/TM nodes to remove the bogus
temperature completely from the system, but one could argue that it does
no harm to keep it as long as it isn't used.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
index 30e3e769d2b4..0974dbd424e1 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
@@ -14,6 +14,15 @@  / {
 	compatible = "google,lazor-rev0", "qcom,sc7180";
 };
 
+/*
+ * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
+ * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
+ * channel to avoid the use of bogus temperature values.
+ */
+/delete-node/ &charger_thermal;
+/delete-node/ &pm6150_adc_charger_thm;
+/delete-node/ &pm6150_adc_tm_charger_thm;
+
 &pp3300_hub {
 	/* pp3300_l7c is used to power the USB hub */
 	/delete-property/regulator-always-on;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
index c2ef06367baf..0381ca85ae97 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
@@ -14,6 +14,15 @@  / {
 	compatible = "google,lazor-rev1", "google,lazor-rev2", "qcom,sc7180";
 };
 
+/*
+ * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
+ * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
+ * channel to avoid the use of bogus temperature values.
+ */
+/delete-node/ &charger_thermal;
+/delete-node/ &pm6150_adc_charger_thm;
+/delete-node/ &pm6150_adc_tm_charger_thm;
+
 &pp3300_hub {
 	/* pp3300_l7c is used to power the USB hub */
 	/delete-property/regulator-always-on;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
index 240c3e067fac..b9473bba8f4a 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
@@ -13,3 +13,12 @@  / {
 	model = "Google Lazor (rev3)";
 	compatible = "google,lazor-rev3", "qcom,sc7180";
 };
+
+/*
+ * rev <= 3 are stuffed with a 47k NTC as charger thermistor which is currently
+ * not supported by the PM6150 ADC driver. Delete the thermal zone and ADC
+ * channel to avoid the use of bogus temperature values.
+ */
+/delete-node/ &charger_thermal;
+/delete-node/ &pm6150_adc_charger_thm;
+/delete-node/ &pm6150_adc_tm_charger_thm;