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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BCD5C4321E for ; Fri, 25 Nov 2022 10:12:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229876AbiKYKMG (ORCPT ); Fri, 25 Nov 2022 05:12:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37002 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229536AbiKYKMF (ORCPT ); Fri, 25 Nov 2022 05:12:05 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 159E61FFAD; Fri, 25 Nov 2022 02:11:54 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D9D826231F; Fri, 25 Nov 2022 10:11:53 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 29B49C433C1; Fri, 25 Nov 2022 10:11:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669371113; bh=CtuOyTzwbNMD8uQ5rLP+h3ZAPmaYchqlqNUidC/4VKY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OiS+a2JQ8OWVB2U+bpcfHNg8bYFoGqb33LPVPwU9wZeFNW0SElLBVm/O8Uh8HoPKm YD5+VxeA1WC7lyHKQR418KIOgXDnsbp5cxu9HgWFMnuhyNIp5KKcFLeLnejN/0CU/a xcOir2W/EU5DEvOlZzn3n3XM+i/pgoY6hWlKwLEFyDwKMalq6xSWKHqom2eP5mrPeo xZjLF0Ob5T6U7eX/1p8jWZoPfT92Yo4K1jQuIgzOl2lESqW7rDpWvqbsCk8Gpk+jkg XxTfdqGmMEc512AxznV6ixlY2wKh3HACZLCMKlMMOzz3PYicIaarCiHiqmYQePvrti 4Q5aEh1OKr4Qg== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1oyVg7-0002QB-Si; Fri, 25 Nov 2022 11:11:28 +0100 Date: Fri, 25 Nov 2022 11:11:27 +0100 From: Johan Hovold To: Luca Weiss Cc: linux-arm-msm@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org, Andy Gross , Bjorn Andersson , Konrad Dybcio , Rob Herring , Krzysztof Kozlowski , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v2 3/3] arm64: dts: qcom: sm6350: Use specific qmpphy compatible Message-ID: References: <20221125092749.46073-1-luca.weiss@fairphone.com> <20221125092749.46073-3-luca.weiss@fairphone.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221125092749.46073-3-luca.weiss@fairphone.com> Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Fri, Nov 25, 2022 at 10:27:49AM +0100, Luca Weiss wrote: > The sc7180 phy compatible works fine for some cases, but it turns out > sm6350 does need proper phy configuration in the driver, so use the > newly added sm6350 compatible. > > Because the sm6350 compatible is using the new binding, we need to > change the node quite a bit to match it. > > This fixes qmpphy init when no USB cable is plugged in during bootloader > stage. > > Signed-off-by: Luca Weiss > --- > @Johan Hovold, in this patch there's also the question about cfg_ahb, > power-domains but I'm also not happy about using the > QMP_USB43DP_USB3_PHY define for the phy reference. Do you think it's a > good idea to introduce e.g. QMP_USB3DP_USB3_PHY with the same value so > it's essentially just an alias to the other? We had that discussion the other week and I believe we agreed that reusing the define with a more general infix (USB43DP) was fine as it's just a name for a constant (and the USB43DP constants will be a superset of the ones needed for USB3-DP PHYs). > This series is tested on next-20221124 with next branch of linux-phy > repo (commit bea3ce759b46) merged in. The dependencies should all be in linux-next as of today. > arch/arm64/boot/dts/qcom/sm6350.dtsi | 46 +++++++--------------------- > 1 file changed, 11 insertions(+), 35 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi > index 0f01ff4feb55..923c8bb7e5f8 100644 > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -1119,50 +1120,25 @@ usb_1_hsphy: phy@88e3000 { > resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>; > }; > > - usb_1_qmpphy: phy@88e9000 { > - compatible = "qcom,sc7180-qmp-usb3-dp-phy"; > - reg = <0 0x088e9000 0 0x200>, > - <0 0x088e8000 0 0x40>, > - <0 0x088ea000 0 0x200>; > - status = "disabled"; > - #address-cells = <2>; > - #size-cells = <2>; > - ranges; > + usb_1_qmpphy: phy@88e8000 { > + compatible = "qcom,sm6350-qmp-usb3-dp-phy"; > + reg = <0 0x088e8000 0 0x3000>; > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>, > <&xo_board>, > <&rpmhcc RPMH_QLINK_CLK>, > - <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>; > - clock-names = "aux", "cfg_ahb", "ref", "com_aux"; > + <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>, > + <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > + clock-names = "aux", "cfg_ahb", "ref", "com_aux", "usb3_pipe"; As I mentioned in my reply to the binding, we should double check the vendor dts and hardware documentation (if possible) before settling on these names which appears to just have been reused from some older platform. > > resets = <&gcc GCC_USB3_DP_PHY_PRIM_BCR>, > <&gcc GCC_USB3_PHY_PRIM_BCR>; > reset-names = "phy", "common"; > > - usb_1_ssphy: usb3-phy@88e9200 { > - reg = <0 0x088e9200 0 0x200>, > - <0 0x088e9400 0 0x200>, > - <0 0x088e9c00 0 0x400>, > - <0 0x088e9600 0 0x200>, > - <0 0x088e9800 0 0x200>, > - <0 0x088e9a00 0 0x100>; > - #clock-cells = <0>; > - #phy-cells = <0>; > - clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>; > - clock-names = "pipe0"; > - clock-output-names = "usb3_phy_pipe_clk_src"; > - }; > + #clock-cells = <1>; > + #phy-cells = <1>; > > - dp_phy: dp-phy@88ea200 { > - reg = <0 0x088ea200 0 0x200>, > - <0 0x088ea400 0 0x200>, > - <0 0x088eac00 0 0x400>, > - <0 0x088ea600 0 0x200>, > - <0 0x088ea800 0 0x200>, > - <0 0x088eaa00 0 0x100>; Note that these registers were not correct to begin with and a fix has been posted here: https://lore.kernel.org/all/20221111094729.11842-2-johan+linaro@kernel.org/ Will hopefully show up in linux-next next week so this can be rebased on top. > - #phy-cells = <0>; > - #clock-cells = <1>; > - }; > + status = "disabled"; > }; > > dc_noc: interconnect@9160000 { > @@ -1236,7 +1212,7 @@ usb_1_dwc3: usb@a600000 { > snps,dis_enblslpm_quirk; > snps,has-lpm-erratum; > snps,hird-threshold = /bits/ 8 <0x10>; > - phys = <&usb_1_hsphy>, <&usb_1_ssphy>; > + phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>; > phy-names = "usb2-phy", "usb3-phy"; > }; > }; Johan