linux-kernel - Re: [PATCH 1/1] scsi/ufs: qcom: Don't enable PHY_QCOM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <715f93be-611d-ae04-42e5-8a4b33ac2cce@codeaurora.org>
Date:   Fri, 20 Apr 2018 00:21:56 +0530
From:   Vivek Gautam <vivek.gautam@...eaurora.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     martin.petersen@...cle.com, kishon@...com, jejb@...ux.vnet.ibm.com,
        subhashj@...eaurora.org, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        "cang@...eaurora.org" <cang@...eaurora.org>
Subject: Re: [PATCH 1/1] scsi/ufs: qcom: Don't enable PHY_QCOM_UFS by default

Hi Bjorn,


On 4/18/2018 4:41 AM, Bjorn Andersson wrote:
> 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).

Right, i get your concern. I will try to refactor the UFS HCI code to 
handle the two
'types' of phys.
I think Can Guo (CC'ed here) was already working on this. I will check 
with him
if he already has some code to do this.

Thanks
Vivek
>
>> On db820c, we can still work with the ufs_qcom_phy.
>>
> I do not have an issue with that.
>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html