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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>
>  .../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