public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur
@ 2026-03-03  9:03 Abel Vesa
  2026-03-03  9:18 ` Johan Hovold
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Abel Vesa @ 2026-03-03  9:03 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Kuogee Hsieh, Abel Vesa
  Cc: Dmitry Baryshkov, Krzysztof Kozlowski, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, stable, Abel Vesa

The Glymur platform has four DisplayPort controllers. The hardware
supports four streams (MST) per controller. However, on Glymur the first
three controllers only have two streams wired to the display subsystem,
while the fourth controller operates in single-stream mode.

Add a dedicated clause for the Glymur compatible to require the register
ranges for all four stream blocks, while allowing either one pixel clock
(for the single-stream controller) or two pixel clocks (for the remaining
controllers).

Update the Glymur MDSS schema example by adding the missing p2, p3,
mst2link and mst3link register blocks. Without these, the bindings
validation fails. Also replace the made-up register addresses with the
actual addresses from the first controller to match the SoC devicetree
description.

Cc: stable@vger.kernel.org # v6.19
Fixes: 8f63bf908213 ("dt-bindings: display: msm: Document the Glymur DiplayPort controller")
Fixes: 1aee577bbc60 ("dt-bindings: display: msm: Document the Glymur Mobile Display SubSystem")
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
Did not pick up Dmitry's R-b tag as patches have been squashed
and commit message re-worded.
---
Changes in v4:
- Squashed so that it doesn't break bisectability, as
  suggested by Krzysztof.
- Link to v3: https://patch.msgid.link/20260302-glymur-fix-dp-bindings-reg-clocks-v3-0-8fe49ac1f556@oss.qualcomm.com

Changes in v3:
- Fixed the reg ranges in the example node in qcom,glymur-mdss.yaml as well.
- Link to v2: https://patch.msgid.link/20260302-glymur-fix-dp-bindings-reg-clocks-v2-0-e99b6f871e3b@oss.qualcomm.com

Changes in v2:
- mistakenly sent without cover subject line. Please ignore.
- Link to v1: https://patch.msgid.link/20260227-glymur-fix-dp-bindings-reg-clocks-v1-1-99f7b42b43aa@oss.qualcomm.com
---
 .../bindings/display/msm/dp-controller.yaml         | 21 ++++++++++++++++++++-
 .../bindings/display/msm/qcom,glymur-mdss.yaml      | 16 ++++++++++------
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index ebda78db87a6..02ddfaab5f56 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -253,7 +253,6 @@ allOf:
             enum:
               # these platforms support 2 streams MST on some interfaces,
               # others are SST only
-              - qcom,glymur-dp
               - qcom,sc8280xp-dp
               - qcom,x1e80100-dp
     then:
@@ -310,6 +309,26 @@ allOf:
           minItems: 6
           maxItems: 8
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              # these platforms support 2 streams MST on some interfaces,
+              # others are SST only, but all controllers have 4 ports
+              - qcom,glymur-dp
+    then:
+      properties:
+        reg:
+          minItems: 9
+          maxItems: 9
+        clocks:
+          minItems: 5
+          maxItems: 6
+        clocks-names:
+          minItems: 5
+          maxItems: 6
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml
index 2329ed96e6cb..64dde43373ac 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,glymur-mdss.yaml
@@ -176,13 +176,17 @@ examples:
                 };
             };
 
-            displayport-controller@ae90000 {
+            displayport-controller@af54000 {
                 compatible = "qcom,glymur-dp";
-                reg = <0xae90000 0x200>,
-                      <0xae90200 0x200>,
-                      <0xae90400 0x600>,
-                      <0xae91000 0x400>,
-                      <0xae91400 0x400>;
+                reg = <0xaf54000 0x200>,
+                      <0xaf54200 0x200>,
+                      <0xaf55000 0xc00>,
+                      <0xaf56000 0x400>,
+                      <0xaf57000 0x400>,
+                      <0xaf58000 0x400>,
+                      <0xaf59000 0x400>,
+                      <0xaf5a000 0x600>,
+                      <0xaf5b000 0x600>;
 
                 interrupt-parent = <&mdss>;
                 interrupts = <12>;

---
base-commit: 7c21b660e919698b10efa8bdb120f0f9bc3d3832
change-id: 20260227-glymur-fix-dp-bindings-reg-clocks-704d0ccbeef9

Best regards,
--  
Abel Vesa <abel.vesa@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur
  2026-03-03  9:03 [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur Abel Vesa
@ 2026-03-03  9:18 ` Johan Hovold
  2026-03-03 21:43 ` Claude review: " Claude Code Review Bot
  2026-03-03 21:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Johan Hovold @ 2026-03-03  9:18 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Kuogee Hsieh, Abel Vesa,
	Dmitry Baryshkov, Krzysztof Kozlowski, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, stable

On Tue, Mar 03, 2026 at 11:03:11AM +0200, Abel Vesa wrote:
> The Glymur platform has four DisplayPort controllers. The hardware
> supports four streams (MST) per controller. However, on Glymur the first
> three controllers only have two streams wired to the display subsystem,
> while the fourth controller operates in single-stream mode.
> 
> Add a dedicated clause for the Glymur compatible to require the register
> ranges for all four stream blocks, while allowing either one pixel clock
> (for the single-stream controller) or two pixel clocks (for the remaining
> controllers).
> 
> Update the Glymur MDSS schema example by adding the missing p2, p3,
> mst2link and mst3link register blocks. Without these, the bindings
> validation fails. Also replace the made-up register addresses with the
> actual addresses from the first controller to match the SoC devicetree
> description.
> 
> Cc: stable@vger.kernel.org # v6.19

No need to backport this, it's essentially just a documentation fix (not
a bug fix).

> Fixes: 8f63bf908213 ("dt-bindings: display: msm: Document the Glymur DiplayPort controller")
> Fixes: 1aee577bbc60 ("dt-bindings: display: msm: Document the Glymur Mobile Display SubSystem")
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

Johan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: dt-bindings: display: msm: Fix reg ranges and clocks on Glymur
  2026-03-03  9:03 [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur Abel Vesa
  2026-03-03  9:18 ` Johan Hovold
