[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c94206c-f70f-4bed-81ec-bb2870748121@kernel.org>
Date: Tue, 22 Jul 2025 11:18:59 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Xiangxu Yin <xiangxu.yin@....qualcomm.com>,
Rob Clark <robin.clark@....qualcomm.com>, Dmitry Baryshkov
<lumag@...nel.org>, Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Kuogee Hsieh <quic_khsieh@...cinc.com>,
Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
dmitry.baryshkov@....qualcomm.com, konrad.dybcio@....qualcomm.com,
fange.zhang@....qualcomm.com, quic_lliu6@...cinc.com,
quic_yongmou@...cinc.com
Subject: Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615
standalone QMP DP PHY
On 22/07/2025 09:22, Xiangxu Yin wrote:
> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> designed to operate independently from the USB3 PHY.
A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>
> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> standalone and does not support USB/DP multiplexing. The binding
> describes the required clocks, resets, TCSR configuration, and clock/PHY
> cells for proper integration.
>
> Signed-off-by: Xiangxu Yin <xiangxu.yin@....qualcomm.com>
> ---
> .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml | 111 +++++++++++++++++++++
> 1 file changed, 111 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QMP PHY controller (DP, QCS615)
That's too vague title. You are not adding here Qualcomm QMP PHY
controllers.
> +
> +maintainers:
> + - Vinod Koul <vkoul@...nel.org>
Hm? Why?
> +
> +description:
> + The QMP DP PHY controller supports DisplayPort physical layer functionality
> + on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
> + support combo mode.
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,qcs615-qmp-dp-phy
> +
> + reg:
> + maxItems: 4
I don't understand what you are doing here. Why previous patch evolved
into this? Where is any reasoning for that in the changelog? You said:
"- Add new binding qcom,qcs615-qmp-dp-phy.yaml for QCS615 standalone DP
[Krzysztof]"
but you must say WHY you are doing things...
Anyway, missing constraints. Look at other Qualcomm bindings.
> +
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + items:
> + - const: cfg_ahb
> + - const: ref
> +
> + clock-output-names:
> + maxItems: 2
> + description:
> + Names of the clocks provided by the PHY.
Drop description, redundant. It cannot be anything else.
> +
> + qcom,tcsr-reg:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + - items:
> + - description: phandle to TCSR hardware block
> + - description: offset of the DP PHY moode register
> + description:
> + DP PHY moode register present in the TCSR
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + items:
> + - const: phy
Drop reset-names, useless.
> +
> + vdda-phy-supply: true
> +
> + vdda-pll-supply: true
> +
> + "#clock-cells":
> + const: 1
> + description:
> + See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> + "#phy-cells":
> + const: 1
> + description:
> + See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - clock-output-names
> + - qcom,tcsr-reg
> + - resets
> + - reset-names
> + - vdda-phy-supply
> + - vdda-pll-supply
> + - "#clock-cells"
> + - "#phy-cells"
> +
Why introducing completely different order? See existing binding and DTS
coding style.
Best regards,
Krzysztof
Powered by blists - more mailing lists