From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753012AbeDQXLP (ORCPT ); Tue, 17 Apr 2018 19:11:15 -0400 Received: from mail-pl0-f65.google.com ([209.85.160.65]:33088 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751857AbeDQXLN (ORCPT ); Tue, 17 Apr 2018 19:11:13 -0400 X-Google-Smtp-Source: AIpwx4/ongJHU5S5kSJGf35TJHNr8SSeDsYZ1sUUoM67OIWjOxEhmOKz15sbZIVs6fiJCUK0cxhENw== Date: Tue, 17 Apr 2018 16:11:09 -0700 From: Bjorn Andersson To: Vivek Gautam 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 Message-ID: <20180417231109.GO18510@minitux> References: <20180409132404.20876-1-vivek.gautam@codeaurora.org> <20180409165127.GK453@tuxbook-pro> <2588cd12-081a-5fad-7ef5-af78b18168eb@codeaurora.org> <20180409200904.GL453@tuxbook-pro> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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