LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Vivek Gautam <vivek.gautam@codeaurora.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	cang@codeaurora.org, Evan Green <evgreen@chromium.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	sayalil@codeaurora.org, Asutosh Das <asutoshd@codeaurora.org>,
	devicetree@vger.kernel.org, liwei213@huawei.com,
	LKML <linux-kernel@vger.kernel.org>,
	Mathieu Malaterre <malat@debian.org>,
	Mark Rutland <mark.rutland@arm.com>
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> (raw)
In-Reply-To: <CAD=FV=XSjfRSHP6B6GtiTqrQuUZaoA=hZNW_L1=71MU9fEXpDQ@mail.gmail.com>



On 10/17/2018 9:41 PM, Doug Anderson wrote:
> Hi,
>
> On Tue, Oct 16, 2018 at 11:28 PM Vivek Gautam
> <vivek.gautam@codeaurora.org> 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
>>> <vivek.gautam@codeaurora.org> 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


  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 21:39 Douglas Anderson
2018-10-15 15:23 ` Vivek Gautam
2018-10-15 17:23   ` Doug Anderson
2018-10-16  5:51     ` Vivek Gautam
2018-10-16 16:59       ` Doug Anderson
2018-10-17  6:28         ` Vivek Gautam
2018-10-17 16:11           ` Doug Anderson
2018-10-18  7:52             ` Vivek Gautam [this message]
2019-01-23 22:17               ` Evan Green
2018-10-18  0:42 ` Rob Herring
2019-02-06 13:59 ` Marc Gonzalez
2019-02-08 22:40   ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20cf6f02-ff3c-0904-2edc-a1b72c135866@codeaurora.org \
    --to=vivek.gautam@codeaurora.org \
    --cc=asutoshd@codeaurora.org \
    --cc=cang@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liwei213@huawei.com \
    --cc=malat@debian.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.petersen@oracle.com \
    --cc=robh+dt@kernel.org \
    --cc=sayalil@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git