LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Vivek Gautam <vivek.gautam@codeaurora.org>
Cc: martin.petersen@oracle.com, kishon@ti.com,
	jejb@linux.vnet.ibm.com, subhashj@codeaurora.org,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 1/1] scsi/ufs: qcom: Don't enable PHY_QCOM_UFS by default
Date: Tue, 17 Apr 2018 16:11:09 -0700	[thread overview]
Message-ID: <20180417231109.GO18510@minitux> (raw)
In-Reply-To: <fa02c578-a89e-7934-2e88-d3728957ab71@codeaurora.org>

On Mon 09 Apr 23:31 PDT 2018, Vivek Gautam wrote:

> 
> 
> On 4/10/2018 1:39 AM, Bjorn Andersson wrote:
> > On Mon 09 Apr 10:38 PDT 2018, Vivek Gautam wrote:
> > > On 4/9/2018 10:21 PM, Bjorn Andersson wrote:
> > > > On Mon 09 Apr 06:24 PDT 2018, Vivek Gautam wrote:
> > [..]
> > > > > diff --git a/include/linux/phy/phy-qcom-ufs.h b/include/linux/phy/phy-qcom-ufs.h
> > > > > index 0a2c18a9771d..1388c2a2965e 100644
> > > > > --- a/include/linux/phy/phy-qcom-ufs.h
> > > > > +++ b/include/linux/phy/phy-qcom-ufs.h
> > > > > @@ -31,8 +31,21 @@ void ufs_qcom_phy_enable_dev_ref_clk(struct phy *phy);
> > > > >     */
> > > > >    void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
> > > > > +#if IS_ENABLED(CONFIG_PHY_QCOM_UFS)
> > > > >    int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
> > > > >    void ufs_qcom_phy_save_controller_version(struct phy *phy,
> > > > > -			u8 major, u16 minor, u16 step);
> > > > > +					  u8 major, u16 minor, u16 step);
> > > > > +#else
> > > > > +static inline int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes)
> > > > > +{
> > > > > +	return -ENOSYS;
> > > > > +}
> > > > > +
> > > > > +static inline void ufs_qcom_phy_save_controller_version(struct phy *phy,
> > > > > +							u8 major, u16 minor,
> > > > > +							u16 step)
> > > > > +{
> > > > > +}
> > > > > +#endif /* PHY_QCOM_UFS */
> > > > What's the timeline for getting rid of the references to these
> > > > functions? I presume that code depending on these being here will
> > > > compile but won't actually work?
> > > Yes, these inline definitions are just to keep ufs-qcom happy with the
> > > direct
> > > calls that it makes to these functions.
> > > As you would know these couple of functions are just used by the 20nm phy.
> > > However, we don't have any platform yet in the upstream that enables this
> > > phy.
> > > I am hoping that we will eventually get rid of these functions when we
> > > further
> > > clean up ufs-qcom driver.
> > > 
> > I see, but that means that we're calling this function with a struct phy
> > that might not be a struct ufs_qcom_phy and as such a defconfig with
> > both enabled will have undefined outcome for the migrated phys.
> 
> No, we will have to add support for separate phys as sdm845 has phy per each
> lane,
> and the older struct phy will exist alongside.
> We will call this function only with the older phy pointer.
> 
> > In particular we do expect that the same kernel will boot on db820c and
> > sdm845-mtp, so we will have to enable support for the 14nm & 20nm phy
> > driver (and we don't want random crashes because someone happened to
> > enable it).
> 
> Right, so we create new struct phy while keeping older one intact to keep
> the
> ufs-qcom work with both - ufs_qcom_phy and qmp_phy.
> Some of the controller drivers, such as usb/dwc3/ keep support for old and
> new phys,
> although there the difference is between generic phy and the usb-phy.
> So, I am assuming that if we want to keep ufs-qcom on platforms using 20nm,
> 14nm and 10nm phys happy, we will have to keep the phys separately for
> sometime.
> What do you say about it?
> 

My concern is only that the UFS HCI driver doesn't have a way to know if
it's the new or old "type" of phy, but if you can get that working then
I don't have any objections about doing so for a transitional period.

But, you may not use kernel config options to handle this, the same
Image should boot on msm8916, msm8996 and sdm845 (with appropriate dtb
for each one).

> On db820c, we can still work with the ufs_qcom_phy.
> 

I do not have an issue with that.

Regards,
Bjorn

  reply	other threads:[~2018-04-17 23:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-09 13:24 Vivek Gautam
2018-04-09 16:51 ` Bjorn Andersson
2018-04-09 17:38   ` Vivek Gautam
2018-04-09 20:09     ` Bjorn Andersson
2018-04-10  6:31       ` Vivek Gautam
2018-04-17 23:11         ` Bjorn Andersson [this message]
2018-04-19 18:51           ` Vivek Gautam

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=20180417231109.GO18510@minitux \
    --to=bjorn.andersson@linaro.org \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=subhashj@codeaurora.org \
    --cc=vivek.gautam@codeaurora.org \
    --subject='Re: [PATCH 1/1] scsi/ufs: qcom: Don'\''t enable PHY_QCOM_UFS by default' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).