From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 750B23E483; Thu, 14 Dec 2023 11:14:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 508ABC433C7; Thu, 14 Dec 2023 11:14:21 +0000 (UTC) Date: Thu, 14 Dec 2023 16:44:09 +0530 From: Manivannan Sadhasivam To: Johan Hovold Cc: Manivannan Sadhasivam , 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, 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, Shazad Hussain , quic_cang@quicinc.com Subject: Re: [PATCH 00/16] Fix Qcom UFS PHY clocks Message-ID: <20231214111409.GB48078@thinkpad> References: <20231214091101.45713-1-manivannan.sadhasivam@linaro.org> <20231214103907.GL2938@thinkpad> Precedence: bulk X-Mailing-List: linux-arm-msm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: + 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 -- மணிவண்ணன் சதாசிவம் 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 B6D65C4167B for ; Thu, 14 Dec 2023 11:14:39 +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=aS4ytu1eRbG7pBKg7OF76dyguq2r8gg+95jonwndYEY=; b=yT3uWKbuMS2zaR Z5Ltfry4jhPN3ZcpAOBnKatc8NtoWgZ8BdBeULNdyRF2/pukX9icN3vY5S1PQfzqcbrp4gbKXuAtS BbGdMUbhJvwDqt8gwKgAZcBUZELRL0iDq/sCIuoVyQVZO+PNRWGdwDb3SABL0sKVFVR2dd40lH6K6 jTN+xC12uTpmQmlItratTYqeLKdkOMg7J0H2qzXBVDWz6/cOfjzjzoonBWRfovmnY9Ij+GWseOpSe Up4qqKg6thFe5Ud1wzPSNj6efe0Lo5hc/QjCV2cq9sFSTRu2ZOSh7fEw9VNexZn3qoK57l6DqkJn/ Q5lQMTPDw7Up7Icag97g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rDjfr-0000Bt-1Q; Thu, 14 Dec 2023 11:14:39 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rDjfo-0000Ai-0a for linux-phy@lists.infradead.org; Thu, 14 Dec 2023 11:14:37 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 4EA80621DD; Thu, 14 Dec 2023 11:14:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 508ABC433C7; Thu, 14 Dec 2023 11:14:21 +0000 (UTC) Date: Thu, 14 Dec 2023 16:44:09 +0530 From: Manivannan Sadhasivam To: Johan Hovold Cc: Manivannan Sadhasivam , 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, 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, Shazad Hussain , quic_cang@quicinc.com Subject: Re: [PATCH 00/16] Fix Qcom UFS PHY clocks Message-ID: <20231214111409.GB48078@thinkpad> References: <20231214091101.45713-1-manivannan.sadhasivam@linaro.org> <20231214103907.GL2938@thinkpad> 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-20231214_031436_327648_4FC5569F X-CRM114-Status: GOOD ( 37.57 ) X-BeenThere: linux-phy@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux Phy Mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-phy" Errors-To: linux-phy-bounces+linux-phy=archiver.kernel.org@lists.infradead.org KyBDYW4KCk9uIFRodSwgRGVjIDE0LCAyMDIzIGF0IDEyOjAwOjQwUE0gKzAxMDAsIEpvaGFuIEhv dm9sZCB3cm90ZToKPiBbICtDQzogU2hhemFkIF0KPiAKPiBPbiBUaHUsIERlYyAxNCwgMjAyMyBh dCAwNDowOTowN1BNICswNTMwLCBNYW5pdmFubmFuIFNhZGhhc2l2YW0gd3JvdGU6Cj4gPiBPbiBU aHUsIERlYyAxNCwgMjAyMyBhdCAxMToxNTozNEFNICswMTAwLCBKb2hhbiBIb3ZvbGQgd3JvdGU6 Cj4gPiA+IE9uIFRodSwgRGVjIDE0LCAyMDIzIGF0IDAyOjQwOjQ1UE0gKzA1MzAsIE1hbml2YW5u YW4gU2FkaGFzaXZhbSB3cm90ZToKPiA+ID4gCj4gPiA+ID4gVGhpcyBzZXJpZXMgZml4ZXMgdGhl IGNsb2NrcyBzdXBwbGllZCB0byBRTVAgUEhZIElQcyBpbiB0aGUgUWNvbSBTb0NzLiBBbGwKPiA+ ID4gPiBvZiB0aGUgUWNvbSBTb0NzIGV4Y2VwdCBNU004OTk2IHJlcXVpcmUgMyBjbG9ja3MgZm9y IFFNUCBVRlM6Cj4gPiA+ID4gCj4gPiA+ID4gKiByZWYgLSAxOS4yTUh6IHJlZmVyZW5jZSBjbG9j ayBmcm9tIFJQTS9SUE1oCj4gPiA+ID4gKiByZWZfYXV4IC0gQXV4aWxpYXJ5IHJlZmVyZW5jZSBj bG9jayBmcm9tIEdDQwo+ID4gPiA+ICogcXJlZiAtIFFSRUYgY2xvY2sgZnJvbSBHQ0Mgb3IgVENT UiAoVENTUiBzaW5jZSBTTTg1NTApCj4gPiA+ID4gCj4gPiA+ID4gTVNNODk5NiBvbmx5IHJlcXVp cmVzICdyZWYnIGFuZCAncXJlZicgY2xvY2tzLgo+ID4gPiA+IAo+ID4gPiA+IEhlbmNlLCB0aGlz IHNlcmllcyBmaXhlcyB0aGUgYmluZGluZywgRFQgYW5kIEdDQyBkcml2ZXIgdG8gcmVmbGVjdCB0 aGUKPiA+ID4gPiBhY3R1YWwgY2xvY2sgdG9wb2xvZ3kuCj4gPiA+IAo+ID4gPiBJcyB0aGlzIGJh c2VkIG9uIGRvY3VtZW50YXRpb24gZm9yIGFsbCB0aGUgU29DcyBvciBvbiBpbmZlcmVuY2UgZnJv bSB0aGUKPiA+ID4gY3VycmVudCAodXBzdHJlYW0gYW5kIGRvd25zdHJlYW0pIGRldmljZXRyZWVz Pwo+ID4gCj4gPiBJdCBpcyBiYXNlZCBvbiB0aGUgaW50ZXJuYWwgZG9jdW1lbnRhdGlvbi4gRXZl biBkb3duc3RyZWFtIGRldmljZXRyZWVzIGFyZQo+ID4gd3JvbmcuIEkgc2hvdWxkJ3ZlIG1lbnRp b25lZCBpdCBpbiB0aGUgY292ZXIgbGV0dGVyLgo+ID4gCj4gPiA+IEFyZSB5b3Ugc3VyZSB0aGF0 IHlvdSBzaG91bGQgbm90IGp1c3QgZGVzY3JpYmUgdGhhdCBzb21lIG9mIHRoZXNlIFVGUwo+ID4g PiByZWZlcmVuY2UgY2xvY2tzIGFyZSBzb3VyY2VkIGZyb20gQ1hPIGluIHRoZSBjbG9jayBkcml2 ZXIgaW5zdGVhZD8KPiA+IAo+ID4gSSBkb24ndCBnZXQgeW91ciBjb21tZW50IGZ1bGx5LiBDb3Vs ZCB5b3UgcGxlYXNlIGVsYWJvcmF0ZT8KPiAKPiBVbmxlc3MgdGhlIFBIWSBjb25zdW1lcyBDWE8g ZGlyZWN0bHksIGl0IHNob3VsZCBub3QgYmUgaW5jbHVkZWQgaW4gdGhlCj4gYmluZGluZyBhcyB5 b3UgYXJlIHN1Z2dlc3RpbmcgaGVyZS4KPiAKClBIWSBpcyBpbmRlZWQgZGlyZWN0bHkgY29uc3Vt aW5nIENYTy4gVGhhdCdzIHdoeSBJIGluY2x1ZGVkIGl0IGluIHRoZSBiaW5kaW5nLgoKPiBXZSBk aXNjdXNzZWQgdGhpcyBhdCBzb21lIGxlbmd0aCBhdCB0aGUgdGltZSB3aXRoIEJqb3JuIGFuZCBT aGF6YWQgd2hvCj4gaGFkIGFjY2VzcyB0byB0aGUgZG9jdW1lbnRhdGlvbiBhbmQgdGhlIGNvbmNs dXNpb24gd2FzIHRoYXQsIGF0IGxlYXN0IG9uCj4gc2M4MjgweHAsIHRoZSBQSFkgZG9lcyBub3Qg dXNlIENYTyBkaXJlY3RseSBhbmQgaW5zdGVhZCBpdCBzaG91bGQgYmUKPiBkZXNjcmliZWQgYXMg YSBwYXJlbnQgdG8gdGhlIFVGUyByZWZjbG9ja3MgaW4gdGhlIGNsb2NrIGRyaXZlcjoKPiAKPiAJ aHR0cHM6Ly9sb3JlLmtlcm5lbC5vcmcvbGttbC9ZMk9Fak5BUFhnNUJmT3hIQGhvdm9sZGNvbnN1 bHRpbmcuY29tLwo+IAo+IFRoZSBkb3duc3RyZWFtIGRldmljZXRyZWVzIGhhdmUgYSBiYWQgaGFi aXQgb2YgaW5jbHVkaW5nIHBhcmVudCBjbG9ja3MKPiBkaXJlY3RseSBpbiB0aGUgY29uc3VtZXIg bm9kZSBpbnN0ZWFkIG9mIG1vZGVsbGluZyB0aGlzIGluIGNsb2NrIGRyaXZlcgo+IGFsc28gZm9y IG90aGVyIHBlcmlwaGVyYWxzLgo+ICAKCk5vLCBJIGNhbiBhc3N1cmUgdGhhdCB5b3UgZ290IHRo ZSB3cm9uZyBpbmZvLiBVRlMgUEhZIGNvbnN1bWVzIHRoZSBjbG9jawpkaXJlY3RseSBmcm9tIFJQ TWguIEl0IHRvb2sgbWUgc2V2ZXJhbCBkYXlzIHRvIGRpZyB0aHJvdWdoIHRoZSBVRlMgYW5kIFBI WSBkb2NzCmFuZCBzcGVjaWFsIHRoYW5rcyB0byBDYW4gR3VvIGZyb20gVUZTIHRlYW0sIHdobyBw cm92aWRlZCBtdWNoIHZhbHVhYmxlCmluZm9ybWF0aW9uIGFib3V0IHRoZXNlIGNsb2Nrcy4KCj4g PiA+IFRha2UgYSBsb29rIGF0IGNvbW1pdHMKPiA+ID4gCj4gPiA+IAlmNDQ2MDIyYjkzMmEgKCJh cm02NDogZHRzOiBxY29tOiBzYzgyODB4cDogZml4IFVGUyByZWZlcmVuY2UgY2xvY2tzIikKPiA+ ID4gCWY2YWJjYzIxZDk0MyAoImNsazogcWNvbTogZ2NjLXNjODI4MHhwOiBhZGQgY3hvIGFzIHBh cmVudCBmb3IgdGhyZWUgdWZzIHJlZiBjbGtzIikKPiA+IAo+ID4gQnR3LCB0aGVzZSBjb21taXRz IGFyZSBub3QgYWNjdXJhdGUuIEluIGFsbCB0aGUgU29DcyBiZWZvcmUgU004NTUwLCByZWZlcmVu Y2UKPiA+IGNsb2NrIGZvciB0aGUgVUZTIGRldmljZSBjb21lcyBmcm9tIHRoZSBVRlMgY29udHJv bGxlci4gVGhlcmUgaXMgYSBkZWRpY2F0ZWQKPiA+IHJlZ2lzdGVyIGluIFVGU0hDIG1lbW9yeSBt YXAgdGhhdCBpcyBiZWluZyB0b2dnbGVkIGJ5IHRoZSBkcml2ZXIgdG8KPiA+IGVuYWJsZS9kaXNh YmxlIHJlZmVyZW5jZSBjbG9jayBmb3IgdGhlIFVGUyBkZXZpY2UuCj4gPgo+ID4gU2luY2UgU004 NTUwLCByZWZlcmVuY2UgY2xvY2sgaXMgZGlyZWN0bHkgc291cmNlZCBmcm9tIFJQTWguIEknbSBw cmVwYXJpbmcgYQo+ID4gc2VyaWVzIHRvIGZpeCBpdC4KPiAKPiBXaGF0IGV4YWN0bHkgaXMgd3Jv bmcgd2l0aCB0aG9zZSBjb21taXRzPyBXZSBrbm93IHRoYXQgdGhlIGNvbnRyb2xsZXIKPiBkb2Vz IG5vdCBjb25zdW1lIEdDQ19VRlNfUkVGX0NMS1JFRl9DTEsgZGlyZWN0bHksIGJ1dCBkZXNjcmli aW5nIHRoYXQgYXMKPiBzdWNoIGZvciBub3cgd2FzIGEgZGVsaWJlcmF0ZSBjaG9pY2U6Cj4gCj4g CUdDQ19VRlNfUkVGX0NMS1JFRl9DTEsgaXMgdGhlIGNsb2NrIHRvIHRoZSBkZXZpY2VzLCBidXQg YXMgd2UKPiAJZG9uJ3QgcmVwcmVzZW50IHRoZSBtZW1vcnkgZGV2aWNlIGV4cGxpY2l0bHkgaXQg c2VlbXMgc3VpdGFibGUKPiAJdG8gdXNlIGFzICJyZWZfY2xrIiBpbiB0aGUgdWZzaGMgbm9kZXMg LSB3aGljaCB3b3VsZCB0aGVuIG1hdGNoCj4gCXRoZSBzcGVjaWFsIGhhbmRsaW5nIG9mIHRoZSAi bGluayBjbG9jayIgaW4gdGhlIFVGUyBkcml2ZXIuCj4gIAoKTm8sIEdDQ19VRlNfUkVGX0NMS1JF Rl9DTEsgaXMgX25vdF8gdGhlIGNsb2NrIHRvIFVGUyBkZXZpY2VzLiBJIGhhdmVuJ3QgZm91bmQK aW5mb3JtYXRpb24gYWJvdXQgdGhpcyBzcGVjaWZpYyByZWdpc3RlciBpbiBHQ0MuIEluaXRpYWxs eSBJIHRob3VnaHQgdGhpcyBpcyBmb3IKZW5hYmxpbmcgUVJFRiBjbG9ja3MgZm9yIHRoZSBVRlMg TUVNIHBoeSwgYnV0IEkgaGF2ZW4ndCBmb3VuZCB0aGUgYW5zd2VyIHlldC4KCkJ1dCBhcyBJIHNh aWQgZWFybGllciwgcmVmZXJlbmNlIGNsb2NrIHRvIFVGUyBkZXZpY2VzIGNvbWVzIGRpcmVjdGx5 IGZyb20gdGhlCmNvbnRyb2xsZXIgYW5kIHRoZXJlIGlzIGEgc3BlY2ZpYyByZWdpc3RlciBmb3Ig Y29udHJvbGxpbmcgdGhhdC4gU3RhcnRpbmcgZnJvbQpTTTg1NTAsIHJlZmVyZW5jZSBjbG9jayBj b21lcyBmcm9tIFJQTWguCgo+ID4gVW5mb3J0dW5hdGVseSwgdGhpcyBpbmZvcm1hdGlvbiBpcyBu b3QgZGVwaWN0ZWQgY29ycmVjdGx5IGluIHRoZSBkb3duc3RyZWFtCj4gPiBkZXZpY2V0cmVlcy4K PiAKPiBJIHdhcyBob3BpbmcgdGhlIGluZm9ybWF0aW9uIHRoYXQgdGhvc2UgY29tbWl0cyBhcmUg YmFzZWQgb24gd291bGQgYmUKPiBjb3JyZWN0IGFzIGl0IGNhbWUgZnJvbSBRdWFsY29tbSBhbmQg Qmpvcm4uIEkgaGF2ZSBubyBpbGx1c2lvbnMgYWJvdXQKPiB0aGUgZG93bnN0cmVhbSBkZXZpY2V0 cmVlcyBiZWluZyBjb3JyZWN0LiA6KQo+IAoKVW5mb3J0dW5hdGVseSwgeW91IGdvdCBpbmFjY3Vy YXRlIGluZm9ybWF0aW9uLiBCdXQgdGhhdCdzIHZlcnkgY29tbW9uLCBzaW5jZQp0aGVzZSBraW5k IG9mIGluZm8gYXJlIGJ1cmllZCBkb3duIGluIHNvbWUgZG9jcyBhbmQgcGVvcGxlJ3MgbWluZCA6 KQoKLSBNYW5pCgo+IEpvaGFuCgotLSAK4K6u4K6j4K6/4K614K6j4K+N4K6j4K6p4K+NIOCumuCu pOCuvuCumuCuv+CuteCuruCvjQoKLS0gCmxpbnV4LXBoeSBtYWlsaW5nIGxpc3QKbGludXgtcGh5 QGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cHM6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4v bGlzdGluZm8vbGludXgtcGh5Cg==