From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vivek Gautam Subject: Re: [PATCH] dt-bindings: ufs: Fix the compatible string definition Date: Thu, 18 Oct 2018 13:22:07 +0530 Message-ID: <20cf6f02-ff3c-0904-2edc-a1b72c135866@codeaurora.org> References: <20181012213926.253765-1-dianders@chromium.org> <1ce7b24c-b154-4ce6-2b4c-9eb0fd0d71cb@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Doug Anderson Cc: Rob Herring , "Martin K. Petersen" , cang@codeaurora.org, Evan Green , linux-arm-msm , sayalil@codeaurora.org, Asutosh Das , devicetree@vger.kernel.org, liwei213@huawei.com, LKML , Mathieu Malaterre , Mark Rutland List-Id: devicetree@vger.kernel.org On 10/17/2018 9:41 PM, Doug Anderson wrote: > Hi, > > On Tue, Oct 16, 2018 at 11:28 PM Vivek Gautam > wrote: >> Hi Doug, >> >> >> On 10/16/2018 10:29 PM, Doug Anderson wrote: >>> Hi, >>> >>> On Mon, Oct 15, 2018 at 10:51 PM Vivek Gautam >>> wrote: >>>>>> P.S.: While you are at it, can you please move 'ufs-qcom.txt' >>>>>> to Documentation/devicetree/bindings/phy/qcom,ufs-phy.txt. >>>>>> The current name and file location is misleading. >>>>> I'd rather someone at Qualcomm do this. Do you have a suggested >>>>> person? The reason I feel that Qualcomm needs to get involved is that >>>>> I see that when I look at the file you refer to says it's for: >>>>> >>>>> "qcom,ufs-phy-qmp-20nm" for 20nm ufs phy, >>>>> "qcom,ufs-phy-qmp-14nm" for legacy 14nm ufs phy, >>>>> "qcom,msm8996-ufs-phy-qmp-14nm" for 14nm ufs phy >>>>> present on MSM8996 chipset. >>>>> >>>>> ...but there's another Qualcomm file, 'qcom-qmp-phy.txt'. That >>>>> handles the compatible string: >>>>> >>>>> "qcom,sdm845-qmp-ufs-phy" for UFS QMP phy on sdm845. >>>>> >>>>> So I'm a little confused. Should the SDM845 UFS PHY been handled by >>>>> the older UFS PHY driver? ...or should all the older UFS PHYs be >>>>> moved to be handled by the newer QMP PHY driver? ...or are they >>>>> really different hardware blocks, in which case how would you describe >>>>> the difference (both are described as UFS QMP PHYs I think). >>>> As you rightly said "ufs/ufs-qcom.txt" describes the bindings for >>>> 14nm, and 20nm ufs phy. These phys are however handled by the older >>>> ufs phy driver present at: >>>> drivers/phy/qualcomm/phy-qcom-ufs-qmp-{14nm,20nm}.c >>>> The sdm845 UFS phy driver is handled by the new consolidated qmp phy >>>> driver: drivers/phy/qualcomm/phy-qcom-qmp.c whose bindings are >>>> described by 'qcom-qmp-phy.txt'. >>>> We didn't attempt to move the 14nm phy to new driver as we already had >>>> 8996 using the bindings. >>>> >>>> So, really these are two separate drivers with different bindings. I >>>> believe it should be okay to move the file. If you are fine, I can >>>> attempt to post a small patch to do that. >>> I guess what I should have said was that the new name you're proposing >>> "qcom,ufs-phy.txt" is confusing and opening the file doesn't help >>> clarify things. The name and the binding make it sound like this is >>> _the_ file to look at for Qualcomm UFS PHYs. ...and then you look in >>> the examples in the file and it seems that this even includes Qualcomm >>> QMP PHYs for UFS. >>> >>> ...so while I agree that the file "ufs-qcom.txt" needs to be moved to >>> the "PHY" directory, I think at the same time we need to change the >>> name of the file and maybe the contents to disambiguate which things >>> belong in this file vs. the "qcom-qmp-phy.txt". ...and I feel like >>> someone at Qualcomm will have the most information to properly do >>> that. >>> >>> For instance, you could call the older bindings >>> "qcom-qmp-phy-14nm-20nm.txt" or something like that. >> Sure, I get your point. I will propose something that removes the confusion. >> >>> One point of clarification I'd like to know is if there's really a >>> good reason to have two drivers here. Certainly if the hardware is >>> really different then a new driver can make sense, but if there are >>> two drivers for arbitrary reasons then maybe they should be combined >>> into one eventually? >> Right, the 14nm phy driver can be happily merged into the new qmp-phy >> driver. >> But we should take care of older bindings. Removing the driver will break >> things on targets with older bindings, precisely 8996. >> >> 20nm is bit tricky as it exported few APIs directly to ufs host >> controller, and >> that's the reason we have declared that as BROKEN after the ufs cleanup. >> So, until we are really in a kill mode, the old ufs-phy driver will have >> to live. > OK, sounds like a plan. I'll assume you're posting the patch to move > the old PHY bindings and add some of the above information to them so > people aren't confused. > > ...all this is sort off the original subject, though. ;-) Just a > quick summary here is that nothing suggests ${SUBJECT} patch shouldn't > land and all the additional discussion has been about making further > improvements to the bindings situation for UFS on Qualcomm. Yes, this patch is good to go. Thanks Vivek > > -Doug