| Message ID | 1371113039-5784-3-git-send-email-kishon@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote: > @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) > otg->start_srp = omap_usb_start_srp; > otg->phy = &phy->phy; > > + pm_runtime_enable(phy->dev); enabling pm_runtime here has the potential to cause a real big problem. How have you tested this patch ?
Hi, On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote: > Hi, > > On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote: >> @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) >> otg->start_srp = omap_usb_start_srp; >> otg->phy = &phy->phy; >> >> + pm_runtime_enable(phy->dev); > > enabling pm_runtime here has the potential to cause a real big problem. > How have you tested this patch ? performed boot and enumeration testing. What issue do you expect here? Thanks Kishon
Hi, On Tue, Jun 18, 2013 at 03:19:03PM +0530, Kishon Vijay Abraham I wrote: > Hi, > > On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote: > >Hi, > > > >On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote: > >>@@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) > >> otg->start_srp = omap_usb_start_srp; > >> otg->phy = &phy->phy; > >> > >>+ pm_runtime_enable(phy->dev); > > > >enabling pm_runtime here has the potential to cause a real big problem. > >How have you tested this patch ? > > performed boot and enumeration testing. What issue do you expect here? hint: look at drvdata usage around the driver. Where is it initialized ? Where is it used ?
Hi, On Tuesday 18 June 2013 03:20 PM, Felipe Balbi wrote: > Hi, > > On Tue, Jun 18, 2013 at 03:19:03PM +0530, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Tuesday 18 June 2013 03:10 PM, Felipe Balbi wrote: >>> Hi, >>> >>> On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote: >>>> @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) >>>> otg->start_srp = omap_usb_start_srp; >>>> otg->phy = &phy->phy; >>>> >>>> + pm_runtime_enable(phy->dev); >>> >>> enabling pm_runtime here has the potential to cause a real big problem. >>> How have you tested this patch ? >> >> performed boot and enumeration testing. What issue do you expect here? > > hint: look at drvdata usage around the driver. Where is it initialized ? > Where is it used ? hmm.. since runtime_get calls weren't made very early, I think I dint see any issues. Thanks for pointing this. Thanks Kishon
Hi, On Tue, Jun 18, 2013 at 03:26:23PM +0530, Kishon Vijay Abraham I wrote: > >>>On Thu, Jun 13, 2013 at 02:13:52PM +0530, Kishon Vijay Abraham I wrote: > >>>>@@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) > >>>> otg->start_srp = omap_usb_start_srp; > >>>> otg->phy = &phy->phy; > >>>> > >>>>+ pm_runtime_enable(phy->dev); > >>> > >>>enabling pm_runtime here has the potential to cause a real big problem. > >>>How have you tested this patch ? > >> > >>performed boot and enumeration testing. What issue do you expect here? > > > >hint: look at drvdata usage around the driver. Where is it initialized ? > >Where is it used ? > > hmm.. since runtime_get calls weren't made very early, I think I dint > see any issues. Thanks for pointing this. right, no problem. BTW, you don't get to_platform_device() -> platform_get_drvdata(). A simple dev_get_drvdata() is sufficient :-)
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c index 844ab68..3c275e7 100644 --- a/drivers/usb/phy/phy-omap-usb2.c +++ b/drivers/usb/phy/phy-omap-usb2.c @@ -28,6 +28,7 @@ #include <linux/pm_runtime.h> #include <linux/delay.h> #include <linux/usb/omap_control_usb.h> +#include <linux/phy/phy.h> /** * omap_usb2_set_comparator - links the comparator present in the sytem with @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend) return 0; } +static int omap_usb_power_off(struct phy *x) +{ + struct omap_usb *phy = dev_get_drvdata(&x->dev); + + omap_control_usb_phy_power(phy->control_dev, 0); + + return 0; +} + +static int omap_usb_power_on(struct phy *x) +{ + struct omap_usb *phy = dev_get_drvdata(&x->dev); + + omap_control_usb_phy_power(phy->control_dev, 1); + + return 0; +} + +static struct phy_ops ops = { + .power_on = omap_usb_power_on, + .power_off = omap_usb_power_off, + .owner = THIS_MODULE, +}; + static int omap_usb2_probe(struct platform_device *pdev) { struct omap_usb *phy; + struct phy *generic_phy; struct usb_otg *otg; + struct phy_provider *phy_provider; phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL); if (!phy) { @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev) phy->phy.otg = otg; phy->phy.type = USB_PHY_TYPE_USB2; + phy_provider = devm_of_phy_provider_register(phy->dev, THIS_MODULE, + of_phy_simple_xlate); + if (IS_ERR(phy_provider)) + return PTR_ERR(phy_provider); + phy->control_dev = omap_get_control_dev(); if (IS_ERR(phy->control_dev)) { dev_dbg(&pdev->dev, "Failed to get control device\n"); @@ -159,6 +191,12 @@ static int omap_usb2_probe(struct platform_device *pdev) otg->start_srp = omap_usb_start_srp; otg->phy = &phy->phy; + pm_runtime_enable(phy->dev); + + generic_phy = devm_phy_create(phy->dev, 0, &ops, "omap-usb2", phy); + if (IS_ERR(generic_phy)) + return PTR_ERR(generic_phy); + phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k"); if (IS_ERR(phy->wkupclk)) { dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n"); @@ -176,8 +214,6 @@ static int omap_usb2_probe(struct platform_device *pdev) platform_set_drvdata(pdev, phy); - pm_runtime_enable(phy->dev); - return 0; }
Used the generic PHY framework API to create the PHY. Now the power off and power on are done in omap_usb_power_off and omap_usb_power_on respectively. However using the old USB PHY library cannot be completely removed because OTG is intertwined with PHY and moving to the new framework will break OTG. Once we have a separate OTG state machine, we can get rid of the USB PHY library. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/usb/phy/phy-omap-usb2.c | 40 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)