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@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:
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.
