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

On Tue, Feb 04, 2020 at 07:45:37PM +0530, Harigovindan P wrote:

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

Yes, the diff tells me that. The commit message should say why.

This change breaks compatibility as well.

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"

You can't just add new clocks...

• "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.
  • "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>.

• clocks: Phandles to device clocks.
• clock-names: the following clocks are required:
• • "mdp_core"
  • "iface"
  • "bus"
• • "core_mmss"
  • "byte"
  • "pixel"
  • "core"

@@ -156,7 +153,6 @@ Example: "core", "core_mmss", "iface",

• 	"mdp_core",
  "pixel";
  

  clocks = <&mmcc MDSS_AXI_CLK>,

@@ -164,7 +160,6 @@ Example: <&mmcc MDSS_ESC0_CLK>, <&mmcc MMSS_MISC_AHB_CLK>, <&mmcc MDSS_AHB_CLK>,

• 	<&mmcc MDSS_MDP_CLK>,
  <&mmcc MDSS_PCLK0_CLK>;
  
  

  assigned-clocks =

-- 2.7.4