LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Rob Clark <robdclark@gmail.com>
Cc: linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Rob Clark <robdclark@chromium.org>,
	Sibi Sankar <sibis@codeaurora.org>,
	Evan Green <evgreen@chromium.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Abhinav Kumar <abhinavk@codeaurora.org>,
	Brian Norris <briannorris@chromium.org>,
	Venkat Gopalakrishnan <venkatg@codeaurora.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Andy Gross <andy.gross@linaro.org>,
	David Brown <david.brown@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Stephen Boyd <swboyd@chromium.org>
Subject: Re: [RFC 1/3] arm64: dts: qcom: sdm845-cheza: add initial cheza dt
Date: Mon, 13 May 2019 15:47:59 -0700
Message-ID: <CAD=FV=UcLs+FkPUeDUC2c=zg8e_MTFHB7=yiz-=W6L6y2+H4+A@mail.gmail.com> (raw)
In-Reply-To: <20190509184415.11592-2-robdclark@gmail.com>

Hi,

On Thu, May 9, 2019 at 11:44 AM Rob Clark <robdclark@gmail.com>wrote:

> From: Rob Clark <robdclark@chromium.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@chromium.org>
> Cc: Sibi Sankar <sibis@codeaurora.org>
> Cc: Evan Green <evgreen@chromium.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Abhinav Kumar <abhinavk@codeaurora.org>
> Cc: Brian Norris <briannorris@chromium.org>
> Cc: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Cc: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Rob Clark <robdclark@chromium.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@88f00000 {
> +                       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

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190509184415.11592-1-robdclark@gmail.com>
2019-05-09 18:44 ` Rob Clark
2019-05-13 22:47   ` Doug Anderson [this message]
2019-05-09 18:44 ` [RFC 2/3] arm64: dts: qcom: sdm845-cheza: Re-add reserved memory Rob Clark
2019-05-13 22:48   ` Doug Anderson
2019-05-15  4:09     ` Rob Clark
2019-05-15 19:24       ` Jordan Crouse
2019-05-15 21:50       ` Doug Anderson
2019-05-15 23:41         ` Bjorn Andersson
2019-05-09 18:44 ` [RFC 3/3] arm64: dts: qcom: sdm845-cheza: delete zap-shader Rob Clark
2019-05-15 21:43   ` Doug Anderson

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAD=FV=UcLs+FkPUeDUC2c=zg8e_MTFHB7=yiz-=W6L6y2+H4+A@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=abhinavk@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=briannorris@chromium.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=evgreen@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mka@chromium.org \
    --cc=rnayak@codeaurora.org \
    --cc=robdclark@chromium.org \
    --cc=robdclark@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sibis@codeaurora.org \
    --cc=swboyd@chromium.org \
    --cc=venkatg@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox