[PATCH 0/4] arm64: dts: qcom: sc7180: Disable the charger thermal zone on more trogdor boards

[PATCH 0/4] arm64: dts: qcom: sc7180: Disable the charger thermal zone on more trogdor boards

[Matthias Kaehlcke]

We already disabled the charger thermal zone for lazor to avoid
bogus temperature readings from an unsupported thermistor. Some
revisions of other trogdor boards that are added by Doug's
'arm64: dts: qcom: Update sc7180-trogdor variants from downstream'
series have the same problem. Disable the charger thermal zone for
them too.

This series is based on v2 of the 'arm64: dts: qcom: Update
sc7180-trogdor variants from downstream' series
(https://patchwork.kernel.org/project/linux-arm-msm/list/?series=440315)


Matthias Kaehlcke (4):
  arm64: dts: qcom: sc7180: lazor: Simplify disabling of charger thermal
    zone
  arm64: dts: qcom: sc7180: Add pompom rev3
  arm64: dts: qcom: sc7180: pompom: Disable charger thermal zone for
    rev1 and rev2
  arm64: dts: qcom: sc7180: Disable charger thermal zone for coachz rev1
    and rev2

 .../dts/qcom/sc7180-trogdor-coachz-r1.dts     |  9 ++++
 .../dts/qcom/sc7180-trogdor-coachz-r2.dts     |  9 ++++
 .../boot/dts/qcom/sc7180-trogdor-lazor-r0.dts |  9 ----
 .../boot/dts/qcom/sc7180-trogdor-lazor-r1.dts |  9 ----
 .../boot/dts/qcom/sc7180-trogdor-lazor-r3.dts |  9 ----
 .../boot/dts/qcom/sc7180-trogdor-lazor.dtsi   |  9 ++++
 .../dts/qcom/sc7180-trogdor-pompom-r1.dts     |  9 ++++
 .../dts/qcom/sc7180-trogdor-pompom-r2-lte.dts |  4 +-
 .../dts/qcom/sc7180-trogdor-pompom-r2.dts     | 13 +++++-
 .../dts/qcom/sc7180-trogdor-pompom-r3-lte.dts | 14 ++++++
 .../dts/qcom/sc7180-trogdor-pompom-r3.dts     | 46 +++++++++++++++++++
 11 files changed, 109 insertions(+), 31 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts

-- 
2.30.1.766.gb4fecdf3b7-goog

[PATCH 1/4] arm64: dts: qcom: sc7180: lazor: Simplify disabling of charger thermal zone

[Matthias Kaehlcke]

Commit f73558cc83d1 ("arm64: dts: qcom: sc7180: Disable charger
thermal zone for lazor") disables the charger thermal zone for
specific lazor revisions due to an unsupported thermistor type.
The initial idea was to disable the thermal zone for older
revisions and leave it enabled for newer ones that use a
supported thermistor. Finally the thermistor won't be changed
on newer revisions, hence the thermal zone should be disabled
for all lazor (and limozeen) revisions. Instead of disabling
it per revision do it once in the shared .dtsi for lazor.

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 ---------
 arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi   | 9 +++++++++
 4 files changed, 9 insertions(+), 27 deletions(-)

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 5c997cd90069..30e3e769d2b4 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
@@ -14,15 +14,6 @@ / {
 	compatible = "google,lazor-rev0", "qcom,sc7180";
 };
 
-/*
- * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
- * not supported by the PM6150 ADC driver. Disable the charger thermal zone
- * to avoid using bogus temperature values.
- */
-&charger_thermal {
-	status = "disabled";
-};
-
 &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 d9fbcc7bc5bd..c2ef06367baf 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
@@ -14,15 +14,6 @@ / {
 	compatible = "google,lazor-rev1", "google,lazor-rev2", "qcom,sc7180";
 };
 
-/*
- * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
- * not supported by the PM6150 ADC driver. Disable the charger thermal zone
- * to avoid using bogus temperature values.
- */
-&charger_thermal {
-	status = "disabled";
-};
-
 &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 19e69adb9e04..1b9d2f46359e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
@@ -13,12 +13,3 @@ / {
 	model = "Google Lazor (rev3+)";
 	compatible = "google,lazor", "qcom,sc7180";
 };
-
-/*
- * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
- * not supported by the PM6150 ADC driver. Disable the charger thermal zone
- * to avoid using bogus temperature values.
- */
-&charger_thermal {
-	status = "disabled";
-};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index 89e5cd29ec09..aa2c4a9098db 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -58,6 +58,15 @@ ap_ts: touchscreen@10 {
 	};
 };
 
+/*
+ * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
+ * not supported by the PM6150 ADC driver. Disable the charger thermal zone
+ * to avoid using bogus temperature values.
+ */
+&charger_thermal {
+	status = "disabled";
+};
+
 /* PINCTRL - modifications to sc7180-trogdor.dtsi */
 
 &ts_reset_l {
-- 
2.30.1.766.gb4fecdf3b7-goog

Re: [PATCH 1/4] arm64: dts: qcom: sc7180: lazor: Simplify disabling of charger thermal zone

[Doug Anderson]

Hi,

On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Commit f73558cc83d1 ("arm64: dts: qcom: sc7180: Disable charger
> thermal zone for lazor") disables the charger thermal zone for
> specific lazor revisions due to an unsupported thermistor type.
> The initial idea was to disable the thermal zone for older
> revisions and leave it enabled for newer ones that use a
> supported thermistor. Finally the thermistor won't be changed
> on newer revisions, hence the thermal zone should be disabled
> for all lazor (and limozeen) revisions. Instead of disabling
> it per revision do it once in the shared .dtsi for lazor.
>
> 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 ---------
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi   | 9 +++++++++
>  4 files changed, 9 insertions(+), 27 deletions(-)
>
> 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 5c997cd90069..30e3e769d2b4 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> @@ -14,15 +14,6 @@ / {
>         compatible = "google,lazor-rev0", "qcom,sc7180";
>  };
>
> -/*
> - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> - * to avoid using bogus temperature values.
> - */
> -&charger_thermal {
> -       status = "disabled";
> -};
> -
>  &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 d9fbcc7bc5bd..c2ef06367baf 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
> @@ -14,15 +14,6 @@ / {
>         compatible = "google,lazor-rev1", "google,lazor-rev2", "qcom,sc7180";
>  };
>
> -/*
> - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> - * to avoid using bogus temperature values.
> - */
> -&charger_thermal {
> -       status = "disabled";
> -};
> -
>  &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 19e69adb9e04..1b9d2f46359e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
> @@ -13,12 +13,3 @@ / {
>         model = "Google Lazor (rev3+)";
>         compatible = "google,lazor", "qcom,sc7180";
>  };
> -
> -/*
> - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> - * to avoid using bogus temperature values.
> - */
> -&charger_thermal {
> -       status = "disabled";
> -};
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> index 89e5cd29ec09..aa2c4a9098db 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> @@ -58,6 +58,15 @@ ap_ts: touchscreen@10 {
>         };
>  };
>
> +/*
> + * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> + * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> + * to avoid using bogus temperature values.
> + */
> +&charger_thermal {
> +       status = "disabled";
> +};
> +
>  /* PINCTRL - modifications to sc7180-trogdor.dtsi */
>
>  &ts_reset_l {

The idea is right, but I'm having a hard time figuring out what tree
you posted your patch against. You said you did it atop my "v2" series
[1], right?  ...but the "sc7180-trogdor-lazor.dtsi" really doesn't
match. In my tree, for instance, right above the PINCTRL comment
should be:

&wifi {
  qcom,ath10k-calibration-variant = "GO_LAZOR";
};

...but that's definitely not what's there in whatever your patch was
written against... It seems like you're also missing the panel and
trackpad nodes...

[1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=440315


-Doug

Re: [PATCH 1/4] arm64: dts: qcom: sc7180: lazor: Simplify disabling of charger thermal zone

[Matthias Kaehlcke]

On Fri, Mar 05, 2021 at 10:36:49AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Commit f73558cc83d1 ("arm64: dts: qcom: sc7180: Disable charger
> > thermal zone for lazor") disables the charger thermal zone for
> > specific lazor revisions due to an unsupported thermistor type.
> > The initial idea was to disable the thermal zone for older
> > revisions and leave it enabled for newer ones that use a
> > supported thermistor. Finally the thermistor won't be changed
> > on newer revisions, hence the thermal zone should be disabled
> > for all lazor (and limozeen) revisions. Instead of disabling
> > it per revision do it once in the shared .dtsi for lazor.
> >
> > 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 ---------
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi   | 9 +++++++++
> >  4 files changed, 9 insertions(+), 27 deletions(-)
> >
> > 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 5c997cd90069..30e3e769d2b4 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r0.dts
> > @@ -14,15 +14,6 @@ / {
> >         compatible = "google,lazor-rev0", "qcom,sc7180";
> >  };
> >
> > -/*
> > - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> > - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> > - * to avoid using bogus temperature values.
> > - */
> > -&charger_thermal {
> > -       status = "disabled";
> > -};
> > -
> >  &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 d9fbcc7bc5bd..c2ef06367baf 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r1.dts
> > @@ -14,15 +14,6 @@ / {
> >         compatible = "google,lazor-rev1", "google,lazor-rev2", "qcom,sc7180";
> >  };
> >
> > -/*
> > - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> > - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> > - * to avoid using bogus temperature values.
> > - */
> > -&charger_thermal {
> > -       status = "disabled";
> > -};
> > -
> >  &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 19e69adb9e04..1b9d2f46359e 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r3.dts
> > @@ -13,12 +13,3 @@ / {
> >         model = "Google Lazor (rev3+)";
> >         compatible = "google,lazor", "qcom,sc7180";
> >  };
> > -
> > -/*
> > - * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> > - * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> > - * to avoid using bogus temperature values.
> > - */
> > -&charger_thermal {
> > -       status = "disabled";
> > -};
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > index 89e5cd29ec09..aa2c4a9098db 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > @@ -58,6 +58,15 @@ ap_ts: touchscreen@10 {
> >         };
> >  };
> >
> > +/*
> > + * Lazor is stuffed with a 47k NTC as charger thermistor which currently is
> > + * not supported by the PM6150 ADC driver. Disable the charger thermal zone
> > + * to avoid using bogus temperature values.
> > + */
> > +&charger_thermal {
> > +       status = "disabled";
> > +};
> > +
> >  /* PINCTRL - modifications to sc7180-trogdor.dtsi */
> >
> >  &ts_reset_l {
> 
> The idea is right, but I'm having a hard time figuring out what tree
> you posted your patch against. You said you did it atop my "v2" series
> [1], right?  ...but the "sc7180-trogdor-lazor.dtsi" really doesn't
> match. In my tree, for instance, right above the PINCTRL comment
> should be:
> 
> &wifi {
>   qcom,ath10k-calibration-variant = "GO_LAZOR";
> };
> 
> ...but that's definitely not what's there in whatever your patch was
> written against... It seems like you're also missing the panel and
> trackpad nodes...
> 
> [1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=440315

You got me! I was too lazy to pick all 13 patches, since most of them are
irrelevant for this series, but apparently I missed some that are. I guess
I'll pick them all for v2 ...

[PATCH 2/4] arm64: dts: qcom: sc7180: Add pompom rev3

[Matthias Kaehlcke]

The only kernel visible change with respect to rev2 is that pompom
rev3 changed the charger thermistor from a 47k to a 100k NTC to use
a thermistor which is supported by the PM6150 ADC driver.

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

 .../dts/qcom/sc7180-trogdor-pompom-r2-lte.dts |  4 +-
 .../dts/qcom/sc7180-trogdor-pompom-r2.dts     |  4 +-
 .../dts/qcom/sc7180-trogdor-pompom-r3-lte.dts | 14 ++++++
 .../dts/qcom/sc7180-trogdor-pompom-r3.dts     | 46 +++++++++++++++++++
 4 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
index 791d496ad046..00e187c08eb9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
@@ -9,6 +9,6 @@
 #include "sc7180-trogdor-lte-sku.dtsi"
 
 / {
-	model = "Google Pompom (rev2+) with LTE";
-	compatible = "google,pompom-sku0", "qcom,sc7180";
+	model = "Google Pompom (rev2) with LTE";
+	compatible = "google,pompom-rev2-sku0", "qcom,sc7180";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
index 984d7337da78..2b2bd906321d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
@@ -10,8 +10,8 @@
 #include "sc7180-trogdor-pompom.dtsi"
 
 / {
-	model = "Google Pompom (rev2+)";
-	compatible = "google,pompom", "qcom,sc7180";
+	model = "Google Pompom (rev2)";
+	compatible = "google,pompom-rev2", "qcom,sc7180";
 };
 
 &keyboard_controller {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
new file mode 100644
index 000000000000..067cb75a011e
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Pompom board device tree source
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include "sc7180-trogdor-pompom-r3.dts"
+#include "sc7180-trogdor-lte-sku.dtsi"
+
+/ {
+	model = "Google Pompom (rev3+) with LTE";
+	compatible = "google,pompom-sku0", "qcom,sc7180";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
new file mode 100644
index 000000000000..12d2d1e8e9e1
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Pompom board device tree source
+ *
+ * Copyright 2021 Google LLC.
+ */
+
+/dts-v1/;
+
+#include "sc7180-trogdor-pompom.dtsi"
+
+/ {
+	model = "Google Pompom (rev3+)";
+	compatible = "google,pompom", "qcom,sc7180";
+};
+
+&keyboard_controller {
+	function-row-physmap = <
+		MATRIX_KEY(0x00, 0x02, 0)	/* T1 */
+		MATRIX_KEY(0x03, 0x02, 0)	/* T2 */
+		MATRIX_KEY(0x02, 0x02, 0)	/* T3 */
+		MATRIX_KEY(0x01, 0x02, 0)	/* T4 */
+		MATRIX_KEY(0x03, 0x04, 0)	/* T5 */
+		MATRIX_KEY(0x02, 0x04, 0)	/* T6 */
+		MATRIX_KEY(0x01, 0x04, 0)	/* T7 */
+		MATRIX_KEY(0x02, 0x09, 0)	/* T8 */
+		MATRIX_KEY(0x01, 0x09, 0)	/* T9 */
+		MATRIX_KEY(0x00, 0x04, 0)	/* T10 */
+	>;
+	linux,keymap = <
+		MATRIX_KEY(0x00, 0x02, KEY_BACK)
+		MATRIX_KEY(0x03, 0x02, KEY_REFRESH)
+		MATRIX_KEY(0x02, 0x02, KEY_ZOOM)
+		MATRIX_KEY(0x01, 0x02, KEY_SCALE)
+		MATRIX_KEY(0x03, 0x04, KEY_SYSRQ)
+		MATRIX_KEY(0x02, 0x04, KEY_BRIGHTNESSDOWN)
+		MATRIX_KEY(0x01, 0x04, KEY_BRIGHTNESSUP)
+		MATRIX_KEY(0x02, 0x09, KEY_MUTE)
+		MATRIX_KEY(0x01, 0x09, KEY_VOLUMEDOWN)
+		MATRIX_KEY(0x00, 0x04, KEY_VOLUMEUP)
+
+		MATRIX_KEY(0x03, 0x09, KEY_SLEEP)	/* LOCK key */
+
+		CROS_STD_MAIN_KEYMAP
+	>;
+};
-- 
2.30.1.766.gb4fecdf3b7-goog

Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add pompom rev3

[Doug Anderson]

Hi,

On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> The only kernel visible change with respect to rev2 is that pompom
> rev3 changed the charger thermistor from a 47k to a 100k NTC to use
> a thermistor which is supported by the PM6150 ADC driver.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
>  .../dts/qcom/sc7180-trogdor-pompom-r2-lte.dts |  4 +-
>  .../dts/qcom/sc7180-trogdor-pompom-r2.dts     |  4 +-
>  .../dts/qcom/sc7180-trogdor-pompom-r3-lte.dts | 14 ++++++
>  .../dts/qcom/sc7180-trogdor-pompom-r3.dts     | 46 +++++++++++++++++++
>  4 files changed, 64 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> index 791d496ad046..00e187c08eb9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> @@ -9,6 +9,6 @@
>  #include "sc7180-trogdor-lte-sku.dtsi"
>
>  / {
> -       model = "Google Pompom (rev2+) with LTE";
> -       compatible = "google,pompom-sku0", "qcom,sc7180";
> +       model = "Google Pompom (rev2) with LTE";
> +       compatible = "google,pompom-rev2-sku0", "qcom,sc7180";
>  };
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> index 984d7337da78..2b2bd906321d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> @@ -10,8 +10,8 @@
>  #include "sc7180-trogdor-pompom.dtsi"
>
>  / {
> -       model = "Google Pompom (rev2+)";
> -       compatible = "google,pompom", "qcom,sc7180";
> +       model = "Google Pompom (rev2)";
> +       compatible = "google,pompom-rev2", "qcom,sc7180";
>  };
>
>  &keyboard_controller {
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
> new file mode 100644
> index 000000000000..067cb75a011e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Pompom board device tree source
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include "sc7180-trogdor-pompom-r3.dts"
> +#include "sc7180-trogdor-lte-sku.dtsi"
> +
> +/ {
> +       model = "Google Pompom (rev3+) with LTE";
> +       compatible = "google,pompom-sku0", "qcom,sc7180";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
> new file mode 100644
> index 000000000000..12d2d1e8e9e1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Pompom board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor-pompom.dtsi"
> +
> +/ {
> +       model = "Google Pompom (rev3+)";
> +       compatible = "google,pompom", "qcom,sc7180";
> +};
> +
> +&keyboard_controller {
> +       function-row-physmap = <
> +               MATRIX_KEY(0x00, 0x02, 0)       /* T1 */
> +               MATRIX_KEY(0x03, 0x02, 0)       /* T2 */
> +               MATRIX_KEY(0x02, 0x02, 0)       /* T3 */
> +               MATRIX_KEY(0x01, 0x02, 0)       /* T4 */
> +               MATRIX_KEY(0x03, 0x04, 0)       /* T5 */
> +               MATRIX_KEY(0x02, 0x04, 0)       /* T6 */
> +               MATRIX_KEY(0x01, 0x04, 0)       /* T7 */
> +               MATRIX_KEY(0x02, 0x09, 0)       /* T8 */
> +               MATRIX_KEY(0x01, 0x09, 0)       /* T9 */
> +               MATRIX_KEY(0x00, 0x04, 0)       /* T10 */
> +       >;
> +       linux,keymap = <
> +               MATRIX_KEY(0x00, 0x02, KEY_BACK)
> +               MATRIX_KEY(0x03, 0x02, KEY_REFRESH)
> +               MATRIX_KEY(0x02, 0x02, KEY_ZOOM)
> +               MATRIX_KEY(0x01, 0x02, KEY_SCALE)
> +               MATRIX_KEY(0x03, 0x04, KEY_SYSRQ)
> +               MATRIX_KEY(0x02, 0x04, KEY_BRIGHTNESSDOWN)
> +               MATRIX_KEY(0x01, 0x04, KEY_BRIGHTNESSUP)
> +               MATRIX_KEY(0x02, 0x09, KEY_MUTE)
> +               MATRIX_KEY(0x01, 0x09, KEY_VOLUMEDOWN)
> +               MATRIX_KEY(0x00, 0x04, KEY_VOLUMEUP)
> +
> +               MATRIX_KEY(0x03, 0x09, KEY_SLEEP)       /* LOCK key */
> +
> +               CROS_STD_MAIN_KEYMAP
> +       >;
> +};

I don't love copying all this keymap stuff.  Options I can think of:

1. Just put it in "-rev3".  Have the "-rev2" dts just include the
"-rev3" dts and then override the model/compatible and disable the
charger_thermal.

2. Put the keyboard stuff in the "dtsi" file and then "-rev1" can have
something like:

/delete-node/ keyboard_controller;
#include <arm/cros-ec-keyboard.dtsi>

In general the preference is that the ugly device trees should get
pushed down to earlier revs since (eventually) they can just be
dropped.

I'll also mention that I don't see a huge benefit in this being a
separate patch from the next one--I'd just squash them together...

-Doug

Re: [PATCH 2/4] arm64: dts: qcom: sc7180: Add pompom rev3

[Matthias Kaehlcke]

On Fri, Mar 05, 2021 at 10:48:16AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > The only kernel visible change with respect to rev2 is that pompom
> > rev3 changed the charger thermistor from a 47k to a 100k NTC to use
> > a thermistor which is supported by the PM6150 ADC driver.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >
> >  .../dts/qcom/sc7180-trogdor-pompom-r2-lte.dts |  4 +-
> >  .../dts/qcom/sc7180-trogdor-pompom-r2.dts     |  4 +-
> >  .../dts/qcom/sc7180-trogdor-pompom-r3-lte.dts | 14 ++++++
> >  .../dts/qcom/sc7180-trogdor-pompom-r3.dts     | 46 +++++++++++++++++++
> >  4 files changed, 64 insertions(+), 4 deletions(-)
> >  create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
> >  create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> > index 791d496ad046..00e187c08eb9 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2-lte.dts
> > @@ -9,6 +9,6 @@
> >  #include "sc7180-trogdor-lte-sku.dtsi"
> >
> >  / {
> > -       model = "Google Pompom (rev2+) with LTE";
> > -       compatible = "google,pompom-sku0", "qcom,sc7180";
> > +       model = "Google Pompom (rev2) with LTE";
> > +       compatible = "google,pompom-rev2-sku0", "qcom,sc7180";
> >  };
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> > index 984d7337da78..2b2bd906321d 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
> > @@ -10,8 +10,8 @@
> >  #include "sc7180-trogdor-pompom.dtsi"
> >
> >  / {
> > -       model = "Google Pompom (rev2+)";
> > -       compatible = "google,pompom", "qcom,sc7180";
> > +       model = "Google Pompom (rev2)";
> > +       compatible = "google,pompom-rev2", "qcom,sc7180";
> >  };
> >
> >  &keyboard_controller {
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
> > new file mode 100644
> > index 000000000000..067cb75a011e
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3-lte.dts
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Google Pompom board device tree source
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +
> > +#include "sc7180-trogdor-pompom-r3.dts"
> > +#include "sc7180-trogdor-lte-sku.dtsi"
> > +
> > +/ {
> > +       model = "Google Pompom (rev3+) with LTE";
> > +       compatible = "google,pompom-sku0", "qcom,sc7180";
> > +};
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
> > new file mode 100644
> > index 000000000000..12d2d1e8e9e1
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r3.dts
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Google Pompom board device tree source
> > + *
> > + * Copyright 2021 Google LLC.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sc7180-trogdor-pompom.dtsi"
> > +
> > +/ {
> > +       model = "Google Pompom (rev3+)";
> > +       compatible = "google,pompom", "qcom,sc7180";
> > +};
> > +
> > +&keyboard_controller {
> > +       function-row-physmap = <
> > +               MATRIX_KEY(0x00, 0x02, 0)       /* T1 */
> > +               MATRIX_KEY(0x03, 0x02, 0)       /* T2 */
> > +               MATRIX_KEY(0x02, 0x02, 0)       /* T3 */
> > +               MATRIX_KEY(0x01, 0x02, 0)       /* T4 */
> > +               MATRIX_KEY(0x03, 0x04, 0)       /* T5 */
> > +               MATRIX_KEY(0x02, 0x04, 0)       /* T6 */
> > +               MATRIX_KEY(0x01, 0x04, 0)       /* T7 */
> > +               MATRIX_KEY(0x02, 0x09, 0)       /* T8 */
> > +               MATRIX_KEY(0x01, 0x09, 0)       /* T9 */
> > +               MATRIX_KEY(0x00, 0x04, 0)       /* T10 */
> > +       >;
> > +       linux,keymap = <
> > +               MATRIX_KEY(0x00, 0x02, KEY_BACK)
> > +               MATRIX_KEY(0x03, 0x02, KEY_REFRESH)
> > +               MATRIX_KEY(0x02, 0x02, KEY_ZOOM)
> > +               MATRIX_KEY(0x01, 0x02, KEY_SCALE)
> > +               MATRIX_KEY(0x03, 0x04, KEY_SYSRQ)
> > +               MATRIX_KEY(0x02, 0x04, KEY_BRIGHTNESSDOWN)
> > +               MATRIX_KEY(0x01, 0x04, KEY_BRIGHTNESSUP)
> > +               MATRIX_KEY(0x02, 0x09, KEY_MUTE)
> > +               MATRIX_KEY(0x01, 0x09, KEY_VOLUMEDOWN)
> > +               MATRIX_KEY(0x00, 0x04, KEY_VOLUMEUP)
> > +
> > +               MATRIX_KEY(0x03, 0x09, KEY_SLEEP)       /* LOCK key */
> > +
> > +               CROS_STD_MAIN_KEYMAP
> > +       >;
> > +};
> 
> I don't love copying all this keymap stuff.  Options I can think of:

agreed

> 1. Just put it in "-rev3".  Have the "-rev2" dts just include the
> "-rev3" dts and then override the model/compatible and disable the
> charger_thermal.
> 
> 2. Put the keyboard stuff in the "dtsi" file and then "-rev1" can have
> something like:
> 
> /delete-node/ keyboard_controller;
> #include <arm/cros-ec-keyboard.dtsi>
> 
> In general the preference is that the ugly device trees should get
> pushed down to earlier revs since (eventually) they can just be
> dropped.

Thanks, I'll evaluate the options you suggested.

> I'll also mention that I don't see a huge benefit in this being a
> separate patch from the next one--I'd just squash them together...

It keeps the individual patches simpler by doing one thing at a time,
but you can certainly go either way.

[PATCH 3/4] arm64: dts: qcom: sc7180: pompom: Disable charger thermal zone for rev1 and rev2

[Matthias Kaehlcke]

Pompom rev1 and rev2 are stuffed with a 47k NTC as thermistor for the
charger temperature which currently isn't supported by the PM6150 ADC
driver. Disable the charger thermal zone to avoid the use of bogus
temperature values.

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

 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts | 9 +++++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
index e720e7bd0d70..bc6574e446a9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r1.dts
@@ -14,6 +14,15 @@ / {
 	compatible = "google,pompom-rev1", "qcom,sc7180";
 };
 
+/*
+ * Pompom rev1 is stuffed with a 47k NTC as charger thermistor which currently
+ * is not supported by the PM6150 ADC driver. Disable the charger thermal zone
+ * to avoid using bogus temperature values.
+ */
+&charger_thermal {
+	status = "disabled";
+};
+
 &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-pompom-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
index 2b2bd906321d..779c90ed35e4 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom-r2.dts
@@ -14,6 +14,15 @@ / {
 	compatible = "google,pompom-rev2", "qcom,sc7180";
 };
 
+/*
+ * Pompom rev2 is stuffed with a 47k NTC as charger thermistor which currently
+ * is not supported by the PM6150 ADC driver. Disable the charger thermal zone
+ * to avoid using bogus temperature values.
+ */
+&charger_thermal {
+	status = "disabled";
+};
+
 &keyboard_controller {
 	function-row-physmap = <
 		MATRIX_KEY(0x00, 0x02, 0)	/* T1 */
-- 
2.30.1.766.gb4fecdf3b7-goog

[PATCH 4/4] arm64: dts: qcom: sc7180: Disable charger thermal zone for coachz rev1 and rev2

[Matthias Kaehlcke]

CoachZ rev1 and rev2 are stuffed with a 47k NTC as thermistor for the
charger temperature which currently isn't supported by the PM6150 ADC
driver. Disable the charger thermal zone to avoid the use of bogus
temperature values.

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

 arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts | 9 +++++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts
index 86619f6c1134..c6b078e70d31 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts
@@ -14,6 +14,15 @@ / {
 	compatible = "google,coachz-rev1", "qcom,sc7180";
 };
 
+/*
+ * CoachZ rev1 is stuffed with a 47k NTC as charger thermistor which currently
+ * is not supported by the PM6150 ADC driver. Disable the charger thermal zone
+ * to avoid using bogus temperature values.
+ */
+&charger_thermal {
+	status = "disabled";
+};
+
 &tlmm {
 	gpio-line-names = "HUB_RST_L",
 			  "AP_RAM_ID0",
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts
index 4f69b6ba299f..67c31795a566 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts
@@ -13,3 +13,12 @@ / {
 	model = "Google CoachZ (rev2+)";
 	compatible = "google,coachz", "qcom,sc7180";
 };
+
+/*
+ * CoachZ rev2 is stuffed with a 47k NTC as charger thermistor which currently
+ * is not supported by the PM6150 ADC driver. Disable the charger thermal zone
+ * to avoid using bogus temperature values.
+ */
+&charger_thermal {
+	status = "disabled";
+};
-- 
2.30.1.766.gb4fecdf3b7-goog

Re: [PATCH 4/4] arm64: dts: qcom: sc7180: Disable charger thermal zone for coachz rev1 and rev2

[Doug Anderson]

Hi,

On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> CoachZ rev1 and rev2 are stuffed with a 47k NTC as thermistor for the
> charger temperature which currently isn't supported by the PM6150 ADC
> driver. Disable the charger thermal zone to avoid the use of bogus
> temperature values.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts | 9 +++++++++
>  arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts | 9 +++++++++
>  2 files changed, 18 insertions(+)

I guess this patch is written with the assumption that eventually
we'll create a "-r3" or "-r4" that has a proper thermistor stuffed but
maybe we're not sure which one?  Right now you're disabling it for
both -r1 and -r2+ which is all revisions, so this could go in the
coachz.dtsi file...

-Doug

Re: [PATCH 4/4] arm64: dts: qcom: sc7180: Disable charger thermal zone for coachz rev1 and rev2

[Matthias Kaehlcke]

On Fri, Mar 05, 2021 at 10:50:12AM -0800, Doug Anderson wrote:
> Hi,
> 
> On Thu, Mar 4, 2021 at 10:04 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > CoachZ rev1 and rev2 are stuffed with a 47k NTC as thermistor for the
> > charger temperature which currently isn't supported by the PM6150 ADC
> > driver. Disable the charger thermal zone to avoid the use of bogus
> > temperature values.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r1.dts | 9 +++++++++
> >  arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz-r2.dts | 9 +++++++++
> >  2 files changed, 18 insertions(+)
> 
> I guess this patch is written with the assumption that eventually
> we'll create a "-r3" or "-r4" that has a proper thermistor stuffed but
> maybe we're not sure which one?  Right now you're disabling it for
> both -r1 and -r2+ which is all revisions, so this could go in the
> coachz.dtsi file...

The proper thermistor should be stuffed in rev3.

For some reason there was already a -r3 .dts in my mental model,
probably I created one when I started with a downstream version of
this series just before you posted your's to bring upstream (mostly)
in sync with downstream. I'll add it in v2.