[PATCH 1/3] arm64: dts: qcom: sc7180: Add lazor rev4

[PATCH 1/3] arm64: dts: qcom: sc7180: Add lazor rev4

[Matthias Kaehlcke]

Lazor rev3 and older are stuffed with a 47k NTC thermistor for the
charger temperature which currently isn't supported by the PM6150 ADC
driver. A supported thermistor is used in rev4 and later revisions.
Add rev4 .dts files to be able to account for this.

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

 arch/arm64/boot/dts/qcom/Makefile             |  3 ++
 .../dts/qcom/sc7180-trogdor-lazor-r3-kb.dts   |  4 +--
 .../dts/qcom/sc7180-trogdor-lazor-r3-lte.dts  |  4 +--
 .../boot/dts/qcom/sc7180-trogdor-lazor-r3.dts |  4 +--
 .../dts/qcom/sc7180-trogdor-lazor-r4-kb.dts   | 20 +++++++++++++
 .../dts/qcom/sc7180-trogdor-lazor-r4-lte.dts  | 28 +++++++++++++++++++
 .../boot/dts/qcom/sc7180-trogdor-lazor-r4.dts | 16 +++++++++++
 7 files changed, 73 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-kb.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-lte.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 549a7a2151d4..8a8ebca07b25 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -38,6 +38,9 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r1-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r3.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r3-kb.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r3-lte.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r4.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r4-kb.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-lazor-r4-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sc7180-trogdor-r1-lte.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm630-sony-xperia-ganges-kirin.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dts
