linux-kernel - Re: [PATCH v1 2/6] phy: qcom-qmp: Power-on PHY before initialization

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]

Date:   Wed, 9 Aug 2017 16:21:21 +0530
From:   Manu Gautam <mgautam@...eaurora.org>
To:     Kishon Vijay Abraham I <kishon@...com>,
        Felipe Balbi <balbi@...nel.org>
Cc:     linux-arm-msm@...r.kernel.org,
        Vivek Gautam <vivek.gautam@...eaurora.org>,
        Jaehoon Chung <jh80.chung@...sung.com>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        Fengguang Wu <fengguang.wu@...el.com>,
        "open list:GENERIC PHY FRAMEWORK" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/6] phy: qcom-qmp: Power-on PHY before initialization



On 7/21/2017 4:31 PM, Manu Gautam wrote:
>  }
> @@ -729,19 +771,17 @@ static int qcom_qmp_phy_init(struct phy *phy)
>  	void __iomem *pcs = qphy->pcs;
>  	void __iomem *status;
>  	unsigned int mask, val;
> -	int ret, i;
> +	int ret;
>  
>  	dev_vdbg(qmp->dev, "Initializing QMP phy\n");
>  
> -	for (i = 0; i < qmp->cfg->num_clks; i++) {
> -		ret = clk_prepare_enable(qmp->clks[i]);
> -		if (ret) {
> -			dev_err(qmp->dev, "failed to enable %s clk, err=%d\n",
> -				qmp->cfg->clk_list[i], ret);
> -			while (--i >= 0)
> -				clk_disable_unprepare(qmp->clks[i]);
> -		}
> -	}
> +	ret = qcom_qmp_phy_poweron(qmp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_qmp_phy_enable_clocks(qmp);
> +	if (ret)
> +		goto err_clk_enable;
>  
>  	ret = qcom_qmp_phy_com_init(qmp);

In next version I will move poweron and clock_enable to com_init
as PCI QMP PHY might use multiple PHY instances (for each lane) each of
them will call phy_init/exit.
qcom_qmp_phy_com_init executed only once, hence better
suited for turning ON clocks/regulators.

>  	if (ret)
> @@ -801,8 +841,9 @@ static int qcom_qmp_phy_init(struct phy *phy)
>  err_lane_rst:
>  	qcom_qmp_phy_com_exit(qmp);
>  err_com_init:
> -	while (--i >= 0)
> -		clk_disable_unprepare(qmp->clks[i]);
> +	qcom_qmp_phy_disable_clocks(qmp);
> +err_clk_enable:
> +	qcom_qmp_phy_poweroff(qmp);
>  
>  	return ret;
>  }
> @@ -812,7 +853,6 @@ static int qcom_qmp_phy_exit(struct phy *phy)
>  	struct qmp_phy *qphy = phy_get_drvdata(phy);
>  	struct qcom_qmp *qmp = qphy->qmp;
>  	const struct qmp_phy_cfg *cfg = qmp->cfg;
> -	int i = cfg->num_clks;
>  
>  	clk_disable_unprepare(qphy->pipe_clk);
>  
> @@ -830,8 +870,9 @@ static int qcom_qmp_phy_exit(struct phy *phy)
>  
>  	qcom_qmp_phy_com_exit(qmp);
>  
> -	while (--i >= 0)
> -		clk_disable_unprepare(qmp->clks[i]);
> +	qcom_qmp_phy_disable_clocks(qmp);
> +
> +	qcom_qmp_phy_poweroff(qmp);

On similar lines will be moving these to qcom_qmp_phy_com_exit().
>  
>  	return 0;
>  }
> @@ -958,8 +999,6 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
>  static const struct phy_ops qcom_qmp_phy_gen_ops = {
>  	.init		= qcom_qmp_phy_init,
>  	.exit		= qcom_qmp_phy_exit,
> -	.power_on	= qcom_qmp_phy_poweron,
> -	.power_off	= qcom_qmp_phy_poweroff,
>  	.owner		= THIS_MODULE,
>  };
>  

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project