Re: [PATCH v10 4/4] dt-bindings: msm/dp: Add bindings of MSM DisplayPort controller

From: Stephen Boyd <swboyd@chromium.org>
To: khsieh@codeaurora.org
Cc: Krishna Manikandan <mkrishn@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, kalyan_t@codeaurora.org,
	tanmay@codeaurora.org, abhinavk@codeaurora.org,
	robdclark@gmail.com, bjorn.andersson@linaro.org,
	vinod.koul@linaro.org, rnayak@codeaurora.org,
	dianders@chromium.org, sibis@codeaurora.org,
	Vara Reddy <varar@codeaurora.org>
Subject: Re: [PATCH v10 4/4] dt-bindings: msm/dp: Add bindings of MSM DisplayPort controller
Date: Fri, 05 Feb 2021 14:38:48 -0800	[thread overview]
Message-ID: <161256472868.76967.11097632368706888024@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <b4bd2263a21f464869ac6d84aa0df131@codeaurora.org>

Quoting khsieh@codeaurora.org (2021-02-05 12:55:47)
> On 2021-02-05 11:02, Stephen Boyd wrote:
> > Quoting Krishna Manikandan (2021-02-03 22:42:19)
> >> Add bindings for Snapdragon DisplayPort controller driver.
> >> 
> >> Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>
> >> Signed-off-by: Vara Reddy <varar@codeaurora.org>
> >> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>
> >> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>

Krishna is sending these patches, so there should be an SoB line here
from Krishna. Please add it next time.

> >> 
> > [...]
> >> 
> >> Changes in v11:
> >> - add ports required of both #address-cells and  #size-cells
> >> - add required operating-points-v2
> >> - add required #sound-dai-cells
> >> - add required power-domains
> >> - update maintainer list
> > 
> > Where's that maintainer file update?
> I had deleted Tanmay.
> Anyway, I will removed Chandan at next upload.

Ok.

> > 
> >> ---
> >>  .../bindings/display/msm/dp-controller.yaml        | 157 
> >> +++++++++++++++++++++
> >>  .../bindings/display/msm/dpu-sc7180.yaml           |  10 ++
> >>  2 files changed, 167 insertions(+)
> >>  create mode 100644 
> >> Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> >> 
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml 
> >> b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> >> new file mode 100644
> >> index 0000000..2b71c7a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> >> @@ -0,0 +1,157 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
[...]
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +  - interrupts
> >> +  - clocks
> >> +  - clock-names
> >> +  - phys
> >> +  - phy-names
> >> +  - "#sound-dai-cells"
> >> +  - power-domains
> >> +  - ports
> >> +
> >> +additionalProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> >> +    #include <dt-bindings/clock/qcom,dispcc-sc7180.h>
> >> +    #include <dt-bindings/power/qcom-aoss-qmp.h>
> >> +    #include <dt-bindings/power/qcom-rpmpd.h>
> >> +
> >> +    mdss@ae00000 {
> > 
> > mdss is not a standard node name. Maybe 'subsystem'?
> Can you give me more details regrading this.
> I had double checked both ti/ti,am65x-dss.yaml and msm/dsi-phy.yaml and 
> can not find any clue for this.
> Can I just followed msm/dsi-phy.yaml for this?

Please look at section "2.2.2 Generic Names Recommendation" of the DT
spec[1] to find a proper name. I don't see 'subsystem' there but that
may be because it shouldn't really exist as a node. Instead we should
have the child nodes sit directly under the SoC node.

[1] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf