Re: [v1] dt-bindings: msm:disp: update dsi and dpu bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-02-04 23:52, Doug Anderson wrote:
Hi,

On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P <harigovi@xxxxxxxxxxxxxx> wrote:

Updating bindings of dsi and dpu by adding and removing certain
properties.

Signed-off-by: Harigovindan P <harigovi@xxxxxxxxxxxxxx>
---

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


Hi,

yaml files have been created and new patchset has been created for that.
https://patchwork.kernel.org/patch/11423653/



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux