Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: add initial trogdor and lazor dt

[Stephen Boyd]

Quoting Rob Clark (2020-08-24 17:33:39)
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> new file mode 100644
> index 000000000000..b04987ab6c22
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -0,0 +1,1364 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
[...]
> +
> +       gpio_keys: gpio-keys {
> +               compatible = "gpio-keys";
> +               status = "disabled";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pen_pdct_l>;
> +
> +               pen-insert {
> +                       label = "Pen Insert";
> +
> +                       /* Insert = low, eject = high */
> +                       gpios = <&tlmm 52 GPIO_ACTIVE_LOW>;
> +                       linux,code = <SW_PEN_INSERTED>;
> +                       linux,input-type = <EV_SW>;
> +                       wakeup-source;
> +               };
> +       };
> +
> +       max98357a: max98357a {

Maybe node name should be 'audio-codec' or 'dac' or 'amp' or 'speaker'
or 'codec'?

> +               compatible = "maxim,max98357a";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&amp_en>;
> +               sdmode-gpios = <&tlmm 23 GPIO_ACTIVE_HIGH>;
> +               #sound-dai-cells = <0>;
> +       };
> +
> +       pwmleds {
> +               compatible = "pwm-leds";
> +               keyboard_backlight: keyboard-backlight {
> +                       status = "disabled";
> +                       label = "cros_ec::kbd_backlight";
> +                       pwms = <&cros_ec_pwm 0>;
> +                       max-brightness = <1023>;
> +               };
> +       };
> +};
> +
> +&qfprom {
> +       vcc-supply = <&pp1800_l11a>;
> +};
> +
> +

Drop double newline?

> +&qspi {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&qspi_clk>, <&qspi_cs0>, <&qspi_data01>;
> +
> +       flash@0 {
> +               compatible = "jedec,spi-nor";
> +               reg = <0>;
> +
> +               /* TODO: Increase frequency after testing */

Maybe drop this TODO? I don't see it being too meaningful.

> +               spi-max-frequency = <25000000>;
> +               spi-tx-bus-width = <2>;
> +               spi-rx-bus-width = <2>;
> +       };
> +};
> +
> +

Drop double newline?

> +&apps_rsc {
> +       pm6150-rpmh-regulators {
> +               compatible = "qcom,pm6150-rpmh-regulators";
> +
> +&wifi {
> +       status = "okay";
> +       vdd-0.8-cx-mx-supply = <&vdd_cx_wlan>;
> +       vdd-1.8-xo-supply = <&pp1800_l1c>;
> +       vdd-1.3-rfa-supply = <&pp1300_l2c>;
> +
> +       /*
> +        * TODO: Put ch1 supply in its rightful place, rather than in ch0's
> +        * spot. Channel 0 is held open by bluetooth for now.
> +        */
> +       vdd-3.3-ch0-supply = <&pp3300_l11c>;
> +       wifi-firmware {
> +               iommus = <&apps_smmu 0xc2 0x1>;
> +       };
> +};
> +
> +/* PINCTRL - additions to nodes defined in sdm845.dtsi */

Oops, that should be sc7180.dtsi

> +
> +&qspi_cs0 {
> +       pinconf {
> +               pins = "gpio68";
> +               bias-disable;
> +       };
[...]
> +/* PINCTRL - board-specific pinctrl */
> +
> +&pm6150_gpio {
> +       status = "disabled"; /* No GPIOs are connected */
> +};
> +
> +&pm6150l_gpio {
> +       gpio-line-names = "AP_SUSPEND",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "",
> +                         "";
> +};
> +
> +&tlmm {
[...]
> +
> +&venus {
> +       video-firmware {
> +               iommus = <&apps_smmu 0x0c42 0x0>;
> +       };
> +};

I believe this node should come before the PINCTRL comment above and be
sorted alphabetically by node name.

 +/* PINCTRL - additions to nodes defined in sdm845.dtsi */