Message ID | 20221125092749.46073-1-luca.weiss@fairphone.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp3880116wrr; Fri, 25 Nov 2022 01:29:28 -0800 (PST) X-Google-Smtp-Source: AA0mqf6g/mD3jl61zhTM00b/hSvn7707gQLL1uB2sdjM+SOwLq6RFmLnsoQ/SPpGb4t20IfD3DMH X-Received: by 2002:a17:902:ba89:b0:189:385f:b7f5 with SMTP id k9-20020a170902ba8900b00189385fb7f5mr14974088pls.69.1669368568514; Fri, 25 Nov 2022 01:29:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669368568; cv=none; d=google.com; s=arc-20160816; b=TZFIWgVWlxn3WSD1SftUKFZrbP2/mxmM4B8Np7/7BwlZ8x2hKlr//xSMU3fe+0us3N 1fVFY/+F1MHFvJx4Bj8P8dinAu3DwwXcb1N21yvTF0/1FV+HR0Ugg2M1+sj+OrLRa+PO kyKw3hkzPhqIeFVlxFeUAzx/HlxtdiNvtVz+i/G/9yzWd5Occf79k9Q90mggNwdDlDyr 3vJxbcRAHg6Hm5Qd4a2UE/OpoOZoUU3xXCwjBprk/H6aJQGkKX56YjMSR66/BESpLRm/ qHj8NcfzqB+BOZ/MFDdLhqhdM3+R6fLcym1Sp2RdDFgJ3geWEP06KBBNiZ0ovOoOb8IM Cdlg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=mpP1Fb3xrzGRgQcSv/nFJciJPCBP7tqCxTn34nBqums=; b=D8e3Id3DNWF+J/oF0obE8OrBwtfm4VGbttUCXT8dujTKw5pLFHkujfSqfARetiFycR QpUfl5v5eLLhKKc8Fsb4TSYO7Gqf5PoZC5au0E8ETZxmp+Zg98z+y3ax4pCFe59l6ROs /7O/TvbGZEMU/bJHr1iYZi0/sQEnyatVnSsO13zQHhiXv8X0Tea45VXcUKs429JkX5f0 KizudQaiUQdFmAGTecwzvZbC31ZbYoUUYaKkf+6z2jcgbNjqsbA2ArSGeFr7QQCL+Ys/ BFoRH3y2JPA5X/P1GCflFnZyRXbQmHqwh+Y0JJ1tZ5DemOL6PthChu9ijflASzzcH76t vtaw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@fairphone.com header.s=fair header.b=M3AG39iy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=fairphone.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id o15-20020a170902d4cf00b001871d062940si3645716plg.400.2022.11.25.01.29.15; Fri, 25 Nov 2022 01:29:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@fairphone.com header.s=fair header.b=M3AG39iy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=fairphone.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229992AbiKYJ3A (ORCPT <rfc822;zxc52fgh@gmail.com> + 99 others); Fri, 25 Nov 2022 04:29:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44984 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229767AbiKYJ2j (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 25 Nov 2022 04:28:39 -0500 Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9A2663AC0A for <linux-kernel@vger.kernel.org>; Fri, 25 Nov 2022 01:28:37 -0800 (PST) Received: by mail-ej1-x635.google.com with SMTP id e27so8949820ejc.12 for <linux-kernel@vger.kernel.org>; Fri, 25 Nov 2022 01:28:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fairphone.com; s=fair; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=mpP1Fb3xrzGRgQcSv/nFJciJPCBP7tqCxTn34nBqums=; b=M3AG39iyzVs6kp0nC+H6PpKMBHKps2QdCUkARwCOOsrhaQzvV03csXlDQrasZJfxXq 5++2hBLuysDfDyTuchXNvhK0/xi/Itvpo6NplBuZk7h6f+vuMyT2zIryoZu4RDAjpH9u 1kG8hhmksgxiGXsd1m8NOYhDz6dv/mVQbu/+mILE1rjce+4lB6agNnAWS4RYUHNpEZZ2 YfMq7mLnGCP1mG1Z8uAYt5dy87jP+gpppV548B0WAKVeVImMIXjTJC26xRJyrzY5jdC4 6vw4cdnVVsBX+MO9sczMwjyzNDYHPZFxsHGOZhipN926gnKVQc0Oy/o1+msxlp0ixOLj odMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=mpP1Fb3xrzGRgQcSv/nFJciJPCBP7tqCxTn34nBqums=; b=0RdDsJXOnqfBCO9rn4sr5b0mNAWKqD3n53uStph49UxmWD2uWalKUJ5OtAMj0s4VIb WmzMDc3i4orpZ2OKdBZjUe93Xdiin4m+9cKVqgPglZPOYaiTQP5tvl5h5tbPkzWYb1w9 RrY+UrutQCmIBq+Wz4v28bqdezFngY7t7QkiXJ5SIJfr41/kmCuNphMcdlK11I3vj0FI PHCcRJzjnlzxiHKoSJCvx5HpBAIGEZdtAbGKRmyjwR4Fx5eb5S5RtIfY2ifWln6PwFyb TQbKSQ0dkkbukikMyt82oiBuXYozkuxh1wCPqKblUkwkKVIlz7UAL4XyS2fSfc8SKCZH czOg== X-Gm-Message-State: ANoB5pmLaByOc1N/UL65VN/nxRaHB3193qRs/s3WB95VYdjKh0jTixQ8 e+65spOkF1KPMgDTYDq1B3WUVA== X-Received: by 2002:a17:906:d281:b0:782:7790:f132 with SMTP id ay1-20020a170906d28100b007827790f132mr15514892ejb.649.1669368516112; Fri, 25 Nov 2022 01:28:36 -0800 (PST) Received: from otso.. (144-178-202-138.static.ef-service.nl. [144.178.202.138]) by smtp.gmail.com with ESMTPSA id q10-20020a170906b28a00b007b47748d22fsm1329315ejz.220.2022.11.25.01.28.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 01:28:35 -0800 (PST) From: Luca Weiss <luca.weiss@fairphone.com> To: linux-arm-msm@vger.kernel.org, Johan Hovold <johan@kernel.org> Cc: ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org, Luca Weiss <luca.weiss@fairphone.com>, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Vinod Koul <vkoul@kernel.org>, Kishon Vijay Abraham I <kishon@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, linux-phy@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible Date: Fri, 25 Nov 2022 10:27:47 +0100 Message-Id: <20221125092749.46073-1-luca.weiss@fairphone.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750459816192916395?= X-GMAIL-MSGID: =?utf-8?q?1750459816192916395?= |
Series |
[RFC,v2,1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible
|
|
Commit Message
Luca Weiss
Nov. 25, 2022, 9:27 a.m. UTC
Add the compatible describing the combo phy found on SM6350.
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, I've sent this v2 as RFC because there are several things
where I have questions on how it should be done.
In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
ssusb@a600000 node, or should I also add it to qmpphy?
.../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote: > Add the compatible describing the combo phy found on SM6350. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > @Johan Hovold, I've sent this v2 as RFC because there are several things > where I have questions on how it should be done. > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the > ssusb@a600000 node, or should I also add it to qmpphy? Yeah, you may need to add a platform specific section of the clocks, which appear to be different, even if I'm not sure they are currently described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are they named in the vendor's dts? It should be OK to include the power-domain also for the PHY node. > .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > index 6f31693d9868..3e39e3e0504d 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > @@ -17,16 +17,18 @@ properties: > compatible: > enum: > - qcom,sc8280xp-qmp-usb43dp-phy > + - qcom,sm6350-qmp-usb3-dp-phy > > reg: > maxItems: 1 > > clocks: > - maxItems: 4 > + maxItems: 5 > > clock-names: > items: > - const: aux > + - const: cfg_ahb > - const: ref > - const: com_aux > - const: usb3_pipe So this would need to be moved to an allOf: construct at the end with one section each for sc8280xp and sm6350. > @@ -61,7 +63,6 @@ required: > - reg > - clocks > - clock-names > - - power-domains > - resets > - reset-names > - vdda-phy-supply Johan
Hi Johan, On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote: > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote: > > Add the compatible describing the combo phy found on SM6350. > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > --- > > @Johan Hovold, I've sent this v2 as RFC because there are several things > > where I have questions on how it should be done. > > > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the > > ssusb@a600000 node, or should I also add it to qmpphy? > > Yeah, you may need to add a platform specific section of the clocks, > which appear to be different, even if I'm not sure they are currently > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are > they named in the vendor's dts? This is the msm-4.19 dts: https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354 > > It should be OK to include the power-domain also for the PHY node. Ack. > > > .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > index 6f31693d9868..3e39e3e0504d 100644 > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > @@ -17,16 +17,18 @@ properties: > > compatible: > > enum: > > - qcom,sc8280xp-qmp-usb43dp-phy > > + - qcom,sm6350-qmp-usb3-dp-phy > > > > reg: > > maxItems: 1 > > > > clocks: > > - maxItems: 4 > > + maxItems: 5 > > > > clock-names: > > items: > > - const: aux > > + - const: cfg_ahb > > - const: ref > > - const: com_aux > > - const: usb3_pipe > > So this would need to be moved to an allOf: construct at the end with > one section each for sc8280xp and sm6350. Ack. Thanks for the quick response! Regards, Luca > > > @@ -61,7 +63,6 @@ required: > > - reg > > - clocks > > - clock-names > > - - power-domains > > - resets > > - reset-names > > - vdda-phy-supply > > Johan
On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote: > Hi Johan, > > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote: > > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote: > > > Add the compatible describing the combo phy found on SM6350. > > > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > > --- > > > @Johan Hovold, I've sent this v2 as RFC because there are several things > > > where I have questions on how it should be done. > > > > > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains > > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the > > > ssusb@a600000 node, or should I also add it to qmpphy? > > > > Yeah, you may need to add a platform specific section of the clocks, > > which appear to be different, even if I'm not sure they are currently > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are > > they named in the vendor's dts? > > This is the msm-4.19 dts: > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354 clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>, <&rpmhcc RPMH_QLINK_CLK>, <&gcc GCC_USB3_PRIM_CLKREF_CLK>, <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>; clock-names = "aux_clk", "pipe_clk", "ref_clk_src", "ref_clk", "com_aux_clk"; So it looks like you don't need update the binding for the clocks as the above matches sc8280xp: aux ref com_aux usb3_pipe Parent clocks (ref_clk_src) should not be included in the binding, but rather be handled by the clock driver. For example, see: https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/ https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/ > > > .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > index 6f31693d9868..3e39e3e0504d 100644 > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > @@ -17,16 +17,18 @@ properties: > > > compatible: > > > enum: > > > - qcom,sc8280xp-qmp-usb43dp-phy > > > + - qcom,sm6350-qmp-usb3-dp-phy > > > > > > reg: > > > maxItems: 1 > > > > > > clocks: > > > - maxItems: 4 > > > + maxItems: 5 > > > > > > clock-names: > > > items: > > > - const: aux > > > + - const: cfg_ahb > > > - const: ref > > > - const: com_aux > > > - const: usb3_pipe > > > > So this would need to be moved to an allOf: construct at the end with > > one section each for sc8280xp and sm6350. > > Ack. So no need to change this it seems. Johan
On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote: > On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote: > > Hi Johan, > > > > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote: > > > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote: > > > > Add the compatible describing the combo phy found on SM6350. > > > > > > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > > > > --- > > > > @Johan Hovold, I've sent this v2 as RFC because there are several things > > > > where I have questions on how it should be done. > > > > > > > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains > > > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the > > > > ssusb@a600000 node, or should I also add it to qmpphy? > > > > > > Yeah, you may need to add a platform specific section of the clocks, > > > which appear to be different, even if I'm not sure they are currently > > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are > > > they named in the vendor's dts? > > > > This is the msm-4.19 dts: > > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354 > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>, > <&rpmhcc RPMH_QLINK_CLK>, > <&gcc GCC_USB3_PRIM_CLKREF_CLK>, > <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>; > clock-names = "aux_clk", "pipe_clk", "ref_clk_src", > "ref_clk", "com_aux_clk"; > > So it looks like you don't need update the binding for the clocks as the > above matches sc8280xp: > > aux > ref > com_aux > usb3_pipe Thanks for checking! > > Parent clocks (ref_clk_src) should not be included in the binding, but > rather be handled by the clock driver. For example, see: > > https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/ > https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/ So I assume you mean that I shouldn't do this: clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, <&rpmhcc RPMH_QLINK_CLK>, <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>, <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; clock-names = "aux", "ref", "com_aux", "usb3_pipe"; But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in debugfs). And for the driver patch, I've discovered that this phy doesn't have separate txa/tbx region, so dts was also wrong there. Do you know if there's a way to test DP phy initialization without having all the USB-C plumbing in place? Might be good to validate at least phy init works if we're already touching all of this. Regards Luca > > > > > .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > > index 6f31693d9868..3e39e3e0504d 100644 > > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml > > > > @@ -17,16 +17,18 @@ properties: > > > > compatible: > > > > enum: > > > > - qcom,sc8280xp-qmp-usb43dp-phy > > > > + - qcom,sm6350-qmp-usb3-dp-phy > > > > > > > > reg: > > > > maxItems: 1 > > > > > > > > clocks: > > > > - maxItems: 4 > > > > + maxItems: 5 > > > > > > > > clock-names: > > > > items: > > > > - const: aux > > > > + - const: cfg_ahb > > > > - const: ref > > > > - const: com_aux > > > > - const: usb3_pipe > > > > > > So this would need to be moved to an allOf: construct at the end with > > > one section each for sc8280xp and sm6350. > > > > Ack. > > So no need to change this it seems. > > Johan
On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote: > On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote: > > On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote: > > > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote: > > > > Yeah, you may need to add a platform specific section of the clocks, > > > > which appear to be different, even if I'm not sure they are currently > > > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are > > > > they named in the vendor's dts? > > > > > > This is the msm-4.19 dts: > > > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354 > > > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > > <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>, > > <&rpmhcc RPMH_QLINK_CLK>, > > <&gcc GCC_USB3_PRIM_CLKREF_CLK>, > > <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>; > > clock-names = "aux_clk", "pipe_clk", "ref_clk_src", > > "ref_clk", "com_aux_clk"; > > > > So it looks like you don't need update the binding for the clocks as the > > above matches sc8280xp: > > > > aux > > ref > > com_aux > > usb3_pipe > > Thanks for checking! > > > > > Parent clocks (ref_clk_src) should not be included in the binding, but > > rather be handled by the clock driver. For example, see: > > > > https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/ > > https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/ > > So I assume you mean that I shouldn't do this: > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > <&rpmhcc RPMH_QLINK_CLK>, > <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>, > <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > clock-names = "aux", "ref", "com_aux", "usb3_pipe"; > > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in > debugfs). Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref, I'd suggest modelling that in the clock driver. Perhaps it has just been left on by the boot firmware. Someone with access to docs may be able explain how it is supposed to be used. > And for the driver patch, I've discovered that this phy doesn't have > separate txa/tbx region, so dts was also wrong there. Do you know if > there's a way to test DP phy initialization without having all the USB-C > plumbing in place? Might be good to validate at least phy init works if > we're already touching all of this. Do you mean that it appears to work as sc8280xp with txa/txb shared by both the USB and DP parts? I guess you need a proper setup to test it properly. Not sure what you'll be able to learn otherwise, apart from whether it passes basic smoke testing. Johan
On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote: > On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote: > > > Parent clocks (ref_clk_src) should not be included in the binding, but > > > rather be handled by the clock driver. For example, see: > > > > > > https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/ > > > https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/ > > > > So I assume you mean that I shouldn't do this: > > > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > > <&rpmhcc RPMH_QLINK_CLK>, > > <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>, > > <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > > clock-names = "aux", "ref", "com_aux", "usb3_pipe"; > > > > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work > > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in > > debugfs). > > Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref, > I'd suggest modelling that in the clock driver. Perhaps it has just been > left on by the boot firmware. Someone with access to docs may be able > explain how it is supposed to be used. RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem). Honestly since it works fine without adding this to gcc driver and I don't really know much about clk (and have no docs for this) would it be okay to just ignore RPMH_QLINK_CLK? > > > And for the driver patch, I've discovered that this phy doesn't have > > separate txa/tbx region, so dts was also wrong there. Do you know if > > there's a way to test DP phy initialization without having all the USB-C > > plumbing in place? Might be good to validate at least phy init works if > > we're already touching all of this. > > Do you mean that it appears to work as sc8280xp with txa/txb shared by > both the USB and DP parts? Yes, looks like it. Can't find any evidence pointing in any other direction at least, everything I've seen shows .txa = 0x1200 & .txb = 0x1600. > > I guess you need a proper setup to test it properly. Not sure what > you'll be able to learn otherwise, apart from whether it passes basic > smoke testing. Currently it's not even smoke testing because dp phy is never getting enabled because there's no consumer. That's why I guess it was never noticed it's wrongly described in dts. Regards Luca > > Johan
On Fri, Nov 25, 2022 at 03:12:24PM +0100, Luca Weiss wrote: > On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote: > > On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote: > > > > Parent clocks (ref_clk_src) should not be included in the binding, but > > > > rather be handled by the clock driver. For example, see: > > > > > > > > https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/ > > > > https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/ > > > > > > So I assume you mean that I shouldn't do this: > > > > > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > > > <&rpmhcc RPMH_QLINK_CLK>, > > > <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>, > > > <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > > > clock-names = "aux", "ref", "com_aux", "usb3_pipe"; > > > > > > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work > > > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in > > > debugfs). > > > > Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref, > > I'd suggest modelling that in the clock driver. Perhaps it has just been > > left on by the boot firmware. Someone with access to docs may be able > > explain how it is supposed to be used. > > RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for > GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem). > > Honestly since it works fine without adding this to gcc driver and I > don't really know much about clk (and have no docs for this) would it be > okay to just ignore RPMH_QLINK_CLK? Preferably it should be fixed now as it may be harder to figure out what's missing in case this causes trouble in some setup later. But, yeah, the lack of documentation is a pain. Hopefully Bjorn or Vinod can help out with getting this sorted properly. > > > And for the driver patch, I've discovered that this phy doesn't have > > > separate txa/tbx region, so dts was also wrong there. Do you know if > > > there's a way to test DP phy initialization without having all the USB-C > > > plumbing in place? Might be good to validate at least phy init works if > > > we're already touching all of this. > > > > Do you mean that it appears to work as sc8280xp with txa/txb shared by > > both the USB and DP parts? > > Yes, looks like it. Can't find any evidence pointing in any other > direction at least, everything I've seen shows .txa = 0x1200 & .txb = > 0x1600. Ok. I've also only seen indirect references to the DP registers for the older platforms, but at least of them do have the separate DP TX regions. > > I guess you need a proper setup to test it properly. Not sure what > > you'll be able to learn otherwise, apart from whether it passes basic > > smoke testing. > > Currently it's not even smoke testing because dp phy is never getting > enabled because there's no consumer. That's why I guess it was never > noticed it's wrongly described in dts. Yeah, people shouldn't be adding (copy-pasted) nodes for peripherals that they are not able to test (especially given the lack of documentation), but I guess the USB3-DP case is a bit of a grey area as the USB part can have been verified. Fortunately, this should be less of any issue with the new binding scheme. Johan
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml index 6f31693d9868..3e39e3e0504d 100644 --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml @@ -17,16 +17,18 @@ properties: compatible: enum: - qcom,sc8280xp-qmp-usb43dp-phy + - qcom,sm6350-qmp-usb3-dp-phy reg: maxItems: 1 clocks: - maxItems: 4 + maxItems: 5 clock-names: items: - const: aux + - const: cfg_ahb - const: ref - const: com_aux - const: usb3_pipe @@ -61,7 +63,6 @@ required: - reg - clocks - clock-names - - power-domains - resets - reset-names - vdda-phy-supply