Hi,
On Thu, Oct 28, 2021 at 3:11 PM Philip Chen <philipchen@xxxxxxxxxxxx> wrote:
>
> Support Lazor/Limozeen rev9 board where Parade ps8640 is added as the
> second source edp bridge.
It probably wouldn't hurt to at least mention in the commit message
why the include of "sc7180.dtsi" was moved out of
"sc7180-trogdor-lazor.dtsi" so that people reviewing the change aren't
confused. Otherwise it just looks arbitrary.
> Signed-off-by: Philip Chen <philipchen@xxxxxxxxxxxx>
> ...
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r5.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r5.dts
> new file mode 100644
> index 000000000000..a2a6e2515ee6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r5.dts
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor Limozeen board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-trogdor-lte-sku.dtsi"
> +
> +/ {
> + model = "Google Lazor Limozeen without Touchscreen (rev5 - rev8)";
> + /* No sku5 post-rev5 */
> + compatible = "google,lazor-rev5-sku6", "google,lazor-rev6-sku6",
> + "google,lazor-rev7-sku6", "google,lazor-rev8-sku6",
> + "google,rev5-sku5", "qcom,sc7180";
Two issues.
1. "google,rev5-sku5" should probably be "google,lazor-rev5-sku5"
2. rev5-sku5 should sort to the start of the list, not to the end.
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-nots-r9.dts
> @@ -2,16 +2,18 @@
> /*
> * Google Lazor Limozeen board device tree source
> *
> - * Copyright 2020 Google LLC.
> + * Copyright 2021 Google LLC.
> */
>
> /dts-v1/;
>
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> #include "sc7180-trogdor-lazor.dtsi"
> #include "sc7180-trogdor-lte-sku.dtsi"
>
> / {
> - model = "Google Lazor Limozeen without Touchscreen";
> + model = "Google Lazor Limozeen without Touchscreen (rev9+)";
> compatible = "google,lazor-sku6", "google,lazor-sku5", "qcom,sc7180";
Earlier you said "sku5" doesn't exist past -rev5. So it doesn't need
to be here, right?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r4.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r4.dts
> new file mode 100644
> index 000000000000..10f48f4b429a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-limozeen-r4.dts
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor Limozeen board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-trogdor-lte-sku.dtsi"
> +
> +/ {
> + model = "Google Lazor Limozeen (rev4 - rev8)";
> + compatible = "google,lazor-rev4-sku4", "google,lazor-rev5-sku4",
> + "google,lazor-rev6-sku4", "google,lazor-rev6-sku4",
-rev6 is listed twice and -rev7 is missing.
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> new file mode 100644
> index 000000000000..326ba9f88394
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-kb.dts
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor board device tree source
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-lite.dtsi"
> +
> +/ {
> + model = "Google Lazor (rev9+) with KB Backlight";
> + compatible = "google,lazor-sku2", "qcom,sc7180";
> +};
> +
> +&keyboard_backlight {
> + status = "okay";
> +};
> +
> +&panel {
> + compatible = "boe,nv133fhm-n62";
> +};
Why do you need the "panel" here?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
> new file mode 100644
> index 000000000000..a705c956f62b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9-lte.dts
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-trogdor-lte-sku.dtsi"
> +
> +/ {
> + model = "Google Lazor (rev9+) with LTE";
> + compatible = "google,lazor-sku0", "qcom,sc7180";
> +};
> +
> +&ap_sar_sensor {
> + status = "okay";
> +};
> +
> +&ap_sar_sensor_i2c {
> + status = "okay";
> +};
> +
> +&keyboard_backlight {
> + status = "okay";
> +};
> +
> +&panel {
> + compatible = "boe,nv133fhm-n62";
> +};
Why do you need the "panel" hre?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts
> new file mode 100644
> index 000000000000..13ff9a851613
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor-r9.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Lazor board device tree source
> + *
> + * Copyright 2021 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +#include "sc7180-trogdor-parade-ps8640.dtsi"
> +#include "sc7180-trogdor-lazor.dtsi"
> +#include "sc7180-lite.dtsi"
> +
> +/ {
> + model = "Google Lazor (rev9+)";
> + compatible = "google,lazor", "qcom,sc7180";
> +};
> +
> +&panel {
> + compatible = "boe,nv133fhm-n62";
> +};
Why do you need the "panel" here?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index faf8b807d0ff..286d3594b6ae 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -11,6 +11,7 @@
> #include <dt-bindings/clock/qcom,lpasscorecc-sc7180.h>
> #include <dt-bindings/clock/qcom,rpmh.h>
> #include <dt-bindings/clock/qcom,videocc-sc7180.h>
> +#include <dt-bindings/gpio/gpio.h>
I don't think you should be adding this to sc7180.dtsi. If you need it
in the bridge chip "fragments" you should add the includes to those.
-Doug
