Re: [PATCH 1/2] dt-bindings: display: add CIX Sky1 DisplayPort transmitter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [Medium] dt-bindings: display: cix,sky1-dptx: missing output port and
  required phys
- [Low] dt-bindings: display: cix,sky1-dptx: non-standard
  cix,dp-max-rate property
- [Low] dt-bindings: display: cix,sky1-dptx: instance index properties
  in binding
- [Low] dt-bindings: display: cix,sky1-dptx: unused dp0 label in DTS
  example
--

commit 8c60d6c22ca4c6d5157c09f9c5eab3f993001d1b
Author: Luo Liu <Luo.Liu@cixtech.com>

dt-bindings: display: add CIX Sky1 DisplayPort transmitter

This commit adds the Devicetree binding schema and MAINTAINERS entry for
the CIX Sky1 DisplayPort transmitter controller.

> diff --git a/Documentation/devicetree/bindings/display/cix,sky1-dptx.yaml b/Documentation/devicetree/bindings/display/cix,sky1-dptx.yaml
> new file mode 100644
> index 0000000000000..9087b5edd9f02
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/cix,sky1-dptx.yaml
> @@ -0,0 +1,108 @@

[ ... ]

> +  phys:
> +    maxItems: 1
> +    description: DisplayPort PHY the controller transmits through.
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description:
> +      Video input from the display pipeline that drives this controller.

[Severity: Medium]
Should this binding define ports with both an input port from the display
pipeline and an output port to the downstream DisplayPort connector, eDP
panel, or Type-C bridge, and mark phys and ports as required?

With only a single input port and additionalProperties: false, board DTS
files that connect the transmitter output to a connector or panel via
ports will fail dtbs_check validation. In addition, cix_dp_parse_dt()
unconditionally calls devm_phy_get(dev, NULL) and fails probe if phys is
absent, yet phys and the OF graph port are omitted from both required and
the example.

> +
> +  cix,adapter-port:
> +    description:
> +      RCSU video adapter port of this transmitter. Sky1 numbers its five
> +      transmitters from 1 to 5. Port 3 is the native DP/eDP output.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 5
> +
> +  cix,dp-max-rate:
> +    description:
> +      Maximum link rate the board is wired for, in units of 10 kbit/s.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [162000, 270000, 540000, 810000]
> +    default: 810000

[Severity: Low]
Could this use the standard link-frequencies property from
video-interfaces.yaml on the output endpoint instead of a custom
cix,dp-max-rate property?

The values 162000, 270000, 540000, and 810000 in 10 kbit/s units come
from Linux DRM's internal drm_dp_bw_code_to_link_rate() representation and
lack a standard unit suffix, whereas DisplayPort bindings normally specify
board-supported link rates in Hz via link-frequencies on the output
endpoint.

> +
> +  cix,stream-id:
> +    description:
> +      Index of the video stream this controller uses. Sky1 has two streams,
> +      numbered 0 and 1.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 1
> +    default: 0

[Severity: Low]
Should cix,adapter-port and cix,stream-id be avoided in favor of
describing hardware differences via compatible strings or OF graph
endpoints?

In cix_dp_cfg_adapter(), each instance already maps its own rcsu region
from reg, and cix,adapter-port is only checked against port 3 because the
native DP/eDP port uses a different RCSU register layout (offset 0x320)
than the USB-C DP ports (offsets 0x300 and 0x304). Similarly,
cix,stream-id encodes the display pipeline stream index as a custom vendor
property rather than representing the stream routing through OF graph
ports or endpoints.

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    dp0: dp@14064000 {

[Severity: Low]
Can the unreferenced dp0 label be dropped from the example?  It is never
referenced within the example and triggers an unused-labels warning from
scripts/dtc/dt-check-style in strict mode.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109103010.110911-1-Luo.Liu@cixtech.com?part=1