Message ID | 20231214091101.45713-1-manivannan.sadhasivam@linaro.org |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:3b04:b0:fb:cd0c:d3e with SMTP id c4csp8412225dys; Thu, 14 Dec 2023 01:11:31 -0800 (PST) X-Google-Smtp-Source: AGHT+IE/bcpwxNJBDVLLalv+wh9cr+3q9syaSu29ICJygroZJ+DGbZkrwkXNf+QLlnjN50dkNxBb X-Received: by 2002:a9d:76ca:0:b0:6d9:dd90:ffbf with SMTP id p10-20020a9d76ca000000b006d9dd90ffbfmr7960321otl.5.1702545090947; Thu, 14 Dec 2023 01:11:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702545090; cv=none; d=google.com; s=arc-20160816; b=hXQ8gyyjmuMMJcm4LuaM6FEtI0gr4rsP5Wz7SYCO58tbqrhpBqJS03EKyXqmoFuMIb F4ftaRpnM0u4PviGFQxNfsTCOMW+uc/zvJsrn9BhCBWAJ0NXcriMRBHSjvrLU7b2vKR8 IkyRAjaMUo8MJzUmxbcs6qwvx/WaKGkT5hpoq4GtcIhuJmA2EyHcJDGoKmSbsBogyBEA ZslA2VbjQp3xYjX6islJMG34FBJiTudX1Kr0G8Th0xU6dshxNbvSpiRT1KRyPyy1KMzQ +Vmg+EAjiDU1VXdtAYvZlRLF0L8OrIO2X0A/5a07VVCqZn3bfgN2epOTFy3beYbJJAf0 YUOQ== 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=fAhAJU5yP9YNumHSLe8ZpA4RAtCq2Rb2d15S/ucYWeo=; fh=H3Hd3oRQ75JkRULKMW2cRJpc3fzoPQfKgjc+kMLfhs0=; b=IeJMpQCUVU7aErph4SWjlJu7c0ShlR6AftyAHe+Cbb7cfVw4YhSsIJF1BZ/v5PRb80 aB62eVAqLNBSJXsTAOeSIFhIBm6a5iK21Q5uFd5m6bzobCAXIXqCThkHwl4PVrur9xZc 7iHtoerOZuhw5zkxqSxK3TbYKLAoEVu+AIQUrFkUUjTm+hBcC2uznMPkkWhusy7t1UqG kB8TuYRbB3JgclskLcpaLjaIn1Hj+JlKOflue8IMCjf/aPmKrreVgSmt1n+zYxE519Pl rchMeA955kTDNDsE1b1y03E9kPKwyATxmqmtMl19r/QXwuVBpN0N3iVCR4V0smX5HhM2 cijA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bGCvINlD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 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 howler.vger.email (howler.vger.email. [2620:137:e000::3:4]) by mx.google.com with ESMTPS id g184-20020a636bc1000000b005c668a5a906si10859179pgc.282.2023.12.14.01.11.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Dec 2023 01:11:30 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bGCvINlD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 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 howler.vger.email (Postfix) with ESMTP id 0DC2382FA5BD; Thu, 14 Dec 2023 01:11:28 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229595AbjLNJLN (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Thu, 14 Dec 2023 04:11:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46440 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229787AbjLNJLM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 14 Dec 2023 04:11:12 -0500 Received: from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com [IPv6:2607:f8b0:4864:20::72d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2C03116 for <linux-kernel@vger.kernel.org>; Thu, 14 Dec 2023 01:11:18 -0800 (PST) Received: by mail-qk1-x72d.google.com with SMTP id af79cd13be357-77f3b4394fdso429049485a.0 for <linux-kernel@vger.kernel.org>; Thu, 14 Dec 2023 01:11:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1702545078; x=1703149878; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=fAhAJU5yP9YNumHSLe8ZpA4RAtCq2Rb2d15S/ucYWeo=; b=bGCvINlDRccgTjBeFtLbI1oG/SrWWvDZpJ1mxXgCNBoXMItDgZb6FPXU07Jx/evJhV 7wfr3c7aYaccoynVOs7uMvVCeOyMovsDqPAGcSUbDWm8B/0nf76HkjwMTlCGDlJ/dNSy E0T0XkgjmbVYL+24RuQ6DQQIZ5lYuOhoax43U6PR7taHJEKmesHzhaw6VxMWqoeDKFRv yfXyYLv4e0VZl2wKLmLveEHtdZlonrGSbupgwZPzn1Jwd3zgDZcv+lzw0fLPIXyMIAxx pqcXW40YOgxk1IyA8fOBHbsUbAExzT+6MSG/sXK2pUi8AUxgBTgJ+kLUGXgfXXujEvwm XF0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702545078; x=1703149878; 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=fAhAJU5yP9YNumHSLe8ZpA4RAtCq2Rb2d15S/ucYWeo=; b=N7QlbOqw/Mh+36Gr0jaUs4x1vq8OdoHioCy5dWb6LZSLIgB3PyJz4Iqzpkyta16bjX it2EHAT5Ozrbd17KOe/IvJ9Dj5/NJcgGWaSMNhrACof5w8kGVP2jS4A02UIjZcEAlatY iQ7cS5qmpsQbR9xcDUiH7xZoZAUs5RU7XwzlJO+3ScNvPxiiwOZesyT8KHyx9qskJ/FO fDH3VX/x43BSu0HafE1U2/Ip9utPQ1KGMxMi6YtUEtrMvkslYpdQAbIxhlq0lf7xV+wx 6E/R/0o6dVo+wZ9yvlCGMhNbBypV/g6ofyr2CWk0mViQSSVlt9vKSuSo/ZDa7pc9fy0Z VWUg== X-Gm-Message-State: AOJu0YyoLoQCIeakvj7/u/V1S9yZDr7CQJWLGZkuofki7clfAfRVA0sm GTqRlfWwjcUM9S8G8OQJHAbA X-Received: by 2002:ae9:f810:0:b0:77f:55b6:7d00 with SMTP id x16-20020ae9f810000000b0077f55b67d00mr10371015qkh.46.1702545077885; Thu, 14 Dec 2023 01:11:17 -0800 (PST) Received: from localhost.localdomain ([117.213.102.12]) by smtp.gmail.com with ESMTPSA id qt13-20020a05620a8a0d00b0077d75164ef9sm5144119qkn.124.2023.12.14.01.11.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Dec 2023 01:11:17 -0800 (PST) From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> To: andersson@kernel.org, konrad.dybcio@linaro.org, vkoul@kernel.org, sboyd@kernel.org, mturquette@baylibre.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org Cc: linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Subject: [PATCH 00/16] Fix Qcom UFS PHY clocks Date: Thu, 14 Dec 2023 14:40:45 +0530 Message-Id: <20231214091101.45713-1-manivannan.sadhasivam@linaro.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, 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 howler.vger.email 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 (howler.vger.email [0.0.0.0]); Thu, 14 Dec 2023 01:11:28 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785247921352153210 X-GMAIL-MSGID: 1785247921352153210 |
Series |
Fix Qcom UFS PHY clocks
|
|
Message
Manivannan Sadhasivam
Dec. 14, 2023, 9:10 a.m. UTC
Hi, This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS: * ref - 19.2MHz reference clock from RPM/RPMh * ref_aux - Auxiliary reference clock from GCC * qref - QREF clock from GCC or TCSR (TCSR since SM8550) MSM8996 only requires 'ref' and 'qref' clocks. Hence, this series fixes the binding, DT and GCC driver to reflect the actual clock topology. Testing ======= Tested on Qualcomm RB5 development board based on SM8250 SoC. I don't expect this series to break other SoCs too. - Mani Manivannan Sadhasivam (16): dt-bindings: phy: qmp-ufs: Fix PHY clocks phy: qcom-qmp-ufs: Switch to devm_clk_bulk_get_all() API dt-bindings: clock: qcom: Add missing UFS QREF clocks clk: qcom: gcc-sc8180x: Add missing UFS QREF clocks arm64: dts: qcom: msm8996: Fix UFS PHY clocks arm64: dts: qcom: msm8998: Fix UFS PHY clocks arm64: dts: qcom: sdm845: Fix UFS PHY clocks arm64: dts: qcom: sm6115: Fix UFS PHY clocks arm64: dts: qcom: sm6125: Fix UFS PHY clocks arm64: dts: qcom: sm6350: Fix UFS PHY clocks arm64: dts: qcom: sm8150: Fix UFS PHY clocks arm64: dts: qcom: sm8250: Fix UFS PHY clocks arm64: dts: qcom: sc8180x: Fix UFS PHY clocks arm64: dts: qcom: sc8280xp: Fix UFS PHY clocks arm64: dts: qcom: sm8350: Fix UFS PHY clocks arm64: dts: qcom: sm8550: Fix UFS PHY clocks .../phy/qcom,sc8280xp-qmp-ufs-phy.yaml | 47 +++++++------- arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +- arch/arm64/boot/dts/qcom/msm8998.dtsi | 12 ++-- arch/arm64/boot/dts/qcom/sc8180x.dtsi | 6 +- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 18 ++++-- arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm6115.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm6125.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm6350.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm8150.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm8250.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm8350.dtsi | 8 ++- arch/arm64/boot/dts/qcom/sm8550.dtsi | 9 ++- drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++ drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 61 +++---------------- include/dt-bindings/clock/qcom,gcc-sc8180x.h | 2 + 16 files changed, 124 insertions(+), 119 deletions(-)
Comments
On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS: > > * ref - 19.2MHz reference clock from RPM/RPMh > * ref_aux - Auxiliary reference clock from GCC > * qref - QREF clock from GCC or TCSR (TCSR since SM8550) > > MSM8996 only requires 'ref' and 'qref' clocks. > > Hence, this series fixes the binding, DT and GCC driver to reflect the > actual clock topology. Is this based on documentation for all the SoCs or on inference from the current (upstream and downstream) devicetrees? Are you sure that you should not just describe that some of these UFS reference clocks are sourced from CXO in the clock driver instead? Take a look at commits f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks") f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks") Johan
On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote: > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > > > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All > > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS: > > > > * ref - 19.2MHz reference clock from RPM/RPMh > > * ref_aux - Auxiliary reference clock from GCC > > * qref - QREF clock from GCC or TCSR (TCSR since SM8550) > > > > MSM8996 only requires 'ref' and 'qref' clocks. > > > > Hence, this series fixes the binding, DT and GCC driver to reflect the > > actual clock topology. > > Is this based on documentation for all the SoCs or on inference from the > current (upstream and downstream) devicetrees? > It is based on the internal documentation. Even downstream devicetrees are wrong. I should've mentioned it in the cover letter. > Are you sure that you should not just describe that some of these UFS > reference clocks are sourced from CXO in the clock driver instead? > I don't get your comment fully. Could you please elaborate? > Take a look at commits > > f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks") > f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks") > Btw, these commits are not accurate. In all the SoCs before SM8550, reference clock for the UFS device comes from the UFS controller. There is a dedicated register in UFSHC memory map that is being toggled by the driver to enable/disable reference clock for the UFS device. Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a series to fix it. Unfortunately, this information is not depicted correctly in the downstream devicetrees. - Mani > Johan >
[ +CC: Shazad ] On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote: > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote: > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > > > > > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All > > > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS: > > > > > > * ref - 19.2MHz reference clock from RPM/RPMh > > > * ref_aux - Auxiliary reference clock from GCC > > > * qref - QREF clock from GCC or TCSR (TCSR since SM8550) > > > > > > MSM8996 only requires 'ref' and 'qref' clocks. > > > > > > Hence, this series fixes the binding, DT and GCC driver to reflect the > > > actual clock topology. > > > > Is this based on documentation for all the SoCs or on inference from the > > current (upstream and downstream) devicetrees? > > It is based on the internal documentation. Even downstream devicetrees are > wrong. I should've mentioned it in the cover letter. > > > Are you sure that you should not just describe that some of these UFS > > reference clocks are sourced from CXO in the clock driver instead? > > I don't get your comment fully. Could you please elaborate? Unless the PHY consumes CXO directly, it should not be included in the binding as you are suggesting here. We discussed this at some length at the time with Bjorn and Shazad who had access to the documentation and the conclusion was that, at least on sc8280xp, the PHY does not use CXO directly and instead it should be described as a parent to the UFS refclocks in the clock driver: https://lore.kernel.org/lkml/Y2OEjNAPXg5BfOxH@hovoldconsulting.com/ The downstream devicetrees have a bad habit of including parent clocks directly in the consumer node instead of modelling this in clock driver also for other peripherals. > > Take a look at commits > > > > f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks") > > f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks") > > Btw, these commits are not accurate. In all the SoCs before SM8550, reference > clock for the UFS device comes from the UFS controller. There is a dedicated > register in UFSHC memory map that is being toggled by the driver to > enable/disable reference clock for the UFS device. > > Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a > series to fix it. What exactly is wrong with those commits? We know that the controller does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as such for now was a deliberate choice: GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we don't represent the memory device explicitly it seems suitable to use as "ref_clk" in the ufshc nodes - which would then match the special handling of the "link clock" in the UFS driver. > Unfortunately, this information is not depicted correctly in the downstream > devicetrees. I was hoping the information that those commits are based on would be correct as it came from Qualcomm and Bjorn. I have no illusions about the downstream devicetrees being correct. :) Johan
+ Can On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote: > [ +CC: Shazad ] > > On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote: > > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote: > > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > > > > > > > This series fixes the clocks supplied to QMP PHY IPs in the Qcom SoCs. All > > > > of the Qcom SoCs except MSM8996 require 3 clocks for QMP UFS: > > > > > > > > * ref - 19.2MHz reference clock from RPM/RPMh > > > > * ref_aux - Auxiliary reference clock from GCC > > > > * qref - QREF clock from GCC or TCSR (TCSR since SM8550) > > > > > > > > MSM8996 only requires 'ref' and 'qref' clocks. > > > > > > > > Hence, this series fixes the binding, DT and GCC driver to reflect the > > > > actual clock topology. > > > > > > Is this based on documentation for all the SoCs or on inference from the > > > current (upstream and downstream) devicetrees? > > > > It is based on the internal documentation. Even downstream devicetrees are > > wrong. I should've mentioned it in the cover letter. > > > > > Are you sure that you should not just describe that some of these UFS > > > reference clocks are sourced from CXO in the clock driver instead? > > > > I don't get your comment fully. Could you please elaborate? > > Unless the PHY consumes CXO directly, it should not be included in the > binding as you are suggesting here. > PHY is indeed directly consuming CXO. That's why I included it in the binding. > We discussed this at some length at the time with Bjorn and Shazad who > had access to the documentation and the conclusion was that, at least on > sc8280xp, the PHY does not use CXO directly and instead it should be > described as a parent to the UFS refclocks in the clock driver: > > https://lore.kernel.org/lkml/Y2OEjNAPXg5BfOxH@hovoldconsulting.com/ > > The downstream devicetrees have a bad habit of including parent clocks > directly in the consumer node instead of modelling this in clock driver > also for other peripherals. > No, I can assure that you got the wrong info. UFS PHY consumes the clock directly from RPMh. It took me several days to dig through the UFS and PHY docs and special thanks to Can Guo from UFS team, who provided much valuable information about these clocks. > > > Take a look at commits > > > > > > f446022b932a ("arm64: dts: qcom: sc8280xp: fix UFS reference clocks") > > > f6abcc21d943 ("clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks") > > > > Btw, these commits are not accurate. In all the SoCs before SM8550, reference > > clock for the UFS device comes from the UFS controller. There is a dedicated > > register in UFSHC memory map that is being toggled by the driver to > > enable/disable reference clock for the UFS device. > > > > Since SM8550, reference clock is directly sourced from RPMh. I'm preparing a > > series to fix it. > > What exactly is wrong with those commits? We know that the controller > does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as > such for now was a deliberate choice: > > GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we > don't represent the memory device explicitly it seems suitable > to use as "ref_clk" in the ufshc nodes - which would then match > the special handling of the "link clock" in the UFS driver. > No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found information about this specific register in GCC. Initially I thought this is for enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet. But as I said earlier, reference clock to UFS devices comes directly from the controller and there is a specfic register for controlling that. Starting from SM8550, reference clock comes from RPMh. > > Unfortunately, this information is not depicted correctly in the downstream > > devicetrees. > > I was hoping the information that those commits are based on would be > correct as it came from Qualcomm and Bjorn. I have no illusions about > the downstream devicetrees being correct. :) > Unfortunately, you got inaccurate information. But that's very common, since these kind of info are buried down in some docs and people's mind :) - Mani > Johan
On Thu, Dec 14, 2023 at 04:44:09PM +0530, Manivannan Sadhasivam wrote: > + Can > > On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote: > > [ +CC: Shazad ] > > > > On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote: > > > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote: > > > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > > Unless the PHY consumes CXO directly, it should not be included in the > > binding as you are suggesting here. > > PHY is indeed directly consuming CXO. That's why I included it in the binding. Ok, good. It's a bit frustrating that people can even seem to agree on answers to direct questions about that. > > We discussed this at some length at the time with Bjorn and Shazad who > > had access to the documentation and the conclusion was that, at least on > > sc8280xp, the PHY does not use CXO directly and instead it should be > > described as a parent to the UFS refclocks in the clock driver: > > > > https://lore.kernel.org/lkml/Y2OEjNAPXg5BfOxH@hovoldconsulting.com/ > > > > The downstream devicetrees have a bad habit of including parent clocks > > directly in the consumer node instead of modelling this in clock driver > > also for other peripherals. > > > > No, I can assure that you got the wrong info. UFS PHY consumes the clock > directly from RPMh. It took me several days to dig through the UFS and PHY docs > and special thanks to Can Guo from UFS team, who provided much valuable > information about these clocks. Sounds like you've done your research. > > What exactly is wrong with those commits? We know that the controller > > does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as > > such for now was a deliberate choice: > > > > GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we > > don't represent the memory device explicitly it seems suitable > > to use as "ref_clk" in the ufshc nodes - which would then match > > the special handling of the "link clock" in the UFS driver. > > > > No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found > information about this specific register in GCC. Initially I thought this is for > enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet. Just quoting Bjorn. > But as I said earlier, reference clock to UFS devices comes directly from the > controller and there is a specfic register for controlling that. Starting from > SM8550, reference clock comes from RPMh. Sure, but that was only part of what those commits did or claimed. Bjorn also explicitly stated that those refclocks were sourced from CXO, even though I now see a claim from Shazad in that thread claiming the opposite: https://lore.kernel.org/all/Y2Imnf1+v5j5CH9r@hovoldconsulting.com/ Without access to docs I can only ask questions and try to do tedious inferences from incomplete open sources (e.g. downstream devicetrees). Johan
On Thu, Dec 14, 2023 at 12:30:45PM +0100, Johan Hovold wrote: > On Thu, Dec 14, 2023 at 04:44:09PM +0530, Manivannan Sadhasivam wrote: > > + Can > > > > On Thu, Dec 14, 2023 at 12:00:40PM +0100, Johan Hovold wrote: > > > [ +CC: Shazad ] > > > > > > On Thu, Dec 14, 2023 at 04:09:07PM +0530, Manivannan Sadhasivam wrote: > > > > On Thu, Dec 14, 2023 at 11:15:34AM +0100, Johan Hovold wrote: > > > > > On Thu, Dec 14, 2023 at 02:40:45PM +0530, Manivannan Sadhasivam wrote: > > > > Unless the PHY consumes CXO directly, it should not be included in the > > > binding as you are suggesting here. > > > > PHY is indeed directly consuming CXO. That's why I included it in the binding. > > Ok, good. It's a bit frustrating that people can even seem to agree on > answers to direct questions about that. > I can understand that. > > > We discussed this at some length at the time with Bjorn and Shazad who > > > had access to the documentation and the conclusion was that, at least on > > > sc8280xp, the PHY does not use CXO directly and instead it should be > > > described as a parent to the UFS refclocks in the clock driver: > > > > > > https://lore.kernel.org/lkml/Y2OEjNAPXg5BfOxH@hovoldconsulting.com/ > > > > > > The downstream devicetrees have a bad habit of including parent clocks > > > directly in the consumer node instead of modelling this in clock driver > > > also for other peripherals. > > > > > > > No, I can assure that you got the wrong info. UFS PHY consumes the clock > > directly from RPMh. It took me several days to dig through the UFS and PHY docs > > and special thanks to Can Guo from UFS team, who provided much valuable > > information about these clocks. > > Sounds like you've done your research. > > > > What exactly is wrong with those commits? We know that the controller > > > does not consume GCC_UFS_REF_CLKREF_CLK directly, but describing that as > > > such for now was a deliberate choice: > > > > > > GCC_UFS_REF_CLKREF_CLK is the clock to the devices, but as we > > > don't represent the memory device explicitly it seems suitable > > > to use as "ref_clk" in the ufshc nodes - which would then match > > > the special handling of the "link clock" in the UFS driver. > > > > > > > No, GCC_UFS_REF_CLKREF_CLK is _not_ the clock to UFS devices. I haven't found > > information about this specific register in GCC. Initially I thought this is for > > enabling QREF clocks for the UFS MEM phy, but I haven't found the answer yet. > > Just quoting Bjorn. > > > But as I said earlier, reference clock to UFS devices comes directly from the > > controller and there is a specfic register for controlling that. Starting from > > SM8550, reference clock comes from RPMh. > > Sure, but that was only part of what those commits did or claimed. Bjorn > also explicitly stated that those refclocks were sourced from CXO, even > though I now see a claim from Shazad in that thread claiming the > opposite: > > https://lore.kernel.org/all/Y2Imnf1+v5j5CH9r@hovoldconsulting.com/ To clarify further, what Shazad said about GCC_UFS_REF_CLKREF_CLK is correct. This clock is not directly sourced by CXO, so it should be voted by the _PHY_ driver separately along with CXO (which still feeds PHY). That's what I represented in the binding. > > Without access to docs I can only ask questions and try to do tedious > inferences from incomplete open sources (e.g. downstream devicetrees). > That's the life for most of us :) Even with access to internal docs, it is difficult to find the information we are looking for. Because, a very few people know where the information is buried. - Mani > Johan