[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD=FV=UcLs+FkPUeDUC2c=zg8e_MTFHB7=yiz-=W6L6y2+H4+A@mail.gmail.com>
Date: Mon, 13 May 2019 15:47:59 -0700
From: Doug Anderson <dianders@...omium.org>
To: Rob Clark <robdclark@...il.com>
Cc: linux-arm-msm <linux-arm-msm@...r.kernel.org>,
Rob Clark <robdclark@...omium.org>,
Sibi Sankar <sibis@...eaurora.org>,
Evan Green <evgreen@...omium.org>,
Matthias Kaehlcke <mka@...omium.org>,
Abhinav Kumar <abhinavk@...eaurora.org>,
Brian Norris <briannorris@...omium.org>,
Venkat Gopalakrishnan <venkatg@...eaurora.org>,
Rajendra Nayak <rnayak@...eaurora.org>,
Andy Gross <andy.gross@...aro.org>,
David Brown <david.brown@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
devicetree@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Stephen Boyd <swboyd@...omium.org>
Subject: Re: [RFC 1/3] arm64: dts: qcom: sdm845-cheza: add initial cheza dt
Hi,
On Thu, May 9, 2019 at 11:44 AM Rob Clark <robdclark@...il.com>wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> This is essentialy a squash of a bunch of history of cheza dt updates
> from chromium kernel, some of which were themselves squashes of history
> from older chromium kernels.
>
> I don't claim any credit other than wanting to more easily boot upstream
> kernel on cheza to have an easier way to test upstream driver work ;-)
>
> I've added below in Cc tags all the original actual authors (apologies
> if I missed any).
>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Sibi Sankar <sibis@...eaurora.org>
> Cc: Evan Green <evgreen@...omium.org>
> Cc: Matthias Kaehlcke <mka@...omium.org>
> Cc: Abhinav Kumar <abhinavk@...eaurora.org>
> Cc: Brian Norris <briannorris@...omium.org>
> Cc: Venkat Gopalakrishnan <venkatg@...eaurora.org>
> Cc: Rajendra Nayak <rnayak@...eaurora.org>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 3 +
> arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts | 238 ++++
> arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts | 238 ++++
> arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts | 180 +++
> arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 1317 ++++++++++++++++++
> 5 files changed, 1976 insertions(+)
In general I am supportive of getting cheza dts landed upstream.
One question is how much cleanup we want to do while landing upstream.
We have a few TODO / HACK comments currently. Do we want to land what
we have and clean these up in separate commits, or should we try to do
cleanup beforehand. Bjron / Andy: do you have any opinions? I've
commented on a few below.
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index b3fe72ff2955..7a038cf81543 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -8,6 +8,9 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8994-angler-rev-101.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8996-mtp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8998-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r1.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
Something about the above makes "git am" unhappy and "patch -p1" think
this is a malformed patch. When I delete the part touching the
Makefile then I can apply OK.
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts b/arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts
> new file mode 100644
> index 000000000000..35bb4629edd4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza-r1.dts
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Cheza board device tree source
> + *
> + * Copyright 2018 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sdm845-cheza.dtsi"
> +
> +/ {
> + model = "Google Cheza (rev1)";
> + compatible = "google,cheza-rev1", "google,cheza", "qcom,sdm845";
See below for rev 3, but I think this might be better as:
compatible = "google,cheza-rev1", "qcom,sdm845";
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts b/arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts
> new file mode 100644
> index 000000000000..4359a82d4bb4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza-r2.dts
> @@ -0,0 +1,238 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Cheza board device tree source
> + *
> + * Copyright 2018 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sdm845-cheza.dtsi"
> +
> +/ {
> + model = "Google Cheza (rev2)";
> + compatible = "google,cheza-rev2", "google,cheza", "qcom,sdm845";
See above and below, but probably:
compatible = "google,cheza-rev2", "qcom,sdm845";
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts b/arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts
> new file mode 100644
> index 000000000000..2c3f815b90c1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza-r3.dts
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Cheza board device tree source
> + *
> + * Copyright 2018 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sdm845-cheza.dtsi"
> +
> +/ {
> + model = "Google Cheza (rev3+)";
> + compatible = "google,cheza-rev15", "google,cheza-rev14",
> + "google,cheza-rev13", "google,cheza-rev12",
> + "google,cheza-rev11", "google,cheza-rev10",
> + "google,cheza-rev9", "google,cheza-rev8",
> + "google,cheza-rev7", "google,cheza-rev6",
> + "google,cheza-rev5", "google,cheza-rev4",
> + "google,cheza-rev3", "google,cheza", "qcom,sdm845";
Julius Werner suggested that a better solution is that the newest dts
should specify no revision. Thus this should be just:
compatible = "google,cheza", "qcom,sdm845";
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> new file mode 100644
> index 000000000000..338c92ddd1c3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> @@ -0,0 +1,1317 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Cheza device tree source (common between revisions)
> + *
> + * Copyright 2018 Google LLC.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include "sdm845.dtsi"
> +
> +/* PMICs depend on spmi_bus label and so must come after SoC */
> +#include "pm8005.dtsi"
> +#include "pm8998.dtsi"
> +
> +/ {
> + aliases {
> + bluetooth0 = &bluetooth;
> + hsuart0 = &uart6;
> + serial0 = &uart9;
> + wifi0 = &wifi;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&cros_ec_pwm 0>;
> + enable-gpios = <&tlmm 37 GPIO_ACTIVE_HIGH>;
> + power-supply = <&ppvar_sys>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&ap_edp_bklten>;
> + };
> +
> + reserved-memory {
> + rmtfs@...00000 {
> + compatible = "qcom,rmtfs-mem";
> + reg = <0x0 0x88f00000 0x0 0x800000>;
> + no-map;
> +
> + qcom,client-id = <1>;
> + };
> + };
You missed part of:
https://crrev.com/c/1588964 - CHROMIUM: arm64: dts: qcom:
sdm845-cheza: Temporarily delete reserved-mem changes
...that should make the above "reserved-memory" section go away.
> +&spi0 {
> + status = "okay";
> + spi-max-frequency = <3000000>; /* TODO: Drop this? */
Drop "spi-max-frequency". That was part of original bindings but
shouldn't be there anymore.
> +};
> +
> +&spi5 {
> + status = "okay";
> + spi-max-frequency = <800000>; /* TODO: Drop this */
> + cr50@0 {
> + compatible = "google,cr50";
> + reg = <0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&h1_ap_int_odl>;
> + spi-max-frequency = <800000>;
> + interrupt-parent = <&tlmm>;
> + interrupts = <129 IRQ_TYPE_EDGE_RISING>;
> + };
> +};
Presumably the above needs to be dropped since "google,cr50" isn't
actually upstream.
> +&spi10 {
> + status = "okay";
> + spi-max-frequency = <3000000>; /* TODO: Drop this? */
Drop "spi-max-frequency". That was part of original bindings but
shouldn't be there anymore.
-Doug
Powered by blists - more mailing lists