Hi Subhash,
On Wed, Sep 27, 2017 at 4:43 AM, Subhash Jadavani
<subhashj@xxxxxxxxxxxxxx> wrote:
> Hi Vivek,
>
> Please find one comment inline below, rest look good.
>
> Regards,
> Subhash
>
>
> On 2017-08-03 23:48, Vivek Gautam wrote:
>>
>> Refactor ufs_qcom_power_up_sequence() to get rid of ugly
>> exported phy APIs and use the phy_init() and phy_power_on()
>> to do the phy initialization.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx>
>> ---
>> drivers/phy/qualcomm/phy-qcom-ufs-i.h | 2 --
>> drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 9 +++++--
>> drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 9 +++++--
>> drivers/phy/qualcomm/phy-qcom-ufs.c | 38
>> ++++++++--------------------
>> drivers/scsi/ufs/ufs-qcom.c | 36
>> ++++++++++----------------
>> include/linux/phy/phy-qcom-ufs.h | 3 ---
>> 6 files changed, 38 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> index 94326ed107c3..495fd5941231 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> @@ -123,7 +123,6 @@ struct ufs_qcom_phy {
>> * struct ufs_qcom_phy_specific_ops - set of pointers to functions which
>> have a
>> * specific implementation per phy. Each UFS phy, should implement
>> * those functions according to its spec and requirements
>> - * @calibrate_phy: pointer to a function that calibrate the phy
>> * @start_serdes: pointer to a function that starts the serdes
>> * @is_physical_coding_sublayer_ready: pointer to a function that
>> * checks pcs readiness. returns 0 for success and non-zero for error.
>> @@ -132,7 +131,6 @@ struct ufs_qcom_phy {
>> * and writes to QSERDES_RX_SIGDET_CNTRL attribute
>> */
>> struct ufs_qcom_phy_specific_ops {
>> - int (*calibrate_phy)(struct ufs_qcom_phy *phy, bool is_rate_B);
>> void (*start_serdes)(struct ufs_qcom_phy *phy);
>> int (*is_physical_coding_sublayer_ready)(struct ufs_qcom_phy
>> *phy);
>> void (*set_tx_lane_enable)(struct ufs_qcom_phy *phy, u32 val);
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> index af65785230b5..c39440b56b6d 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> @@ -44,7 +44,13 @@ void ufs_qcom_phy_qmp_14nm_advertise_quirks(struct
>> ufs_qcom_phy *phy_common)
>>
>> static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy)
>> {
>> - return 0;
>> + struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
>> + bool is_rate_B = false;
>> +
>> + if (phy_common->mode == PHY_MODE_UFS_HS_B)
>> + is_rate_B = true;
>> +
>> + return ufs_qcom_phy_qmp_14nm_phy_calibrate(phy_common, is_rate_B);
>> }
>>
>> static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)
>> @@ -120,7 +126,6 @@ static int
>> ufs_qcom_phy_qmp_14nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
>> };
>>
>> static struct ufs_qcom_phy_specific_ops phy_14nm_ops = {
>> - .calibrate_phy = ufs_qcom_phy_qmp_14nm_phy_calibrate,
>> .start_serdes = ufs_qcom_phy_qmp_14nm_start_serdes,
>> .is_physical_coding_sublayer_ready =
>> ufs_qcom_phy_qmp_14nm_is_pcs_ready,
>> .set_tx_lane_enable =
>> ufs_qcom_phy_qmp_14nm_set_tx_lane_enable,
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> index 5c18c41dbdb4..5705a2d4c6d2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> @@ -63,7 +63,13 @@ void ufs_qcom_phy_qmp_20nm_advertise_quirks(struct
>> ufs_qcom_phy *phy_common)
>>
>> static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy)
>> {
>> - return 0;
>> + struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
>> + bool is_rate_B = false;
>> +
>> + if (phy_common->mode == PHY_MODE_UFS_HS_B)
>> + is_rate_B = true;
>> +
>> + return ufs_qcom_phy_qmp_20nm_phy_calibrate(phy_common, is_rate_B);
>> }
>>
>> static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)
>> @@ -178,7 +184,6 @@ static int
>> ufs_qcom_phy_qmp_20nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
>> };
>>
>> static struct ufs_qcom_phy_specific_ops phy_20nm_ops = {
>> - .calibrate_phy = ufs_qcom_phy_qmp_20nm_phy_calibrate,
>> .start_serdes = ufs_qcom_phy_qmp_20nm_start_serdes,
>> .is_physical_coding_sublayer_ready =
>> ufs_qcom_phy_qmp_20nm_is_pcs_ready,
>> .set_tx_lane_enable =
>> ufs_qcom_phy_qmp_20nm_set_tx_lane_enable,
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs.c
>> index 43865ef340e2..1febe3294fe3 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs.c
>> @@ -518,9 +518,8 @@ void ufs_qcom_phy_disable_iface_clk(struct
>> ufs_qcom_phy *phy)
>> }
>> }
>>
>> -int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
>> +static int ufs_qcom_phy_start_serdes(struct ufs_qcom_phy *ufs_qcom_phy)
>> {
>> - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
>> int ret = 0;
>>
>> if (!ufs_qcom_phy->phy_spec_ops->start_serdes) {
>> @@ -533,7 +532,6 @@ int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
>>
>> return ret;
>> }
>> -EXPORT_SYMBOL_GPL(ufs_qcom_phy_start_serdes);
>>
>> int ufs_qcom_phy_set_tx_lane_enable(struct phy *generic_phy, u32
>> tx_lanes)
>> {
>> @@ -564,31 +562,8 @@ void ufs_qcom_phy_save_controller_version(struct
>> phy *generic_phy,
>> }
>> EXPORT_SYMBOL_GPL(ufs_qcom_phy_save_controller_version);
>>
>> -int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B)
>> -{
>> - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
>> - int ret = 0;
>> -
>> - if (!ufs_qcom_phy->phy_spec_ops->calibrate_phy) {
>> - dev_err(ufs_qcom_phy->dev, "%s: calibrate_phy() callback
>> is not supported\n",
>> - __func__);
>> - ret = -ENOTSUPP;
>> - } else {
>> - ret = ufs_qcom_phy->phy_spec_ops->
>> - calibrate_phy(ufs_qcom_phy, is_rate_B);
>> - if (ret)
>> - dev_err(ufs_qcom_phy->dev, "%s: calibrate_phy()
>> failed %d\n",
>> - __func__, ret);
>> - }
>> -
>> - return ret;
>> -}
>> -EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy);
>> -
>> -int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy)
>> +static int ufs_qcom_phy_is_pcs_ready(struct ufs_qcom_phy *ufs_qcom_phy)
>> {
>> - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
>> -
>> if
>> (!ufs_qcom_phy->phy_spec_ops->is_physical_coding_sublayer_ready) {
>> dev_err(ufs_qcom_phy->dev, "%s:
>> is_physical_coding_sublayer_ready()
>> callback is not supported\n",
>> __func__);
>> @@ -598,7 +573,6 @@ int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy)
>> return ufs_qcom_phy->phy_spec_ops->
>> is_physical_coding_sublayer_ready(ufs_qcom_phy);
>> }
>> -EXPORT_SYMBOL_GPL(ufs_qcom_phy_is_pcs_ready);
>>
>> int ufs_qcom_phy_power_on(struct phy *generic_phy)
>> {
>> @@ -609,6 +583,14 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy)
>> if (phy_common->is_powered_on)
>> return 0;
>>
>> + err = ufs_qcom_phy_start_serdes(phy_common);
>> + if (err)
>> + return err;
>> +
>> + err = ufs_qcom_phy_is_pcs_ready(phy_common);
>> + if (err)
>> + return err;
>> +
>
>
> Now that we are doing serdes start (and checking the PCS ready), I am not
> sure we can call phy_power_on() from ufs_qcom_resume(). Please check.
>
Right. Thanks for catching this. I will check this further.
BRs
Vivek
[snip]
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