index 6985beb97e53..a578326ffbad 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-kb.dts
@@ -8,8 +8,8 @@
 #include "sc7180-trogdor-lazor-r3.dts"
 
 / {
-	model = "Google Lazor (rev3+) with KB Backlight";
-	compatible = "google,lazor-sku2", "qcom,sc7180";
+	model = "Google Lazor (rev3) with KB Backlight";
+	compatible = "google,lazor-rev3-sku2", "qcom,sc7180";
 };
 
 &keyboard_backlight {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dts
index 0881f8dd02c9..40169b4a48a3 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3-lte.dts
@@ -9,8 +9,8 @@
 #include "sc7180-trogdor-lte-sku.dtsi"
 
 / {
-	model = "Google Lazor (rev3+) with LTE";
-	compatible = "google,lazor-sku0", "qcom,sc7180";
+	model = "Google Lazor (rev3) with LTE";
+	compatible = "google,lazor-rev3-sku0", "qcom,sc7180";
 };
 
 &ap_sar_sensor {
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 1b9d2f46359e..240c3e067fac 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
@@ -10,6 +10,6 @@
 #include "sc7180-trogdor-lazor.dtsi"
 
 / {
-	model = "Google Lazor (rev3+)";
-	compatible = "google,lazor", "qcom,sc7180";
+	model = "Google Lazor (rev3)";
+	compatible = "google,lazor-rev3", "qcom,sc7180";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-kb.dts
new file mode 100644
index 000000000000..e8c6d3745fed
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-kb.dts
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2021 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-lite.dtsi"
+
+/ {
+	model = "Google Lazor (rev4+) with KB Backlight";
+	compatible = "google,lazor-sku2", "qcom,sc7180";
+};
+
+&keyboard_backlight {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-lte.dts
new file mode 100644
index 000000000000..b260bcf85d4c
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4-lte.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2021 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+	model = "Google Lazor (rev4+) with LTE";
+	compatible = "google,lazor-sku0", "qcom,sc7180";
+};
+
+&ap_sar_sensor {
+	status = "okay";
+};
+
+&ap_sar_sensor_i2c {
+	status = "okay";
+};
+
+&keyboard_backlight {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4.dts
new file mode 100644
index 000000000000..e6f4e030763e
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r4.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Lazor board device tree source
+ *
+ * Copyright 2021 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor-lazor.dtsi"
+#include "sc7180-lite.dtsi"
+
+/ {
+	model = "Google Lazor (rev4+)";
+	compatible = "google,lazor", "qcom,sc7180";
+};
-- 
2.30.0.617.g56c4b15f3c-goog

[PATCH 2/3] arm64: dts: qcom: sc7180: trogdor: Add labels to charger thermal zone and ADC channel

[Matthias Kaehlcke]

Some revisions of trogdor boards use a thermistor for the charger
temperature which currently isn't supported by the PM6150 ADC
driver. Add labels for the charger thermal zone and ADC channel
to allow the removal of these nodes from affected boards and
avoid the use of bogus temperature values.

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

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 07c8b2c926c0..fa996387715b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -15,7 +15,7 @@
 
 / {
 	thermal-zones {
-		charger-thermal {
+		charger_thermal: charger-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 
@@ -706,7 +706,7 @@ &mdss {
 };
 
 &pm6150_adc {
-	charger-thermistor@4f {
+	pm6150_adc_charger_thm: charger-thermistor@4f {
 		reg = <ADC5_AMUX_THM3_100K_PU>;
 		qcom,ratiometric;
 		qcom,hw-settle-time = <200>;
@@ -716,7 +716,7 @@ charger-thermistor@4f {
 &pm6150_adc_tm {
 	status = "okay";
 
-	charger-thermistor@1 {
+	pm6150_adc_tm_charger_thm: charger-thermistor@1 {
 		reg = <1>;
 		io-channels = <&pm6150_adc ADC5_AMUX_THM3_100K_PU>;
 		qcom,ratiometric;
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 2/3] arm64: dts: qcom: sc7180: trogdor: Add labels to charger thermal zone and ADC channel

[Doug Anderson]

Hi,

On Fri, Feb 19, 2021 at 6:11 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Some revisions of trogdor boards use a thermistor for the charger
> temperature which currently isn't supported by the PM6150 ADC
> driver. Add labels for the charger thermal zone and ADC channel
> to allow the removal of these nodes from affected boards and
> avoid the use of bogus temperature values.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

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

[Matthias Kaehlcke]

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;
+
 &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;
-- 
2.30.0.617.g56c4b15f3c-goog

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

[Stephen Boyd]

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;

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

[Matthias Kaehlcke]

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.

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

[Stephen Boyd]

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?

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

[Doug Anderson]

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

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

[Matthias Kaehlcke]

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.

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

[Dmitry Baryshkov]

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?

-- 
With best wishes
Dmitry

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

[Matthias Kaehlcke]

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.

Re: [PATCH 1/3] arm64: dts: qcom: sc7180: Add lazor rev4

[Doug Anderson]

Hi,

On Fri, Feb 19, 2021 at 6:11 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Lazor rev3 and older are stuffed with a 47k NTC thermistor for the
> charger temperature which currently isn't supported by the PM6150 ADC
> driver. A supported thermistor is used in rev4 and later revisions.
> Add rev4 .dts files to be able to account for this.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/Makefile             |  3 ++
>  .../dts/qcom/sc7180-trogdor-lazor-r3-kb.dts   |  4 +--
>  .../dts/qcom/sc7180-trogdor-lazor-r3-lte.dts  |  4 +--
>  .../boot/dts/qcom/sc7180-trogdor-lazor-r3.dts |  4 +--
>  .../dts/qcom/sc7180-trogdor-lazor-r4-kb.dts   | 20 +++++++++++++
>  .../dts/qcom/sc7180-trogdor-lazor-r4-lte.dts  | 28 +++++++++++++++++++
>  .../boot/dts/qcom/sc7180-trogdor-lazor-r4.dts | 16 +++++++++++
>  7 files changed, 73 insertions(+), 6 deletions(-)

From what I can see in the latest discussions -r4 _won't_ get stuffed
with the 100K resistor.  Thus we can just treat -r4 as the same as all
the other revisoins now, right?

-Doug

Re: [PATCH 1/3] arm64: dts: qcom: sc7180: Add lazor rev4

[Matthias Kaehlcke]

On Mon, Feb 22, 2021 at 03:20:53PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Fri, Feb 19, 2021 at 6:11 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Lazor rev3 and older are stuffed with a 47k NTC thermistor for the
> > charger temperature which currently isn't supported by the PM6150 ADC
> > driver. A supported thermistor is used in rev4 and later revisions.
> > Add rev4 .dts files to be able to account for this.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >
> >  arch/arm64/boot/dts/qcom/Makefile             |  3 ++
> >  .../dts/qcom/sc7180-trogdor-lazor-r3-kb.dts   |  4 +--
> >  .../dts/qcom/sc7180-trogdor-lazor-r3-lte.dts  |  4 +--
> >  .../boot/dts/qcom/sc7180-trogdor-lazor-r3.dts |  4 +--
> >  .../dts/qcom/sc7180-trogdor-lazor-r4-kb.dts   | 20 +++++++++++++
> >  .../dts/qcom/sc7180-trogdor-lazor-r4-lte.dts  | 28 +++++++++++++++++++
> >  .../boot/dts/qcom/sc7180-trogdor-lazor-r4.dts | 16 +++++++++++
> >  7 files changed, 73 insertions(+), 6 deletions(-)
> 
> From what I can see in the latest discussions -r4 _won't_ get stuffed
> with the 100K resistor.  Thus we can just treat -r4 as the same as all
> the other revisoins now, right?

Yes, looks like there is not need for an explicit -r4 after all.