by Doug Anderson

[permalink] [raw]

Subject: Re: [PATCH v6 2/6] arm64: dts: qcom: sc7180: Add wormdingler dts files

Hi,

On Fri, Jun 17, 2022 at 11:12 AM Joseph S. Barrera III
<[email protected]> wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dts
> new file mode 100644
> index 000000000000..1a921a540075
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dts
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Wormdingler board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + *
> + * SKU: 0x401 => 1025
> + * - bits 11..8: Panel ID: 0x4 (BOE)
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor-wormdingler.dtsi"
> +
> +/ {
> + model = "Google Wormdingler rev1+ (BOE, rt5682s)";
> + compatible = "google,wormdingler-sku1025", "qcom,sc7180";
> +};
> +
> +&panel {
> + compatible = "boe,tv110c9m-ll3";
> +};
> +
> +&alc5682 {
> + compatible = "realtek,rt5682s";
> + realtek,dmic1-clk-pin = <2>;
> + realtek,dmic-clk-rate-hz = <2048000>;
> +};
> +
> +&dsi_phy {
> + qcom,phy-rescode-offset-top = /bits/ 8 <31 31 31 31 (-32)>;
> + qcom,phy-rescode-offset-bot = /bits/ 8 <31 31 31 31 (-32)>;
> + qcom,phy-drive-ldo-level = <450>;> +};

This seems to have undone the changes I asked for previously. :(
Please go back to what you had in v5 where
`sc7180-trogdor-wormdingler-rev1-boe-rt5682s.dts` includes
`sc7180-trogdor-wormdingler-rev1-boe.dts` and just overrides the audio
stuff.

> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-inx-rt5682s.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-inx-rt5682s.dts
> new file mode 100644
> index 000000000000..1129e3fed165
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler-rev1-inx-rt5682s.dts
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Wormdingler board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + *
> + * SKU: 0x0001 => 1
> + * - bits 11..8: Panel ID: 0x0 (INX)
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor-wormdingler.dtsi"
> +
> +/ {
> + model = "Google Wormdingler rev1+ (INX, rt5682s)";
> + compatible = "google,wormdingler-sku1", "qcom,sc7180";
> +};
> +
> +&panel {
> + compatible = "innolux,hj110iz-01a";
> +};

This seems to have undone the changes I asked for previously. :(
Please go back to what you had in v5 where
`sc7180-trogdor-wormdingler-rev1-inx-rt5682s.dts` includes
`sc7180-trogdor-wormdingler-rev1-inx.dts` and just overrides the audio
stuff.

> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
> new file mode 100644
> index 000000000000..cced926364e5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Wormdingler board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +
> +ap_ec_spi: &spi6 {};
> +ap_h1_spi: &spi0 {};
> +
> +#include "sc7180-trogdor.dtsi"

Since you sent your v5, Stephen landed his "Simplify!" series. See commits:

19794489fa24 arm64: dts: qcom: Only include sc7180.dtsi in sc7180-trogdor.dtsi
d277cab7afc7 arm64: dts: qcom: sc7180-trogdor: Simplify spi0/spi6 labeling
51d30402be75 arm64: dts: qcom: sc7180-trogdor: Simplify trackpad enabling

Your series needs to take into account what Stephen did and do the
same thing with the new boards you're adding. That means you shouldn't
be doing thue weird spi6 / spi0 definitions here. It also means you
shouldn't be including "sc7180.dtsi" directly in the wormdingler file.
...and, of course, you'll need to see how the different trackpad
enabling applies to your files.

This is what I asked you do in my response to your v5:
https://lore.kernel.org/r/CAD=FV=Vkiqxv5QRiEqCjZVSNUH=K1N+JuYEsXu=XtFNxRtvttw@mail.gmail.com/