From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C5BF1C4345F for ; Fri, 12 Apr 2024 14:04:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=47IRsLM7g1qp1maxfEOqp8Fg7fPh9nlbAiwIbugwW1s=; b=WG2nMRgXOKqQ6y MX0NqwDsjX/499RoBnQv85Ag2Di5xC9dQIH+93lXY7w5By+Yhtqnos2+8rA08k5eYVZmvcm8ZxYKh K+d2TW0s+01/Ibkc3fG0LQvZma6QOgtOPYPNUma7hv7UYshFMhYkAc7JMLs0BABMsH0Grqqb26B/6 /yV/dSYPgsmd4RCq2SVLgMs8yFi0RIQWNXIHLhtWex3T+bU2o9zrYyUDaB1dlx+HuhELv9QSw0sPK R5Z7XwFVXPmzsSCiYT6Edoa/EVtFhb65JPW9A+RtRQqdmT5BFlMD81Ntawz+2w7aN14ws8T1fkZ7b PMdNZZOwVH2iWvgCTQLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvHVe-0000000HTMM-3Hzy; Fri, 12 Apr 2024 14:04:06 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvHVX-0000000HTJe-0Fik; Fri, 12 Apr 2024 14:04:05 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 8213A6192D; Fri, 12 Apr 2024 14:03:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E72A0C113CC; Fri, 12 Apr 2024 14:03:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712930634; bh=Q/xDCfodTTDzCgOAtTjTkNdHm3g2Vt761sA8oFCgTzw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PrWb29HUbTPHZgpK8TNIrBLrmHY/lJshbsxKkn7fMnuN/CMsJecrzAsYVn2ey3s2o wCHnYBrORsGcECk52hWMNWr2Y8h89kBqswYZkGqZUuZbzj+pp3Q0FjeD2+XsiiJTQ8 bS4YXNN4uuyV2oEy/T6qQKWfvN7oqHTHSUNlxLjh0A3vcwRo+7av7N72CDfmkszydh KU7XsyPnELKzFpdlVPHDXzYLOXY0fopneuPQVa19ZLhVU6AQBR+1TGFLUdRh6FKZ7+ ChlnOOi3SrUXmjGMDqGzlobwBXObF9t2ELcUG85hxhCf7MVBw4uCoYAPM0BwxD0WnD s/JtuNmpeJfow== Date: Fri, 12 Apr 2024 16:03:48 +0200 From: Niklas Cassel To: Sebastian Reichel Cc: Vinod Koul , Kishon Vijay Abraham I , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Heiko Stuebner , devicetree@vger.kernel.org, Michal Tomek , Damien Le Moal , Jon Lin , Krzysztof Kozlowski , linux-phy@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org Subject: Re: [PATCH v2 1/2] dt-bindings: phy: rockchip,pcie3-phy: add rockchip,rx-common-refclk-mode Message-ID: References: <20240412125818.17052-1-cassel@kernel.org> <20240412125818.17052-2-cassel@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240412_070359_317130_EC68CF22 X-CRM114-Status: GOOD ( 28.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 12, 2024 at 03:36:11PM +0200, Sebastian Reichel wrote: > Hi, > > On Fri, Apr 12, 2024 at 02:58:15PM +0200, Niklas Cassel wrote: > > From the RK3588 Technical Reference Manual, Part1, > > section 6.19 PCIe3PHY_GRF Register Description: > > "rxX_cmn_refclk_mode" > > RX common reference clock mode for lane X. This mode should be enabled > > only when the far-end and near-end devices are running with a common > > reference clock. > > > > The hardware reset value for this field is 0x1 (enabled). > > Note that this register field is only available on RK3588, not on RK3568. > > > > The link training either fails or is highly unstable (link state will jump > > continuously between L0 and recovery) when this mode is enabled while > > using an endpoint running in Separate Reference Clock with No SSC (SRNS) > > mode or Separate Reference Clock with SSC (SRIS) mode. > > (Which is usually the case when using a real SoC as endpoint, e.g. the > > RK3588 PCIe controller can run in both Root Complex and Endpoint mode.) > > > > Add a rockchip specific property to enable/disable the rxX_cmn_refclk_mode > > per lane. (Since this PHY supports bifurcation.) > > > > Signed-off-by: Niklas Cassel > > Acked-by: Krzysztof Kozlowski > > --- > > .../devicetree/bindings/phy/rockchip,pcie3-phy.yaml | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > > index c4fbffcde6e4..ba67dca5a446 100644 > > --- a/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml > > @@ -54,6 +54,16 @@ properties: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: phandle to the syscon managing the pipe "general register files" > > > > + rockchip,rx-common-refclk-mode: > > + description: which lanes (by position) should be configured to run in > > + RX common reference clock mode. 0 means disabled, 1 means enabled. > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 1 > > + maxItems: 16 > > + items: > > + minimum: 0 > > + maximum: 1 > > Why is this not simply using a single u32 with each bit standing for > one PCIe lane? I.e. like this: Hello Sebastian, The reason for the existing way to specify each lane in an int32-array is to be consistent with the existing property "data-lanes", which already uses this representation. e.g. data-lanes = <1 1 2 2>; rockchip,rx-common-refclk-mode = <0 0 1 1>; It would be very weird if this was instead: data-lanes = <1 1 2 2>; rockchip,rx-common-refclk-mode = 0xc; > > rockchip,rx-common-refclk-mode: > description: bitmap describing which lanes should be configured to run > in RX common reference clock mode. Bit offset maps to PCIe lanes and > a bit set means enabled, unset bit means disabled. > $ref: /schemas/types.yaml#/definitions/uint32 > minimum: 0 > maximum: 0xffff > default: 0xffff > > Apart from that the PHY only supports up to 4 lanes on all existing hardware, > so 0xf should be enough? Again, in order to be consistent with the existing "data-lanes" property in this binding, which defines: minItems: 2 maxItems: 16 which means that the binding already supports up to 16 lanes. (The reason why "data-lanes" specifies minItems:2 is because bifurcation doesn't make sense if you just have a single lane. The rx-common-refclk-mode property however makes sense even in the case where there is just a single lane.) Kind regards, Niklas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel