Messages in this thread | | | Date | Mon, 27 Apr 2020 22:29:19 +0530 | From | Vinod Koul <> | Subject | Re: [PATCH] phy: qcom-snps: Add runtime suspend and resume handlers |
| |
On 23-04-20, 10:26, Wesley Cheng wrote:
> +static int qcom_snps_hsphy_suspend(struct qcom_snps_hsphy *hsphy) > +{ > + if (hsphy->suspended) > + return 0; > + > + dev_dbg(&hsphy->phy->dev, "Suspend QCOM SNPS PHY, mode = %d \n", hsphy->mode); > + > + if (hsphy->mode == PHY_MODE_USB_HOST) { > + /* Enable auto-resume to meet remote wakeup timing */ > + qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_CTRL2, > + USB2_AUTO_RESUME, USB2_AUTO_RESUME); > + usleep_range(500, 1000); > + qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_CTRL2, > + 0, USB2_AUTO_RESUME);
Kernel has a coding guideline where we try to "stick" to 80 char limit and is sometimes okay like debug logs. Above is not okay. Please fix it and run ./scripts/checkpatch.pl --strict on your patch and fix all errors. Warning and checks at your discretion using common sense. When in doubt do ask :)
> + } > + > + clk_disable_unprepare(hsphy->cfg_ahb_clk); > + hsphy->suspended = true;
why do you need to track this?
> + > + return 0; > +} > + > +static int qcom_snps_hsphy_resume(struct qcom_snps_hsphy *hsphy) > +{ > + int ret = 0;
superfluous init..
> static int qcom_snps_hsphy_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -251,6 +333,14 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev) > return ret; > } > > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev);
would it not make sense to enable this after pjy in initialized?
-- ~Vinod
| |