@ 2026-03-03 21:43 ` Claude Code Review Bot
  2026-03-03 21:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 21:43 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dt-bindings: display: msm: Fix reg ranges and clocks on Glymur
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>
Patches: 2
Reviewed: 2026-03-04T07:43:55.780213

---

This is a single-patch fix for the Glymur DisplayPort DT bindings. It addresses two issues introduced in v6.19: the DP controller binding had incorrect register range constraints (5 instead of 9) by grouping Glymur with platforms that have a different register layout, and the MDSS example used wrong/incomplete register addresses. The fix is straightforward and correct — it moves `qcom,glymur-dp` out of the `sc8280xp-dp`/`x1e80100-dp` group into its own clause requiring all 9 register blocks, and updates the example to match.

There is one minor issue to flag: the patch perpetuates a pre-existing typo (`clocks-names` vs. `clock-names`).

**Verdict**: Looks good overall. One minor issue worth fixing.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: dt-bindings: display: msm: Fix reg ranges and clocks on Glymur
  2026-03-03  9:03 [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur Abel Vesa
  2026-03-03  9:18 ` Johan Hovold
  2026-03-03 21:43 ` Claude review: " Claude Code Review Bot
@ 2026-03-03 21:43 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 21:43 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Commit message**: Clear and well-written. Good explanation of the hardware topology (four controllers, three with 2 streams, one SST). The `Fixes:` and `Cc: stable` tags are appropriate.

**dp-controller.yaml changes**:

The removal of `qcom,glymur-dp` from the existing 2-stream group and addition of a dedicated clause is the right approach. The new constraints match the hardware:
- `reg: minItems: 9, maxItems: 9` — correct for all 9 register blocks (ahb, aux, link, p0–p3, mst2link, mst3link), consistent with `qcom,sa8775p-dp` which has a similar 4-port topology.
- `clocks: minItems: 5, maxItems: 6` — correct for allowing SST (5 clocks) or 2-stream MST (6 clocks, with `stream_1_pixel`).

**Issue — `clocks-names` typo**:

```yaml
+        clocks-names:
+          minItems: 5
+          maxItems: 6
```

The property is defined as `clock-names` (line 101 of the file) and listed in the `required:` section as `clock-names` (line 184). The `clocks-names` used here (with the extra 's') doesn't match any defined property, so this constraint is silently ignored by the schema validator. This is a **pre-existing bug** — the same typo appears in all the other `allOf` clauses in this file — but since you're adding new code, this would be a good opportunity to use the correct name. At minimum it should be `clock-names:` in the new clause. Ideally a follow-up (or inclusion in this patch) would fix the other clauses too.

**qcom,glymur-mdss.yaml changes**:

The example update looks correct:

```yaml
-            displayport-controller@ae90000 {
+            displayport-controller@af54000 {
                 compatible = "qcom,glymur-dp";
-                reg = <0xae90000 0x200>,
-                      <0xae90200 0x200>,
-                      <0xae90400 0x600>,
-                      <0xae91000 0x400>,
-                      <0xae91400 0x400>;
+                reg = <0xaf54000 0x200>,
+                      <0xaf54200 0x200>,
+                      <0xaf55000 0xc00>,
+                      <0xaf56000 0x400>,
+                      <0xaf57000 0x400>,
+                      <0xaf58000 0x400>,
+                      <0xaf59000 0x400>,
+                      <0xaf5a000 0x600>,
+                      <0xaf5b000 0x600>;
```

- Addresses updated from made-up values to actual first-controller addresses.
- Link register block size corrected from `0x600` to `0xc00` (matching the sc7180 example and presumably the actual hardware).
- Four new register blocks added (p2, p3, mst2link, mst3link) to reach the required 9 entries.
- The `@` unit address in the node name matches the first reg entry. Good.

**Summary**: The fix is correct. The only issue is the `clocks-names` → `clock-names` typo that should be corrected before merging.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-03 21:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03  9:03 [PATCH v4] dt-bindings: display: msm: Fix reg ranges and clocks on Glymur Abel Vesa
2026-03-03  9:18 ` Johan Hovold
2026-03-03 21:43 ` Claude review: " Claude Code Review Bot
2026-03-03 21:43 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox