Message ID | 20231122-phy-qualcomm-edp-x1e80100-v3-2-576fc4e9559d@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp4690586vqy; Thu, 7 Dec 2023 02:53:17 -0800 (PST) X-Google-Smtp-Source: AGHT+IEWn2IQlHzMSYdn1f3c7mdRco7WpXY5rwYJRoY3oKYhuAFl5UcMAe8msPLOqja/tCojEZjU X-Received: by 2002:a05:6870:c0e:b0:1fb:75b:130f with SMTP id le14-20020a0568700c0e00b001fb075b130fmr2445447oab.97.1701946396751; Thu, 07 Dec 2023 02:53:16 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701946396; cv=none; d=google.com; s=arc-20160816; b=Wg414KWNsqRaM11NWTqc0dQJ1uhpAKVFrATW69XVZXjb8zf7N9biht3uu6Kum6FgiK LZRiU2p1bBM11HzoJ9bAC79zUVuF1CNO/hqNp168Xnb2nYL/XeSoDNKO7ZlOtCYzSP7g vGkG4P4Hoc15NlKowUdGhU2lzOSrqAC0GRZxfjIQyabEL83A9zWo4O7nkZ3fb+DuFyYJ feKoOESyoCx+TW5pFiB7GPtfRnydveffr1fTAJGidNKHM/LHNGTetQgKUVG37wsbiyG4 qjXBnuHxfHH/g7twoToHvE/xtoyUnP6iug+kpesEMa2i8BS8DL/UDk4asRbT8wlY1zV2 gLuA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=iCKGAHJfEaYYU45TomxL/sg269wwfTen9MuUFIzedOU=; fh=GSy8hy3QRwoDgZJEBlEaSaMToAfS2O8J9/+ubYJRa/4=; b=KXpaEqOwvZOl5K3tbYjxXEkuq7kIZ43AEmTFOMWlHFr6zOQiWczon/5IYH4OysXdaR Hs5QwSHHh0ZRvZfrwjvFhC5X023OY6jPqBs1vajo7jfbiQN8+Afdw6dPDpC/w39xdkOS aKo6Xztq5k6jLVq+00owCn9sdBoAM5Q50RyrVBp4LY//IP/wl9s4R0gQWNBezhirR7dQ KiCX5fzPnt86d5ESOX91FncUzrCDPdA0R7VsWK2pG9DLvwvz/JFTX1yn/iNE5qIsr4IR Obi3hNjj9t2nTv4xeBzYX1t74NWr9TglK/ndPswqEbRzXdyrd6/FNeepSGw5kKLfb6V5 XL8w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=YRUjzAQm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id cb11-20020a056a02070b00b005a9debd7854si1001077pgb.828.2023.12.07.02.53.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 02:53:16 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=YRUjzAQm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id D193A803E4AB; Thu, 7 Dec 2023 02:53:15 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378770AbjLGKxE (ORCPT <rfc822;pusanteemu@gmail.com> + 99 others); Thu, 7 Dec 2023 05:53:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232262AbjLGKw6 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 7 Dec 2023 05:52:58 -0500 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 05547D5B for <linux-kernel@vger.kernel.org>; Thu, 7 Dec 2023 02:53:05 -0800 (PST) Received: by mail-wm1-x334.google.com with SMTP id 5b1f17b1804b1-40c09b021daso10176685e9.0 for <linux-kernel@vger.kernel.org>; Thu, 07 Dec 2023 02:53:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1701946383; x=1702551183; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=iCKGAHJfEaYYU45TomxL/sg269wwfTen9MuUFIzedOU=; b=YRUjzAQmOqMiZCgvS963MmpgPu89aus/EqKCZzeyz+9EOPq0fo+VdRu9A4HGuTrmHP Ir4zxrWUymOXjyzB1sd0lG4SGhz2073MZ+8pUcmR/KfG82vrvL5jBfsTRuzERYUnqMRs cLzS7OPdBTGeFQ7snhu0hLRIHQwIOeqIIYjg521LvEx0mGLANkLz+texvHkfWWwuoiVK El5C0dA/2fcHw9OHsYtjhVGbV1W1thkgDmPUUstvuuUWbYCORjGNaF4YdnpNtskCaRXM E5Vggkq1KGOTVVNSkiMVK2iQdJwOlo2mJo58ObmVREmaVaKibLnNLWfEoxOoUE2ifp08 9B1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701946383; x=1702551183; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=iCKGAHJfEaYYU45TomxL/sg269wwfTen9MuUFIzedOU=; b=RAj8u8H0r2bBDCTpIOq00MxSHSgwkM/JA4BAeJQB5/ayJ9fCFtoLHG++/YnU59ngvV 72YOXir6sTgbgCBsPMmi3jsbcCh7s2+BnPz1R2Rt/DUYqmFHYOFspJM7XZ9479RwglLY jktIZFtUI5SstjdPsc+Bn3h+/kN/ESbGLz/9jkP7GoXIgIWbVyq98l+zJkc3XhoFabbu ubocO6rZfcuQoKCSDUPnzfh0k+9zZIUNzlJhGCuYeSICZSPnBzLYWLofnCoHLGBykr7R Qc6uk83v+MEknYo5ZsW3ah50SV+HzbtSv4XK0NG5MkXyU8c0hRMxi24f8q6y1xm7LRLO 0f8g== X-Gm-Message-State: AOJu0YzHf/T4vbLRAIBuGSG9LpxOMq1DH9UPgFideWNpcMxgqpBWjBoX xtA8ALyimMskxlbZNJ9lIsVcTg== X-Received: by 2002:a05:600c:1992:b0:40b:5e21:bdd4 with SMTP id t18-20020a05600c199200b0040b5e21bdd4mr1259875wmq.99.1701946383387; Thu, 07 Dec 2023 02:53:03 -0800 (PST) Received: from [127.0.1.1] ([82.79.186.233]) by smtp.gmail.com with ESMTPSA id k23-20020a1709063e1700b00a1db8b08610sm668700eji.148.2023.12.07.02.53.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 02:53:03 -0800 (PST) From: Abel Vesa <abel.vesa@linaro.org> Date: Thu, 07 Dec 2023 12:52:49 +0200 Subject: [PATCH v3 2/3] dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231122-phy-qualcomm-edp-x1e80100-v3-2-576fc4e9559d@linaro.org> References: <20231122-phy-qualcomm-edp-x1e80100-v3-0-576fc4e9559d@linaro.org> In-Reply-To: <20231122-phy-qualcomm-edp-x1e80100-v3-0-576fc4e9559d@linaro.org> To: 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>, Conor Dooley <conor+dt@kernel.org>, Abhinav Kumar <quic_abhinavk@quicinc.com>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Johan Hovold <johan@kernel.org> Cc: linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Abel Vesa <abel.vesa@linaro.org> X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=1283; i=abel.vesa@linaro.org; h=from:subject:message-id; bh=n8ZqqMAvPtzdcc+WNuLj5kAU8rnzZCEsot/TFJ/D1xs=; b=owEBbQKS/ZANAwAKARtfRMkAlRVWAcsmYgBlcaQIB3wgrkjQortdPRoUK1F4EBwZVv4pQXdlr f+C0XZ0PC+JAjMEAAEKAB0WIQRO8+4RTnqPKsqn0bgbX0TJAJUVVgUCZXGkCAAKCRAbX0TJAJUV VpA3EACKl8OPg+nm1ZW8ePqjPuHN9iPzkp9ftIEgCULnE5jiNPXcYOShIVk3qQjpjJxAqsg5qhC sKsQf0Flrdv+ekTWnpf0drw6fhSwxoFq+/S/4JgcGjKW82SBSDDS+5TcwTLUennmVAEEgWOuG5y X4EzeSo/NkuS8wigyzYe5FWsSvvOQV/Ll5vgubC1g+V6Cdzmag7kKxS1OMVzZFh9ik+xbyb1P3j yghxsBadxT2ptRufZDursfZLgAqEQ3qI7yoJ0uRAV8V8TzXG4ZiJ0XZvnJnwtjW+WJNQypnOr/n om+M02nT8fJpMb1A2TEynbg6FIZajiwKqT/1JhfAWb4s1akMq9mFeqNgzdApn3vijHQVfRc2BVW ZahQgnNlieP3VJN1rIVhu9eDU5wY4wiwaWdit6jB9AHU4kculs9eDtx1v8bxMF8K+Sx3QbKBMuQ 89Wb4M037BlNWfEbkeizZu4FOPLdQsFPy9MoErAj7uAJuEm465dxjD1RXUFaNsrNAtsK7BZ/iTf eFM6OOzBlMKpCnMlFVWzaY/aDKVtGzesUzEpl1JeMZ2tLgpy4sJQiy8zqQEuXoj4i6OBsckVVey g4yfFAMP91ShAw6uMxjLb3uMEmD/CfXAruDFmA6RIDGaf0s2RWyDJGQHEggnImHTlS8vlFd1u8I Kf9evD9WPZlo9IQ== X-Developer-Key: i=abel.vesa@linaro.org; a=openpgp; fpr=6AFF162D57F4223A8770EF5AF7BF214136F41FAE 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_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE 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-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 07 Dec 2023 02:53:16 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784620144952392011 X-GMAIL-MSGID: 1784620144952392011 |
Series |
phy: qcom: edp: Add support for X1E80100
|
|
Commit Message
Abel Vesa
Dec. 7, 2023, 10:52 a.m. UTC
The Qualcomm X1E80100 platform has multiple PHYs that can work in both
eDP or DP mode, add compatibles for these. New platforms can use the
phy-type property to specify which mode the PHY should be configured in.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Comments
On 07/12/2023 11:52, Abel Vesa wrote: > The Qualcomm X1E80100 platform has multiple PHYs that can work in both > eDP or DP mode, add compatibles for these. New platforms can use the > phy-type property to specify which mode the PHY should be configured in. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml > index 6566353f1a02..3341283577ce 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml > @@ -21,6 +21,7 @@ properties: > - qcom,sc8180x-edp-phy > - qcom,sc8280xp-dp-phy > - qcom,sc8280xp-edp-phy > + - qcom,x1e80100-dp-phy > > reg: > items: > @@ -59,6 +60,20 @@ required: > > additionalProperties: false Please put it after allOf: block (IOW, allOf: before additonalProperties:) > > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,x1e80100-dp-phy > + then: > + properties: > + phy-type: > + description: DP (default) or eDP type Properties must be defined in top-level "properties:" block. In allOf:if:then you only disallow them for other variants. > + enum: [ 6, 13 ] > + default: 6 Anyway, I was thinking this should be rather argument to phy-cells. Best regards, Krzysztof
On 12/7/23 17:51, Krzysztof Kozlowski wrote: [...] >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - qcom,x1e80100-dp-phy >> + then: >> + properties: >> + phy-type: >> + description: DP (default) or eDP type > > Properties must be defined in top-level "properties:" block. In > allOf:if:then you only disallow them for other variants. > >> + enum: [ 6, 13 ] >> + default: 6 > > Anyway, I was thinking this should be rather argument to phy-cells. I'm not sure I'm for this, because the results would be: --- device.dts --- &dp_controller0 { phys = <&dp_phy0 PHY_EDP>; }; &dp_controller1 { phys = <&dp_phy1 PHY_DP>; }; ------------------ as opposed to: --- device.dts --- &dp_phy0 { phy-type <PHY_EDP>; }; &dp_phy1 { phy-type = <PHY_DP>; }; ------------------ i.e., we would be saying "this board is connected to this phy instead" vs "this phy is of this type on this board". While none of them really fit the "same hw, different config" situation, I'd vote for the latter one being closer to the truth Konrad
On 07/12/2023 20:16, Konrad Dybcio wrote: > > > On 12/7/23 17:51, Krzysztof Kozlowski wrote: > > [...] > >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - qcom,x1e80100-dp-phy >>> + then: >>> + properties: >>> + phy-type: >>> + description: DP (default) or eDP type >> >> Properties must be defined in top-level "properties:" block. In >> allOf:if:then you only disallow them for other variants. >> >>> + enum: [ 6, 13 ] >>> + default: 6 >> >> Anyway, I was thinking this should be rather argument to phy-cells. > I'm not sure I'm for this, because the results would be: > > --- device.dts --- > &dp_controller0 { > phys = <&dp_phy0 PHY_EDP>; > }; > > &dp_controller1 { > phys = <&dp_phy1 PHY_DP>; > }; > ------------------ > > as opposed to: > > --- device.dts --- > &dp_phy0 { > phy-type <PHY_EDP>; > }; > > &dp_phy1 { > phy-type = <PHY_DP>; > }; > ------------------ Which is exactly what I proposed/wanted to see. > > i.e., we would be saying "this board is connected to this phy > instead" vs "this phy is of this type on this board". > > While none of them really fit the "same hw, different config" > situation, I'd vote for the latter one being closer to the > truth Then maybe I miss the bigger picture, but commit msg clearly says: "multiple PHYs that can work in both eDP or DP mode" If this is not the case, describe the hardware correctly in the commit msg, so people will not ask stupid questions... Best regards, Krzysztof
On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 07/12/2023 20:16, Konrad Dybcio wrote: > > > > > > On 12/7/23 17:51, Krzysztof Kozlowski wrote: > > > > [...] > > > >>> +allOf: > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - qcom,x1e80100-dp-phy > >>> + then: > >>> + properties: > >>> + phy-type: > >>> + description: DP (default) or eDP type > >> > >> Properties must be defined in top-level "properties:" block. In > >> allOf:if:then you only disallow them for other variants. > >> > >>> + enum: [ 6, 13 ] > >>> + default: 6 > >> > >> Anyway, I was thinking this should be rather argument to phy-cells. > > I'm not sure I'm for this, because the results would be: > > > > --- device.dts --- > > &dp_controller0 { > > phys = <&dp_phy0 PHY_EDP>; > > }; > > > > &dp_controller1 { > > phys = <&dp_phy1 PHY_DP>; > > }; > > ------------------ > > > > as opposed to: > > > > --- device.dts --- > > &dp_phy0 { > > phy-type <PHY_EDP>; > > }; > > > > &dp_phy1 { > > phy-type = <PHY_DP>; > > }; > > ------------------ > > Which is exactly what I proposed/wanted to see. > > > > > i.e., we would be saying "this board is connected to this phy > > instead" vs "this phy is of this type on this board". > > > > While none of them really fit the "same hw, different config" > > situation, I'd vote for the latter one being closer to the > > truth > > Then maybe I miss the bigger picture, but commit msg clearly says: > "multiple PHYs that can work in both eDP or DP mode" > > If this is not the case, describe the hardware correctly in the commit > msg, so people will not ask stupid questions... There are multiple PHYs (each of them at its own address space). Each of the PHYs in question can be used either for the DisplayPort output (directly or through the USB-C) or to drive the eDP panel. Same applies to the displayport-controller. It can either drive the DP or eDP output, hardware-wise it is the same.
On 08/12/2023 12:04, Dmitry Baryshkov wrote: > On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 07/12/2023 20:16, Konrad Dybcio wrote: >>> >>> >>> On 12/7/23 17:51, Krzysztof Kozlowski wrote: >>> >>> [...] >>> >>>>> +allOf: >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - qcom,x1e80100-dp-phy >>>>> + then: >>>>> + properties: >>>>> + phy-type: >>>>> + description: DP (default) or eDP type >>>> >>>> Properties must be defined in top-level "properties:" block. In >>>> allOf:if:then you only disallow them for other variants. >>>> >>>>> + enum: [ 6, 13 ] >>>>> + default: 6 >>>> >>>> Anyway, I was thinking this should be rather argument to phy-cells. >>> I'm not sure I'm for this, because the results would be: >>> >>> --- device.dts --- >>> &dp_controller0 { >>> phys = <&dp_phy0 PHY_EDP>; >>> }; >>> >>> &dp_controller1 { >>> phys = <&dp_phy1 PHY_DP>; >>> }; >>> ------------------ >>> >>> as opposed to: >>> >>> --- device.dts --- >>> &dp_phy0 { >>> phy-type <PHY_EDP>; >>> }; >>> >>> &dp_phy1 { >>> phy-type = <PHY_DP>; >>> }; >>> ------------------ >> >> Which is exactly what I proposed/wanted to see. >> >>> >>> i.e., we would be saying "this board is connected to this phy >>> instead" vs "this phy is of this type on this board". >>> >>> While none of them really fit the "same hw, different config" >>> situation, I'd vote for the latter one being closer to the >>> truth >> >> Then maybe I miss the bigger picture, but commit msg clearly says: >> "multiple PHYs that can work in both eDP or DP mode" >> >> If this is not the case, describe the hardware correctly in the commit >> msg, so people will not ask stupid questions... > > There are multiple PHYs (each of them at its own address space). Each > of the PHYs in question can be used either for the DisplayPort output > (directly or through the USB-C) or to drive the eDP panel. > > Same applies to the displayport-controller. It can either drive the DP > or eDP output, hardware-wise it is the same. Therefore what I proposed was correct - the block which uses the phy configures its mode. Because this part: "this phy is of this type on this board". is not true. The phy is both types. Best regards, Krzysztof
On Fri, 8 Dec 2023 at 13:45, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/12/2023 12:04, Dmitry Baryshkov wrote: > > On Fri, 8 Dec 2023 at 09:47, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 07/12/2023 20:16, Konrad Dybcio wrote: > >>> > >>> > >>> On 12/7/23 17:51, Krzysztof Kozlowski wrote: > >>> > >>> [...] > >>> > >>>>> +allOf: > >>>>> + - if: > >>>>> + properties: > >>>>> + compatible: > >>>>> + contains: > >>>>> + enum: > >>>>> + - qcom,x1e80100-dp-phy > >>>>> + then: > >>>>> + properties: > >>>>> + phy-type: > >>>>> + description: DP (default) or eDP type > >>>> > >>>> Properties must be defined in top-level "properties:" block. In > >>>> allOf:if:then you only disallow them for other variants. > >>>> > >>>>> + enum: [ 6, 13 ] > >>>>> + default: 6 > >>>> > >>>> Anyway, I was thinking this should be rather argument to phy-cells. > >>> I'm not sure I'm for this, because the results would be: > >>> > >>> --- device.dts --- > >>> &dp_controller0 { > >>> phys = <&dp_phy0 PHY_EDP>; > >>> }; > >>> > >>> &dp_controller1 { > >>> phys = <&dp_phy1 PHY_DP>; > >>> }; > >>> ------------------ > >>> > >>> as opposed to: > >>> > >>> --- device.dts --- > >>> &dp_phy0 { > >>> phy-type <PHY_EDP>; > >>> }; > >>> > >>> &dp_phy1 { > >>> phy-type = <PHY_DP>; > >>> }; > >>> ------------------ > >> > >> Which is exactly what I proposed/wanted to see. > >> > >>> > >>> i.e., we would be saying "this board is connected to this phy > >>> instead" vs "this phy is of this type on this board". > >>> > >>> While none of them really fit the "same hw, different config" > >>> situation, I'd vote for the latter one being closer to the > >>> truth > >> > >> Then maybe I miss the bigger picture, but commit msg clearly says: > >> "multiple PHYs that can work in both eDP or DP mode" > >> > >> If this is not the case, describe the hardware correctly in the commit > >> msg, so people will not ask stupid questions... > > > > There are multiple PHYs (each of them at its own address space). Each > > of the PHYs in question can be used either for the DisplayPort output > > (directly or through the USB-C) or to drive the eDP panel. > > > > Same applies to the displayport-controller. It can either drive the DP > > or eDP output, hardware-wise it is the same. > > Therefore what I proposed was correct - the block which uses the phy > configures its mode. Because this part: > "this phy is of this type on this board". > is not true. The phy is both types. But hopefully you don't mean using #phy-cells here. There are no sub-PHYs or anything like that.
On 08/12/2023 13:17, Dmitry Baryshkov wrote: >>>>>> Anyway, I was thinking this should be rather argument to phy-cells. >>>>> I'm not sure I'm for this, because the results would be: >>>>> >>>>> --- device.dts --- >>>>> &dp_controller0 { >>>>> phys = <&dp_phy0 PHY_EDP>; >>>>> }; >>>>> >>>>> &dp_controller1 { >>>>> phys = <&dp_phy1 PHY_DP>; >>>>> }; >>>>> ------------------ >>>>> >>>>> as opposed to: >>>>> >>>>> --- device.dts --- >>>>> &dp_phy0 { >>>>> phy-type <PHY_EDP>; >>>>> }; >>>>> >>>>> &dp_phy1 { >>>>> phy-type = <PHY_DP>; >>>>> }; >>>>> ------------------ >>>> >>>> Which is exactly what I proposed/wanted to see. >>>> >>>>> >>>>> i.e., we would be saying "this board is connected to this phy >>>>> instead" vs "this phy is of this type on this board". >>>>> >>>>> While none of them really fit the "same hw, different config" >>>>> situation, I'd vote for the latter one being closer to the >>>>> truth >>>> >>>> Then maybe I miss the bigger picture, but commit msg clearly says: >>>> "multiple PHYs that can work in both eDP or DP mode" >>>> >>>> If this is not the case, describe the hardware correctly in the commit >>>> msg, so people will not ask stupid questions... >>> >>> There are multiple PHYs (each of them at its own address space). Each >>> of the PHYs in question can be used either for the DisplayPort output >>> (directly or through the USB-C) or to drive the eDP panel. >>> >>> Same applies to the displayport-controller. It can either drive the DP >>> or eDP output, hardware-wise it is the same. >> >> Therefore what I proposed was correct - the block which uses the phy >> configures its mode. Because this part: >> "this phy is of this type on this board". >> is not true. The phy is both types. > > But hopefully you don't mean using #phy-cells here. There are no > sub-PHYs or anything like that. I am exactly talking about phy-cells. Look at first example from Abel's code. Best regards, Krzysztof
On Fri, 8 Dec 2023 at 14:21, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/12/2023 13:17, Dmitry Baryshkov wrote: > >>>>>> Anyway, I was thinking this should be rather argument to phy-cells. > >>>>> I'm not sure I'm for this, because the results would be: > >>>>> > >>>>> --- device.dts --- > >>>>> &dp_controller0 { > >>>>> phys = <&dp_phy0 PHY_EDP>; > >>>>> }; > >>>>> > >>>>> &dp_controller1 { > >>>>> phys = <&dp_phy1 PHY_DP>; > >>>>> }; > >>>>> ------------------ > >>>>> > >>>>> as opposed to: > >>>>> > >>>>> --- device.dts --- > >>>>> &dp_phy0 { > >>>>> phy-type <PHY_EDP>; > >>>>> }; > >>>>> > >>>>> &dp_phy1 { > >>>>> phy-type = <PHY_DP>; > >>>>> }; > >>>>> ------------------ > >>>> > >>>> Which is exactly what I proposed/wanted to see. > >>>> > >>>>> > >>>>> i.e., we would be saying "this board is connected to this phy > >>>>> instead" vs "this phy is of this type on this board". > >>>>> > >>>>> While none of them really fit the "same hw, different config" > >>>>> situation, I'd vote for the latter one being closer to the > >>>>> truth > >>>> > >>>> Then maybe I miss the bigger picture, but commit msg clearly says: > >>>> "multiple PHYs that can work in both eDP or DP mode" > >>>> > >>>> If this is not the case, describe the hardware correctly in the commit > >>>> msg, so people will not ask stupid questions... > >>> > >>> There are multiple PHYs (each of them at its own address space). Each > >>> of the PHYs in question can be used either for the DisplayPort output > >>> (directly or through the USB-C) or to drive the eDP panel. > >>> > >>> Same applies to the displayport-controller. It can either drive the DP > >>> or eDP output, hardware-wise it is the same. > >> > >> Therefore what I proposed was correct - the block which uses the phy > >> configures its mode. Because this part: > >> "this phy is of this type on this board". > >> is not true. The phy is both types. > > > > But hopefully you don't mean using #phy-cells here. There are no > > sub-PHYs or anything like that. > > I am exactly talking about phy-cells. Look at first example from Abel's > code. I always had an impression that #foo-cells means that there are different units within the major handler. I.e. #clock-cells mean that there are several different clocks, #reset-cells mean that there are several resets, etc. Ok, maybe this is not a perfect description. We need cells to identify a particular instance within the major block. Maybe that sounds more correct. For the USB+DP PHY we use #phy-cells to select between USB3 and DP PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc. There is a single PHY which works in either of the modes. Last, but not least, using #phy-cells in this way would create asymmetry with all the other PHYs (and especially other QMP PHYs) present on these platforms. If you feel that phy-type is not an appropriate solution, I'd vote for not having the type in DT at all, letting the DP controller determine the proper mode on its own.
On 08/12/2023 13:35, Dmitry Baryshkov wrote: >>>>> Same applies to the displayport-controller. It can either drive the DP >>>>> or eDP output, hardware-wise it is the same. >>>> >>>> Therefore what I proposed was correct - the block which uses the phy >>>> configures its mode. Because this part: >>>> "this phy is of this type on this board". >>>> is not true. The phy is both types. >>> >>> But hopefully you don't mean using #phy-cells here. There are no >>> sub-PHYs or anything like that. >> >> I am exactly talking about phy-cells. Look at first example from Abel's >> code. > > I always had an impression that #foo-cells means that there are > different units within the major handler. I.e. #clock-cells mean that > there are several different clocks, #reset-cells mean that there are > several resets, etc. > Ok, maybe this is not a perfect description. We need cells to identify > a particular instance within the major block. Maybe that sounds more > correct. No, the cells have also meaning of additional arguments. See usage of phy-type (not the one here, but the correct one) and PWMs. > > For the USB+DP PHY we use #phy-cells to select between USB3 and DP > PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc. > There is a single PHY which works in either of the modes. Is it different than case here? > > Last, but not least, using #phy-cells in this way would create > asymmetry with all the other PHYs (and especially other QMP PHYs) > present on these platforms. OK. Is phy-type not something different? > > If you feel that phy-type is not an appropriate solution, I'd vote for > not having the type in DT at all, letting the DP controller determine > the proper mode on its own. Can we do it? That's BTW the best option. Best regards, Krzysztof
On Fri, 8 Dec 2023 at 14:47, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/12/2023 13:35, Dmitry Baryshkov wrote: > >>>>> Same applies to the displayport-controller. It can either drive the DP > >>>>> or eDP output, hardware-wise it is the same. > >>>> > >>>> Therefore what I proposed was correct - the block which uses the phy > >>>> configures its mode. Because this part: > >>>> "this phy is of this type on this board". > >>>> is not true. The phy is both types. > >>> > >>> But hopefully you don't mean using #phy-cells here. There are no > >>> sub-PHYs or anything like that. > >> > >> I am exactly talking about phy-cells. Look at first example from Abel's > >> code. > > > > I always had an impression that #foo-cells means that there are > > different units within the major handler. I.e. #clock-cells mean that > > there are several different clocks, #reset-cells mean that there are > > several resets, etc. > > Ok, maybe this is not a perfect description. We need cells to identify > > a particular instance within the major block. Maybe that sounds more > > correct. > > No, the cells have also meaning of additional arguments. See usage of > phy-type (not the one here, but the correct one) and PWMs. phy-type being used for the 7nm DSI PHY, where it specify exactly the same thing: whether the connected device uses D-PHY or C-PHY modes of the PHY. cdns,phy-type - selecs between PCIe, DP, USB3 or other modes of the PHY ti/emif.txt: phy-type specifies which PHY is attached / used in the controller xlnx,phy-type: deprecated in favour of phy-mode, selects MII mode for the PHY marvell,xenon-phy-type: I _think_ this specifies the actual PHY attached to the controller in hardware. > > For the USB+DP PHY we use #phy-cells to select between USB3 and DP > > PHYs. But for these PHYs we do not have sub-devices, sub-blocks, etc. > > There is a single PHY which works in either of the modes. > > Is it different than case here? Hmm, I was not clear enough. USB+DP = two different PHYs in the same hardware block. DP-eDP = single PHY, working in one of the modes. > > > > > Last, but not least, using #phy-cells in this way would create > > asymmetry with all the other PHYs (and especially other QMP PHYs) > > present on these platforms. > > OK. Is phy-type not something different? No. It doesn't redefine what we already have for other QMP PHYs, it defines new property. > > > > > If you feel that phy-type is not an appropriate solution, I'd vote for > > not having the type in DT at all, letting the DP controller determine > > the proper mode on its own. > > Can we do it? That's BTW the best option. That's a good question. We have separate -dp and -edp compatibles for the DP controller, but I think those also should go, at least for newer platforms. And the reason is the same, there is a single hardware block, just two modes of operation. See mdss_dp3 in the X13s's DT file. I had a thought of using aux-bus presence to determine whether the controller is working in the DP or eDP modes. But this might need additional care for older DT files.
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index 6566353f1a02..3341283577ce 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -21,6 +21,7 @@ properties: - qcom,sc8180x-edp-phy - qcom,sc8280xp-dp-phy - qcom,sc8280xp-edp-phy + - qcom,x1e80100-dp-phy reg: items: @@ -59,6 +60,20 @@ required: additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + enum: + - qcom,x1e80100-dp-phy + then: + properties: + phy-type: + description: DP (default) or eDP type + enum: [ 6, 13 ] + default: 6 + examples: - | phy@aec2a00 {