From: Doug Anderson <dianders@chromium.org>
To: Harigovindan P <harigovi@codeaurora.org>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
dri-devel <dri-devel@lists.freedesktop.org>,
Sean Paul <seanpaul@chromium.org>,
Kalyan Thota <kalyan_t@codeaurora.org>,
"Kristian H. Kristensen" <hoegsberg@chromium.org>,
freedreno <freedreno@lists.freedesktop.org>
Subject: Re: [v1] dt-bindings: msm:disp: update dsi and dpu bindings
Date: Tue, 4 Feb 2020 10:22:09 -0800 [thread overview]
Message-ID: <CAD=FV=XXyYTqVV4=e8Kz0tYQ=5TWjZi2QETNL_0BaFqKi5o0Cg@mail.gmail.com> (raw)
In-Reply-To: <1580825737-27189-1-git-send-email-harigovi@codeaurora.org>
Hi,
On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P <harigovi@codeaurora.org> wrote:
>
> Updating bindings of dsi and dpu by adding and removing certain
> properties.
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
>
> Changes in v1:
> - Adding "ahb" clock as a required property.
> - Adding "bus", "rot", "lut" as optional properties for sc7180 device.
> - Removing properties from dsi bindings that are unused.
> - Removing power-domain property since DSI is the child node of MDSS
> and it will inherit supply from its parent.
>
> Documentation/devicetree/bindings/display/msm/dpu.txt | 7 +++++++
> Documentation/devicetree/bindings/display/msm/dsi.txt | 5 -----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
> index 551ae26..dd58472a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt
> @@ -19,6 +19,7 @@ Required properties:
> The following clocks are required:
> * "iface"
> * "bus"
> + * "ahb"
This is only required for sc7180? ...or old SoCs should have had it
all along too?
> * "core"
> - interrupts: interrupt signal from MDSS.
> - interrupt-controller: identifies the node as an interrupt controller.
> @@ -50,6 +51,8 @@ Required properties:
> - clock-names: device clock names, must be in same order as clocks property.
> The following clocks are required.
> * "bus"
> + For the device "qcom,sc7180-dpu":
> + * "bus" - is an optional property due to architecture change.
This is a really odd way to write it for two reasons:
* You're breaking up the flow of the list.
* This shouldn't be listed as "optional" in sc7180 but unless there is
some reason to ever provide it on sc7180. It should simply be
disallowed.
Maybe instead just:
The following clocks are required.
- * "bus"
+ * "bus" (anything other than qcom,sc7180-dpu)
We really need to get this into yaml ASAP but that'd probably be OK to
tide us over.
NOTE: when converting to yaml, ideally we'll have a separate file per
SoC to avoid crazy spaghetti, see commit 2a8aa18c1131 ("dt-bindings:
clk: qcom: Fix self-validation, split, and clean cruft") in clk-next
for an example of starting the transition to one yaml per SoC (at
least for anything majorly different).
> * "iface"
> * "core"
> * "vsync"
> @@ -70,6 +73,10 @@ Optional properties:
> - assigned-clocks: list of clock specifiers for clocks needing rate assignment
> - assigned-clock-rates: list of clock frequencies sorted in the same order as
> the assigned-clocks property.
> +- For the device "qcom,sc7180-dpu":
> + clock-names: optional device clocks, needed for accessing LUT blocks.
> + * "rot"
> + * "lut"
>
> Example:
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt b/Documentation/devicetree/bindings/display/msm/dsi.txt
> index af95586..61d659a 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
> @@ -8,13 +8,10 @@ Required properties:
> - reg-names: The names of register regions. The following regions are required:
> * "dsi_ctrl"
> - interrupts: The interrupt signal from the DSI block.
> -- power-domains: Should be <&mmcc MDSS_GDSC>.
Is this supposed to be removed from all SoCs using this bindings, or
just yours?
I'll also note that you left it in the "Example:" below.
> - clocks: Phandles to device clocks.
> - clock-names: the following clocks are required:
> - * "mdp_core"
> * "iface"
> * "bus"
> - * "core_mmss"
As Jeffrey pointed out, you shouldn't be removing these from old SoCs.
In "drivers/gpu/drm/msm/dsi/dsi_cfg.c" you can clearly see them used.
Maybe it's time for you to do the yaml conversion and handle this
correctly per-SoC.
-Doug
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-02-04 18:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-04 14:15 [v1] dt-bindings: msm:disp: update dsi and dpu bindings Harigovindan P
2020-02-04 14:44 ` [Freedreno] " Jeffrey Hugo
2020-02-04 18:22 ` Doug Anderson [this message]
2020-03-06 11:41 ` harigovi
2020-02-06 19:13 ` Rob Herring
Reply instructions:
You may reply publicly 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=XXyYTqVV4=e8Kz0tYQ=5TWjZi2QETNL_0BaFqKi5o0Cg@mail.gmail.com' \
--to=dianders@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=harigovi@codeaurora.org \
--cc=hoegsberg@chromium.org \
--cc=kalyan_t@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=seanpaul@chromium.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).