On 7/22/2025 5:18 PM, Krzysztof Kozlowski wrote: > 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
Ok, will update subject in next patch. >> 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....@oss.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. Will update to 'Qualcomm QMP USB3-DP PHY controller (DP, QCS615)' in next patch. > >> + >> +maintainers: >> + - Vinod Koul <vk...@kernel.org> > Hm? Why? I referred to the definitions in qcom,msm8998-qmp-usb3-phy.yaml and qcom,sc8280xp-qmp-usb43dp-phy.yaml. I also found that Vinod Koul is listed as the maintainer of the GENERIC PHY FRAMEWORK in linux-next/MAINTAINERS. Did I misunderstand anything here? >> + >> +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. I misunderstood your earlier comment in '20241129-add-displayport-support-for-qcs615-platform-v1-2-09a4338d9...@quicinc.com' and mistakenly included "[Krzysztof]" in the commit message as if it were a quote. Apologies for the confusion and will drop this part in cover letter in next patch. In next patch, the address regions will be consolidated into a single range, similar to other QMP PHYs, and |maxItems| will be updated to |1|. > >> + >> + 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. Ok, will drop it. > >> + >> + 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. In next patch, this config will be updated to USB or DP PHY binding with two resets: 'phy_phy' for USB and 'dp_phy' for DP. Shall I keep 'reset-names' prop here? > >> + >> + 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 Ok, will update to follow the existing order. compatible reg clocks clock-names resets reset-names vdda-phy-supply vdda-pll-supply #clock-cells #phy-cells qcom,tcsr-reg
