diff mbox series

[08/20] phy: qcom-qmp-combo: clean up status polling

Message ID 20221012085002.24099-8-johan+linaro@kernel.org
State Accepted
Commit f7075f4905e79e340b0e5f0f097c8ce896be8bb3
Headers show
Series phy: qcom-qmp: further prep fixes and cleanups (set 3) | expand

Commit Message

Johan Hovold Oct. 12, 2022, 8:49 a.m. UTC 
Clean up the PHY status polling by dropping the configuration mask which
is no longer needed since the QMP driver split.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

Comments

Dmitry Baryshkov Oct. 12, 2022, 9:14 p.m. UTC | #1 
On 12/10/2022 11:49, Johan Hovold wrote:
> Clean up the PHY status polling by dropping the configuration mask which
> is no longer needed since the QMP driver split.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

This comes in expectation that 4.20 / 5.20 PHYs will be limited to PCIe. 
I'd add this to the commit message. With that fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 14 ++------------
>   1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 82055d3a3536..e312cad6d9c2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -853,8 +853,6 @@ struct qmp_phy_cfg {
>   
>   	unsigned int start_ctrl;
>   	unsigned int pwrdn_ctrl;
> -	/* bit offset of PHYSTATUS in QPHY_PCS_STATUS register */
> -	unsigned int phy_status;
>   
>   	/* true, if PHY needs delay after POWER_DOWN */
>   	bool has_pwrdn_delay;
> @@ -1023,7 +1021,6 @@ static const struct qmp_phy_cfg sc7180_usb3phy_cfg = {
>   
>   	.start_ctrl		= SERDES_START | PCS_START,
>   	.pwrdn_ctrl		= SW_PWRDN,
> -	.phy_status		= PHYSTATUS,
>   
>   	.has_pwrdn_delay	= true,
>   };
> @@ -1092,7 +1089,6 @@ static const struct qmp_phy_cfg sdm845_usb3phy_cfg = {
>   
>   	.start_ctrl		= SERDES_START | PCS_START,
>   	.pwrdn_ctrl		= SW_PWRDN,
> -	.phy_status		= PHYSTATUS,
>   
>   	.has_pwrdn_delay	= true,
>   };
> @@ -1127,7 +1123,6 @@ static const struct qmp_phy_cfg sm8150_usb3phy_cfg = {
>   
>   	.start_ctrl		= SERDES_START | PCS_START,
>   	.pwrdn_ctrl		= SW_PWRDN,
> -	.phy_status		= PHYSTATUS,
>   
>   	.has_pwrdn_delay	= true,
>   };
> @@ -1197,7 +1192,6 @@ static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = {
>   
>   	.start_ctrl		= SERDES_START | PCS_START,
>   	.pwrdn_ctrl		= SW_PWRDN,
> -	.phy_status		= PHYSTATUS,
>   };
>   
>   static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = {
> @@ -1267,7 +1261,6 @@ static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
>   
>   	.start_ctrl		= SERDES_START | PCS_START,
>   	.pwrdn_ctrl		= SW_PWRDN,
> -	.phy_status		= PHYSTATUS,
>   
>   	.has_pwrdn_delay	= true,
>   };
> @@ -2017,7 +2010,7 @@ static int qmp_combo_power_on(struct phy *phy)
>   	void __iomem *rx = qphy->rx;
>   	void __iomem *pcs = qphy->pcs;
>   	void __iomem *status;
> -	unsigned int mask, val, ready;
> +	unsigned int val;
>   	int ret;
>   
>   	qmp_combo_serdes_init(qphy);
> @@ -2059,10 +2052,7 @@ static int qmp_combo_power_on(struct phy *phy)
>   		qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
>   
>   		status = pcs + cfg->regs[QPHY_PCS_STATUS];
> -		mask = cfg->phy_status;
> -		ready = 0;
> -
> -		ret = readl_poll_timeout(status, val, (val & mask) == ready, 10,
> +		ret = readl_poll_timeout(status, val, !(val & PHYSTATUS), 10,
>   					 PHY_INIT_COMPLETE_TIMEOUT);
>   		if (ret) {
>   			dev_err(qmp->dev, "phy initialization timed-out\n");
Johan Hovold Oct. 17, 2022, 6:55 a.m. UTC | #2 
On Thu, Oct 13, 2022 at 12:14:17AM +0300, Dmitry Baryshkov wrote:
> On 12/10/2022 11:49, Johan Hovold wrote:
> > Clean up the PHY status polling by dropping the configuration mask which
> > is no longer needed since the QMP driver split.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> This comes in expectation that 4.20 / 5.20 PHYs will be limited to PCIe. 
> I'd add this to the commit message.

I don't think that's needed. The commit message is correct as it stands,
and the fact that we may need to change code in case we add support for
new devices that may need it is quite natural.

Also note that the vendor drivers (5.4) only have such a provision for
PCIe PHYs and not for the combo and USB PHY drivers.

> With that fixed:
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Johan
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 82055d3a3536..e312cad6d9c2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -853,8 +853,6 @@  struct qmp_phy_cfg {
 
 	unsigned int start_ctrl;
 	unsigned int pwrdn_ctrl;
-	/* bit offset of PHYSTATUS in QPHY_PCS_STATUS register */
-	unsigned int phy_status;
 
 	/* true, if PHY needs delay after POWER_DOWN */
 	bool has_pwrdn_delay;
@@ -1023,7 +1021,6 @@  static const struct qmp_phy_cfg sc7180_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.phy_status		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 };
@@ -1092,7 +1089,6 @@  static const struct qmp_phy_cfg sdm845_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.phy_status		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 };
@@ -1127,7 +1123,6 @@  static const struct qmp_phy_cfg sm8150_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.phy_status		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 };
@@ -1197,7 +1192,6 @@  static const struct qmp_phy_cfg sc8280xp_usb43dp_usb_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.phy_status		= PHYSTATUS,
 };
 
 static const struct qmp_phy_cfg sc8280xp_usb43dp_dp_cfg = {
@@ -1267,7 +1261,6 @@  static const struct qmp_phy_cfg sm8250_usb3phy_cfg = {
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN,
-	.phy_status		= PHYSTATUS,
 
 	.has_pwrdn_delay	= true,
 };
@@ -2017,7 +2010,7 @@  static int qmp_combo_power_on(struct phy *phy)
 	void __iomem *rx = qphy->rx;
 	void __iomem *pcs = qphy->pcs;
 	void __iomem *status;
-	unsigned int mask, val, ready;
+	unsigned int val;
 	int ret;
 
 	qmp_combo_serdes_init(qphy);
@@ -2059,10 +2052,7 @@  static int qmp_combo_power_on(struct phy *phy)
 		qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
 
 		status = pcs + cfg->regs[QPHY_PCS_STATUS];
-		mask = cfg->phy_status;
-		ready = 0;
-
-		ret = readl_poll_timeout(status, val, (val & mask) == ready, 10,
+		ret = readl_poll_timeout(status, val, !(val & PHYSTATUS), 10,
 					 PHY_INIT_COMPLETE_TIMEOUT);
 		if (ret) {
 			dev_err(qmp->dev, "phy initialization timed-out\n");