On Fri, Jan 12, 2018 at 2:16 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote:
> Hi Vivek,
>
>
> On 1/12/2018 2:14 PM, Vivek Gautam wrote:
>> On Wed, Jan 3, 2018 at 4:58 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote:
>>> PHY block or asynchronous reset requires signal
>>> to be asserted before de-asserting. Driver is only
>>> de-asserting signal which is already low, hence
>>> reset operation is a no-op. Fix this by asserting
>>> signal first. Also, resetting requires PHY clocks
>>> to be turned ON only after reset is finished. Fix
>>> that as well.
>>>
>>> Signed-off-by: Manu Gautam <mgautam@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/phy/qualcomm/phy-qcom-qmp.c | 28 +++++++++++++++++++---------
>>> 1 file changed, 19 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
>>> index 1b82cea..ecff261 100644
>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
>>> @@ -752,13 +752,16 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>> goto err_reg_enable;
>>> }
>>>
>>> - ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
>>> - if (ret) {
>>> - dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
>>> - goto err_clk_enable;
>>> + for (i = 0; i < cfg->num_resets; i++) {
>>> + ret = reset_control_assert(qmp->resets[i]);
>>> + if (ret) {
>>> + dev_err(qmp->dev, "%s reset assert failed\n",
>>> + cfg->reset_list[i]);
>>> + goto err_rst_assert;
>>> + }
>>> }
>>>
>>> - for (i = 0; i < cfg->num_resets; i++) {
>>> + for (i = cfg->num_resets - 1; i >= 0; i--) {
>> Do we a dependency on the order in which these resets are
>> applied?
>> If not then we can use the 'bulk reset' APIs as well.
>
> We need to follow an order for assert and opposite order for
> de-assert, hence cant use 'bulk reset' APIs.
Okay, you can add my review then.
Thanks.
>
>>
>> With that bulk reset change you can add my review.
>>
>> Reviewed-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx>
>>
>> Thanks
>> Vivek
>>
>>> ret = reset_control_deassert(qmp->resets[i]);
>>> if (ret) {
>>> dev_err(qmp->dev, "%s reset deassert failed\n",
>>> @@ -767,6 +770,12 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>> }
>>> }
>>>
>>> + ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
>>> + if (ret) {
>>> + dev_err(qmp->dev, "failed to enable clks, err=%d\n", ret);
>>> + goto err_rst;
>>> + }
>>> +
>>> if (cfg->has_phy_com_ctrl)
>>> qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
>>> SW_PWRDN);
>>> @@ -791,7 +800,7 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>> if (ret) {
>>> dev_err(qmp->dev,
>>> "phy common block init timed-out\n");
>>> - goto err_rst;
>>> + goto err_com_init;
>>> }
>>> }
>>>
>>> @@ -799,11 +808,12 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
>>>
>>> return 0;
>>>
>>> +err_com_init:
>>> + clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks);
>>> err_rst:
>>> - while (--i >= 0)
>>> + while (++i < cfg->num_resets)
>>> reset_control_assert(qmp->resets[i]);
>>> - clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks);
>>> -err_clk_enable:
>>> +err_rst_assert:
>>> regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
>>> err_reg_enable:
>>> mutex_unlock(&qmp->phy_mutex);
>>> --
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>> a Linux Foundation Collaborative Project
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
