* [PATCH 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM @ 2015-05-12 16:07 Roger Quadros 2015-05-12 16:07 ` [PATCH 1/3] phy: ti-pipe3: fix suspend Roger Quadros ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Roger Quadros @ 2015-05-12 16:07 UTC (permalink / raw) To: kishon, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap, Roger Quadros Hi, We fix 2 things here to ensure that PHY clocks are disabled during system suspend. 1) Stop relying on pm_runtime ops to disable our clocks as they don't fit in well with the PHY framework usage scenario. This ensures that all clocks are disabled when phy_exit() is called. 2) Implement workaround for SATA errata i783 (SATA Lockup After SATA DPLL Unlock/Relock) so that we can now turn off sata_refclk to support suspend-to-ram. cheers, -roger Roger Quadros (3): phy: ti-pipe3: fix suspend phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Documentation/devicetree/bindings/phy/ti-phy.txt | 16 +++ arch/arm/boot/dts/dra7.dtsi | 1 + drivers/phy/phy-ti-pipe3.c | 173 ++++++++++------------- 3 files changed, 90 insertions(+), 100 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] phy: ti-pipe3: fix suspend 2015-05-12 16:07 [PATCH 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM Roger Quadros @ 2015-05-12 16:07 ` Roger Quadros 2015-05-20 13:04 ` Kishon Vijay Abraham I 2015-05-12 16:07 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Roger Quadros 2015-05-12 16:07 ` [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Roger Quadros 2 siblings, 1 reply; 16+ messages in thread From: Roger Quadros @ 2015-05-12 16:07 UTC (permalink / raw) To: kishon, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap, Roger Quadros Relying on PM-ops for shutting down PHY clocks was a bad idea since the users (e.g. PCIe/SATA) might not have been suspended by then. The main culprit for not shutting down the clocks was the stray pm_runtime_get() call in probe. Fix the whole thing in the right way by getting rid of that pm_runtime_get() call from probe and removing all PM-ops. It is the sole responsibility of the PHY user to properly turn OFF and de-initialize the PHY as part of its suspend routine. As PHY core serializes init/exit we don't need to use a spinlock in this driver. So get rid of it. Signed-off-by: Roger Quadros <rogerq@ti.com> Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- drivers/phy/phy-ti-pipe3.c | 112 ++++++++------------------------------------- 1 file changed, 18 insertions(+), 94 deletions(-) diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c index 53f295c..e13a306 100644 --- a/drivers/phy/phy-ti-pipe3.c +++ b/drivers/phy/phy-ti-pipe3.c @@ -28,7 +28,6 @@ #include <linux/delay.h> #include <linux/phy/omap_control_phy.h> #include <linux/of_platform.h> -#include <linux/spinlock.h> #define PLL_STATUS 0x00000004 #define PLL_GO 0x00000008 @@ -83,10 +82,6 @@ struct ti_pipe3 { struct clk *refclk; struct clk *div_clk; struct pipe3_dpll_map *dpll_map; - bool enabled; - spinlock_t lock; /* serialize clock enable/disable */ - /* the below flag is needed specifically for SATA */ - bool refclk_enabled; }; static struct pipe3_dpll_map dpll_map_usb[] = { @@ -137,6 +132,9 @@ static struct pipe3_dpll_params *ti_pipe3_get_dpll_params(struct ti_pipe3 *phy) return NULL; } +static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy); +static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy); + static int ti_pipe3_power_off(struct phy *x) { struct ti_pipe3 *phy = phy_get_drvdata(x); @@ -217,6 +215,7 @@ static int ti_pipe3_init(struct phy *x) u32 val; int ret = 0; + ti_pipe3_enable_clocks(phy); /* * Set pcie_pcs register to 0x96 for proper functioning of phy * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table @@ -277,6 +276,8 @@ static int ti_pipe3_exit(struct phy *x) return -EBUSY; } + ti_pipe3_disable_clocks(phy); + return 0; } static struct phy_ops ops = { @@ -305,8 +306,7 @@ static int ti_pipe3_probe(struct platform_device *pdev) if (!phy) return -ENOMEM; - phy->dev = &pdev->dev; - spin_lock_init(&phy->lock); + phy->dev = &pdev->dev; if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { match = of_match_device(ti_pipe3_id_table, &pdev->dev); @@ -402,6 +402,9 @@ static int ti_pipe3_probe(struct platform_device *pdev) platform_set_drvdata(pdev, phy); pm_runtime_enable(phy->dev); + /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) + clk_prepare_enable(phy->refclk); generic_phy = devm_phy_create(phy->dev, NULL, &ops); if (IS_ERR(generic_phy)) @@ -413,24 +416,19 @@ static int ti_pipe3_probe(struct platform_device *pdev) if (IS_ERR(phy_provider)) return PTR_ERR(phy_provider); - pm_runtime_get(&pdev->dev); - return 0; } static int ti_pipe3_remove(struct platform_device *pdev) { - if (!pm_runtime_suspended(&pdev->dev)) - pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); return 0; } -#ifdef CONFIG_PM static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) { - if (!IS_ERR(phy->refclk) && !phy->refclk_enabled) { + if (!IS_ERR(phy->refclk)) { int ret; ret = clk_prepare_enable(phy->refclk); @@ -438,7 +436,6 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) dev_err(phy->dev, "Failed to enable refclk %d\n", ret); return ret; } - phy->refclk_enabled = true; } return 0; @@ -448,28 +445,21 @@ static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) { if (!IS_ERR(phy->refclk)) clk_disable_unprepare(phy->refclk); - - phy->refclk_enabled = false; } static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) { int ret = 0; - unsigned long flags; - - spin_lock_irqsave(&phy->lock, flags); - if (phy->enabled) - goto err1; ret = ti_pipe3_enable_refclk(phy); if (ret) - goto err1; + return ret; if (!IS_ERR(phy->wkupclk)) { ret = clk_prepare_enable(phy->wkupclk); if (ret) { dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret); - goto err2; + goto disable_refclk; } } @@ -477,96 +467,31 @@ static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) ret = clk_prepare_enable(phy->div_clk); if (ret) { dev_err(phy->dev, "Failed to enable div_clk %d\n", ret); - goto err3; + goto disable_wkupclk; } } - phy->enabled = true; - spin_unlock_irqrestore(&phy->lock, flags); return 0; -err3: +disable_wkupclk: if (!IS_ERR(phy->wkupclk)) clk_disable_unprepare(phy->wkupclk); -err2: - if (!IS_ERR(phy->refclk)) - clk_disable_unprepare(phy->refclk); - +disable_refclk: ti_pipe3_disable_refclk(phy); -err1: - spin_unlock_irqrestore(&phy->lock, flags); + return ret; } static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy) { - unsigned long flags; - - spin_lock_irqsave(&phy->lock, flags); - if (!phy->enabled) { - spin_unlock_irqrestore(&phy->lock, flags); - return; - } - if (!IS_ERR(phy->wkupclk)) clk_disable_unprepare(phy->wkupclk); - /* Don't disable refclk for SATA PHY due to Errata i783 */ - if (!of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) - ti_pipe3_disable_refclk(phy); + ti_pipe3_disable_refclk(phy); if (!IS_ERR(phy->div_clk)) clk_disable_unprepare(phy->div_clk); - phy->enabled = false; - spin_unlock_irqrestore(&phy->lock, flags); } -static int ti_pipe3_runtime_suspend(struct device *dev) -{ - struct ti_pipe3 *phy = dev_get_drvdata(dev); - - ti_pipe3_disable_clocks(phy); - return 0; -} - -static int ti_pipe3_runtime_resume(struct device *dev) -{ - struct ti_pipe3 *phy = dev_get_drvdata(dev); - int ret = 0; - - ret = ti_pipe3_enable_clocks(phy); - return ret; -} - -static int ti_pipe3_suspend(struct device *dev) -{ - struct ti_pipe3 *phy = dev_get_drvdata(dev); - - ti_pipe3_disable_clocks(phy); - return 0; -} - -static int ti_pipe3_resume(struct device *dev) -{ - struct ti_pipe3 *phy = dev_get_drvdata(dev); - int ret; - - ret = ti_pipe3_enable_clocks(phy); - if (ret) - return ret; - - pm_runtime_disable(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - return 0; -} -#endif - -static const struct dev_pm_ops ti_pipe3_pm_ops = { - SET_RUNTIME_PM_OPS(ti_pipe3_runtime_suspend, - ti_pipe3_runtime_resume, NULL) - SET_SYSTEM_SLEEP_PM_OPS(ti_pipe3_suspend, ti_pipe3_resume) -}; - static const struct of_device_id ti_pipe3_id_table[] = { { .compatible = "ti,phy-usb3", @@ -592,7 +517,6 @@ static struct platform_driver ti_pipe3_driver = { .remove = ti_pipe3_remove, .driver = { .name = "ti-pipe3", - .pm = &ti_pipe3_pm_ops, .of_match_table = ti_pipe3_id_table, }, }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] phy: ti-pipe3: fix suspend 2015-05-12 16:07 ` [PATCH 1/3] phy: ti-pipe3: fix suspend Roger Quadros @ 2015-05-20 13:04 ` Kishon Vijay Abraham I 2015-05-20 13:48 ` Roger Quadros 0 siblings, 1 reply; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-05-20 13:04 UTC (permalink / raw) To: Roger Quadros, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Hi Roger, On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: > Relying on PM-ops for shutting down PHY clocks was a > bad idea since the users (e.g. PCIe/SATA) might not > have been suspended by then. > > The main culprit for not shutting down the clocks was > the stray pm_runtime_get() call in probe. > > Fix the whole thing in the right way by getting rid > of that pm_runtime_get() call from probe and > removing all PM-ops. It is the sole responsibility > of the PHY user to properly turn OFF and de-initialize > the PHY as part of its suspend routine. > > As PHY core serializes init/exit we don't need > to use a spinlock in this driver. So get rid of it. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/phy/phy-ti-pipe3.c | 112 ++++++++------------------------------------- > 1 file changed, 18 insertions(+), 94 deletions(-) > > diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c > index 53f295c..e13a306 100644 > --- a/drivers/phy/phy-ti-pipe3.c > +++ b/drivers/phy/phy-ti-pipe3.c > @@ -28,7 +28,6 @@ > #include <linux/delay.h> > #include <linux/phy/omap_control_phy.h> > #include <linux/of_platform.h> > -#include <linux/spinlock.h> > > #define PLL_STATUS 0x00000004 > #define PLL_GO 0x00000008 > @@ -83,10 +82,6 @@ struct ti_pipe3 { > struct clk *refclk; > struct clk *div_clk; > struct pipe3_dpll_map *dpll_map; > - bool enabled; > - spinlock_t lock; /* serialize clock enable/disable */ > - /* the below flag is needed specifically for SATA */ > - bool refclk_enabled; > }; > > static struct pipe3_dpll_map dpll_map_usb[] = { > @@ -137,6 +132,9 @@ static struct pipe3_dpll_params *ti_pipe3_get_dpll_params(struct ti_pipe3 *phy) > return NULL; > } > > +static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy); > +static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy); > + > static int ti_pipe3_power_off(struct phy *x) > { > struct ti_pipe3 *phy = phy_get_drvdata(x); > @@ -217,6 +215,7 @@ static int ti_pipe3_init(struct phy *x) > u32 val; > int ret = 0; > > + ti_pipe3_enable_clocks(phy); > /* > * Set pcie_pcs register to 0x96 for proper functioning of phy > * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table > @@ -277,6 +276,8 @@ static int ti_pipe3_exit(struct phy *x) > return -EBUSY; > } > > + ti_pipe3_disable_clocks(phy); > + > return 0; > } > static struct phy_ops ops = { > @@ -305,8 +306,7 @@ static int ti_pipe3_probe(struct platform_device *pdev) > if (!phy) > return -ENOMEM; > > - phy->dev = &pdev->dev; > - spin_lock_init(&phy->lock); > + phy->dev = &pdev->dev; this looks more of a cleanup. > > if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { > match = of_match_device(ti_pipe3_id_table, &pdev->dev); > @@ -402,6 +402,9 @@ static int ti_pipe3_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, phy); > pm_runtime_enable(phy->dev); > + /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ > + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) > + clk_prepare_enable(phy->refclk); > > generic_phy = devm_phy_create(phy->dev, NULL, &ops); > if (IS_ERR(generic_phy)) > @@ -413,24 +416,19 @@ static int ti_pipe3_probe(struct platform_device *pdev) > if (IS_ERR(phy_provider)) > return PTR_ERR(phy_provider); > > - pm_runtime_get(&pdev->dev); > - > return 0; > } > > static int ti_pipe3_remove(struct platform_device *pdev) > { > - if (!pm_runtime_suspended(&pdev->dev)) > - pm_runtime_put(&pdev->dev); > pm_runtime_disable(&pdev->dev); > > return 0; > } > > -#ifdef CONFIG_PM > static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) > { > - if (!IS_ERR(phy->refclk) && !phy->refclk_enabled) { > + if (!IS_ERR(phy->refclk)) { > int ret; > > ret = clk_prepare_enable(phy->refclk); > @@ -438,7 +436,6 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) > dev_err(phy->dev, "Failed to enable refclk %d\n", ret); > return ret; > } > - phy->refclk_enabled = true; > } > > return 0; > @@ -448,28 +445,21 @@ static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) > { > if (!IS_ERR(phy->refclk)) > clk_disable_unprepare(phy->refclk); > - > - phy->refclk_enabled = false; > } > > static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) > { > int ret = 0; > - unsigned long flags; > - > - spin_lock_irqsave(&phy->lock, flags); > - if (phy->enabled) > - goto err1; > > ret = ti_pipe3_enable_refclk(phy); we can enable refclk here itself instead of having a separate function it? > if (ret) > - goto err1; > + return ret; > > if (!IS_ERR(phy->wkupclk)) { > ret = clk_prepare_enable(phy->wkupclk); > if (ret) { > dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret); > - goto err2; > + goto disable_refclk; > } > } > > @@ -477,96 +467,31 @@ static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) > ret = clk_prepare_enable(phy->div_clk); > if (ret) { > dev_err(phy->dev, "Failed to enable div_clk %d\n", ret); > - goto err3; > + goto disable_wkupclk; > } > } > > - phy->enabled = true; > - spin_unlock_irqrestore(&phy->lock, flags); > return 0; > > -err3: > +disable_wkupclk: > if (!IS_ERR(phy->wkupclk)) > clk_disable_unprepare(phy->wkupclk); > > -err2: > - if (!IS_ERR(phy->refclk)) > - clk_disable_unprepare(phy->refclk); > - > +disable_refclk: > ti_pipe3_disable_refclk(phy); > -err1: > - spin_unlock_irqrestore(&phy->lock, flags); > + > return ret; > } > > static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy) > { > - unsigned long flags; > - > - spin_lock_irqsave(&phy->lock, flags); > - if (!phy->enabled) { > - spin_unlock_irqrestore(&phy->lock, flags); > - return; > - } > - > if (!IS_ERR(phy->wkupclk)) > clk_disable_unprepare(phy->wkupclk); > - /* Don't disable refclk for SATA PHY due to Errata i783 */ > - if (!of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) > - ti_pipe3_disable_refclk(phy); > + ti_pipe3_disable_refclk(phy); here too. we can get rid of ti_pipe3_disable_refclk and ti_pipe3_enable_refclk. Thanks Kishon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] phy: ti-pipe3: fix suspend 2015-05-20 13:04 ` Kishon Vijay Abraham I @ 2015-05-20 13:48 ` Roger Quadros 0 siblings, 0 replies; 16+ messages in thread From: Roger Quadros @ 2015-05-20 13:48 UTC (permalink / raw) To: Kishon Vijay Abraham I, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Kishon, On 20/05/15 16:04, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >> Relying on PM-ops for shutting down PHY clocks was a >> bad idea since the users (e.g. PCIe/SATA) might not >> have been suspended by then. >> >> The main culprit for not shutting down the clocks was >> the stray pm_runtime_get() call in probe. >> >> Fix the whole thing in the right way by getting rid >> of that pm_runtime_get() call from probe and >> removing all PM-ops. It is the sole responsibility >> of the PHY user to properly turn OFF and de-initialize >> the PHY as part of its suspend routine. >> >> As PHY core serializes init/exit we don't need >> to use a spinlock in this driver. So get rid of it. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> drivers/phy/phy-ti-pipe3.c | 112 >> ++++++++------------------------------------- >> 1 file changed, 18 insertions(+), 94 deletions(-) >> >> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >> index 53f295c..e13a306 100644 >> --- a/drivers/phy/phy-ti-pipe3.c >> +++ b/drivers/phy/phy-ti-pipe3.c >> @@ -28,7 +28,6 @@ >> #include <linux/delay.h> >> #include <linux/phy/omap_control_phy.h> >> #include <linux/of_platform.h> >> -#include <linux/spinlock.h> >> >> #define PLL_STATUS 0x00000004 >> #define PLL_GO 0x00000008 >> @@ -83,10 +82,6 @@ struct ti_pipe3 { >> struct clk *refclk; >> struct clk *div_clk; >> struct pipe3_dpll_map *dpll_map; >> - bool enabled; >> - spinlock_t lock; /* serialize clock enable/disable */ >> - /* the below flag is needed specifically for SATA */ >> - bool refclk_enabled; >> }; >> >> static struct pipe3_dpll_map dpll_map_usb[] = { >> @@ -137,6 +132,9 @@ static struct pipe3_dpll_params >> *ti_pipe3_get_dpll_params(struct ti_pipe3 *phy) >> return NULL; >> } >> >> +static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy); >> +static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy); >> + >> static int ti_pipe3_power_off(struct phy *x) >> { >> struct ti_pipe3 *phy = phy_get_drvdata(x); >> @@ -217,6 +215,7 @@ static int ti_pipe3_init(struct phy *x) >> u32 val; >> int ret = 0; >> >> + ti_pipe3_enable_clocks(phy); >> /* >> * Set pcie_pcs register to 0x96 for proper functioning of phy >> * as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table >> @@ -277,6 +276,8 @@ static int ti_pipe3_exit(struct phy *x) >> return -EBUSY; >> } >> >> + ti_pipe3_disable_clocks(phy); >> + >> return 0; >> } >> static struct phy_ops ops = { >> @@ -305,8 +306,7 @@ static int ti_pipe3_probe(struct platform_device >> *pdev) >> if (!phy) >> return -ENOMEM; >> >> - phy->dev = &pdev->dev; >> - spin_lock_init(&phy->lock); >> + phy->dev = &pdev->dev; > > this looks more of a cleanup. ok will get rid of it. >> >> if (!of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >> match = of_match_device(ti_pipe3_id_table, &pdev->dev); >> @@ -402,6 +402,9 @@ static int ti_pipe3_probe(struct platform_device >> *pdev) >> >> platform_set_drvdata(pdev, phy); >> pm_runtime_enable(phy->dev); >> + /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >> + clk_prepare_enable(phy->refclk); >> >> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >> if (IS_ERR(generic_phy)) >> @@ -413,24 +416,19 @@ static int ti_pipe3_probe(struct platform_device >> *pdev) >> if (IS_ERR(phy_provider)) >> return PTR_ERR(phy_provider); >> >> - pm_runtime_get(&pdev->dev); >> - >> return 0; >> } >> >> static int ti_pipe3_remove(struct platform_device *pdev) >> { >> - if (!pm_runtime_suspended(&pdev->dev)) >> - pm_runtime_put(&pdev->dev); >> pm_runtime_disable(&pdev->dev); >> >> return 0; >> } >> >> -#ifdef CONFIG_PM >> static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) >> { >> - if (!IS_ERR(phy->refclk) && !phy->refclk_enabled) { >> + if (!IS_ERR(phy->refclk)) { >> int ret; >> >> ret = clk_prepare_enable(phy->refclk); >> @@ -438,7 +436,6 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >> *phy) >> dev_err(phy->dev, "Failed to enable refclk %d\n", ret); >> return ret; >> } >> - phy->refclk_enabled = true; >> } >> >> return 0; >> @@ -448,28 +445,21 @@ static void ti_pipe3_disable_refclk(struct >> ti_pipe3 *phy) >> { >> if (!IS_ERR(phy->refclk)) >> clk_disable_unprepare(phy->refclk); >> - >> - phy->refclk_enabled = false; >> } >> >> static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) >> { >> int ret = 0; >> - unsigned long flags; >> - >> - spin_lock_irqsave(&phy->lock, flags); >> - if (phy->enabled) >> - goto err1; >> >> ret = ti_pipe3_enable_refclk(phy); > > we can enable refclk here itself instead of having a separate function it? >> if (ret) >> - goto err1; >> + return ret; >> >> if (!IS_ERR(phy->wkupclk)) { >> ret = clk_prepare_enable(phy->wkupclk); >> if (ret) { >> dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret); >> - goto err2; >> + goto disable_refclk; >> } >> } >> >> @@ -477,96 +467,31 @@ static int ti_pipe3_enable_clocks(struct >> ti_pipe3 *phy) >> ret = clk_prepare_enable(phy->div_clk); >> if (ret) { >> dev_err(phy->dev, "Failed to enable div_clk %d\n", ret); >> - goto err3; >> + goto disable_wkupclk; >> } >> } >> >> - phy->enabled = true; >> - spin_unlock_irqrestore(&phy->lock, flags); >> return 0; >> >> -err3: >> +disable_wkupclk: >> if (!IS_ERR(phy->wkupclk)) >> clk_disable_unprepare(phy->wkupclk); >> >> -err2: >> - if (!IS_ERR(phy->refclk)) >> - clk_disable_unprepare(phy->refclk); >> - >> +disable_refclk: >> ti_pipe3_disable_refclk(phy); >> -err1: >> - spin_unlock_irqrestore(&phy->lock, flags); >> + >> return ret; >> } >> >> static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy) >> { >> - unsigned long flags; >> - >> - spin_lock_irqsave(&phy->lock, flags); >> - if (!phy->enabled) { >> - spin_unlock_irqrestore(&phy->lock, flags); >> - return; >> - } >> - >> if (!IS_ERR(phy->wkupclk)) >> clk_disable_unprepare(phy->wkupclk); >> - /* Don't disable refclk for SATA PHY due to Errata i783 */ >> - if (!of_device_is_compatible(phy->dev->of_node, >> "ti,phy-pipe3-sata")) >> - ti_pipe3_disable_refclk(phy); >> + ti_pipe3_disable_refclk(phy); > > here too. we can get rid of ti_pipe3_disable_refclk and > ti_pipe3_enable_refclk. > fine. cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-12 16:07 [PATCH 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM Roger Quadros 2015-05-12 16:07 ` [PATCH 1/3] phy: ti-pipe3: fix suspend Roger Quadros @ 2015-05-12 16:07 ` Roger Quadros 2015-05-20 13:19 ` Kishon Vijay Abraham I 2015-05-12 16:07 ` [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Roger Quadros 2 siblings, 1 reply; 16+ messages in thread From: Roger Quadros @ 2015-05-12 16:07 UTC (permalink / raw) To: kishon, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap, Roger Quadros SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled between a SATA DPLL unlock and re-lock to prevent SATA lockup. Introduce a new DT parameter 'syscon-pllreset' to provide the syscon regmap access to this register which sits in the control module. If the register is not provided we fallback to the old behaviour i.e. SATA DPLL refclk will not be disabled and we prevent SoC low power states. Signed-off-by: Roger Quadros <rogerq@ti.com> --- Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ drivers/phy/phy-ti-pipe3.c | 67 ++++++++++++++++++++---- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt index 305e3df..f0f5537 100644 --- a/Documentation/devicetree/bindings/phy/ti-phy.txt +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt @@ -82,6 +82,9 @@ Optional properties: - id: If there are multiple instance of the same type, in order to differentiate between each instance "id" can be used (e.g., multi-lane PCIe PHY). If "id" is not provided, it is set to default value of '1'. + - syscon-pllreset: Handle to system control region that contains the + CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0 + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy. This is usually a subnode of ocp2scp to which it is connected. @@ -100,3 +103,16 @@ usb3phy@4a084400 { "sysclk", "refclk"; }; + +sata_phy: phy@4A096000 { + compatible = "ti,phy-pipe3-sata"; + reg = <0x4A096000 0x80>, /* phy_rx */ + <0x4A096400 0x64>, /* phy_tx */ + <0x4A096800 0x40>; /* pll_ctrl */ + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; + ctrl-module = <&omap_control_sata>; + clocks = <&sys_clkin1>, <&sata_ref_clk>; + clock-names = "sysclk", "refclk"; + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; + #phy-cells = <0>; +}; diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c index e13a306..d730142 100644 --- a/drivers/phy/phy-ti-pipe3.c +++ b/drivers/phy/phy-ti-pipe3.c @@ -28,6 +28,8 @@ #include <linux/delay.h> #include <linux/phy/omap_control_phy.h> #include <linux/of_platform.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #define PLL_STATUS 0x00000004 #define PLL_GO 0x00000008 @@ -52,6 +54,8 @@ #define PLL_LOCK 0x2 #define PLL_IDLE 0x1 +#define SATA_PLL_SOFT_RESET BIT(18) + /* * This is an Empirical value that works, need to confirm the actual * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status @@ -82,6 +86,9 @@ struct ti_pipe3 { struct clk *refclk; struct clk *div_clk; struct pipe3_dpll_map *dpll_map; + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ + unsigned int dpll_reset_reg; /* reg. index within syscon */ + bool sata_refclk_enabled; }; static struct pipe3_dpll_map dpll_map_usb[] = { @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) u32 val; unsigned long timeout; - /* SATA DPLL can't be powered down due to Errata i783 and PCIe - * does not have internal DPLL + /* If dpll_reset_syscon is not present we wont power down SATA DPLL + * due to Errata i783 */ - if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") || - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") && + !phy->dpll_reset_syscon) + return 0; + + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) return 0; /* Put DPLL in IDLE mode */ @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) return -EBUSY; } + /* i783: SATA needs control bit toggle after PLL unlock */ + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) { + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, + SATA_PLL_SOFT_RESET, 0); + } + ti_pipe3_disable_clocks(phy); return 0; @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device *pdev) } } else { phy->wkupclk = ERR_PTR(-ENODEV); + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, + "syscon-pllreset"); + if (IS_ERR(phy->dpll_reset_syscon)) { + dev_info(&pdev->dev, + "can't get syscon-pllreset, sata dpll won't idle\n"); + phy->dpll_reset_syscon = NULL; + } else { + if (of_property_read_u32_index(node, + "syscon-pllreset", 1, + &phy->dpll_reset_reg)) { + dev_err(&pdev->dev, + "couldn't get pllreset reg. offset\n"); + return -EINVAL; + } + } } if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device *pdev) platform_set_drvdata(pdev, phy); pm_runtime_enable(phy->dev); - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) - clk_prepare_enable(phy->refclk); + + /* + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 + */ + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { + if (!IS_ERR(phy->refclk)) { + clk_prepare_enable(phy->refclk); + phy->sata_refclk_enabled = true; + } + } generic_phy = devm_phy_create(phy->dev, NULL, &ops); if (IS_ERR(generic_phy)) @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) { - if (!IS_ERR(phy->refclk)) + if (!IS_ERR(phy->refclk)) { clk_disable_unprepare(phy->refclk); + /* + * SATA refclk needs an additional disable as we left it + * on in probe to avoid Errata i783 + */ + if (phy->sata_refclk_enabled) { + clk_disable_unprepare(phy->refclk); + phy->sata_refclk_enabled = false; + } + } } static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) @@ -476,7 +526,6 @@ static int ti_pipe3_enable_clocks(struct ti_pipe3 *phy) disable_wkupclk: if (!IS_ERR(phy->wkupclk)) clk_disable_unprepare(phy->wkupclk); - disable_refclk: ti_pipe3_disable_refclk(phy); -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-12 16:07 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Roger Quadros @ 2015-05-20 13:19 ` Kishon Vijay Abraham I 2015-05-20 13:47 ` Roger Quadros 2015-06-02 8:17 ` Roger Quadros 0 siblings, 2 replies; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-05-20 13:19 UTC (permalink / raw) To: Roger Quadros, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Hi Roger, On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: > SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled > between a SATA DPLL unlock and re-lock to prevent SATA lockup. > > Introduce a new DT parameter 'syscon-pllreset' to provide the syscon > regmap access to this register which sits in the control module. > > If the register is not provided we fallback to the old behaviour > i.e. SATA DPLL refclk will not be disabled and we prevent SoC low > power states. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ > drivers/phy/phy-ti-pipe3.c | 67 ++++++++++++++++++++---- > 2 files changed, 74 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt > index 305e3df..f0f5537 100644 > --- a/Documentation/devicetree/bindings/phy/ti-phy.txt > +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt > @@ -82,6 +82,9 @@ Optional properties: > - id: If there are multiple instance of the same type, in order to > differentiate between each instance "id" can be used (e.g., multi-lane PCIe > PHY). If "id" is not provided, it is set to default value of '1'. > + - syscon-pllreset: Handle to system control region that contains the > + CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0 > + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy. > > This is usually a subnode of ocp2scp to which it is connected. > > @@ -100,3 +103,16 @@ usb3phy@4a084400 { > "sysclk", > "refclk"; > }; > + > +sata_phy: phy@4A096000 { > + compatible = "ti,phy-pipe3-sata"; > + reg = <0x4A096000 0x80>, /* phy_rx */ > + <0x4A096400 0x64>, /* phy_tx */ > + <0x4A096800 0x40>; /* pll_ctrl */ > + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; > + ctrl-module = <&omap_control_sata>; > + clocks = <&sys_clkin1>, <&sata_ref_clk>; > + clock-names = "sysclk", "refclk"; > + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; > + #phy-cells = <0>; > +}; > diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c > index e13a306..d730142 100644 > --- a/drivers/phy/phy-ti-pipe3.c > +++ b/drivers/phy/phy-ti-pipe3.c > @@ -28,6 +28,8 @@ > #include <linux/delay.h> > #include <linux/phy/omap_control_phy.h> > #include <linux/of_platform.h> > +#include <linux/mfd/syscon.h> > +#include <linux/regmap.h> > > #define PLL_STATUS 0x00000004 > #define PLL_GO 0x00000008 > @@ -52,6 +54,8 @@ > #define PLL_LOCK 0x2 > #define PLL_IDLE 0x1 > > +#define SATA_PLL_SOFT_RESET BIT(18) > + > /* > * This is an Empirical value that works, need to confirm the actual > * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status > @@ -82,6 +86,9 @@ struct ti_pipe3 { > struct clk *refclk; > struct clk *div_clk; > struct pipe3_dpll_map *dpll_map; > + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ > + unsigned int dpll_reset_reg; /* reg. index within syscon */ > + bool sata_refclk_enabled; > }; > > static struct pipe3_dpll_map dpll_map_usb[] = { > @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) > u32 val; > unsigned long timeout; > > - /* SATA DPLL can't be powered down due to Errata i783 and PCIe > - * does not have internal DPLL > + /* If dpll_reset_syscon is not present we wont power down SATA DPLL > + * due to Errata i783 > */ > - if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") || > - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) > + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") && > + !phy->dpll_reset_syscon) > + return 0; > + > + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ I think it's better to fix it in this patch itself.. to disable clocks for PCIe. > + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) > return 0; > > /* Put DPLL in IDLE mode */ > @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) > return -EBUSY; > } > > + /* i783: SATA needs control bit toggle after PLL unlock */ > + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) { > + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, > + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); > + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, > + SATA_PLL_SOFT_RESET, 0); > + } > + > ti_pipe3_disable_clocks(phy); > > return 0; > @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device *pdev) > } > } else { > phy->wkupclk = ERR_PTR(-ENODEV); > + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, > + "syscon-pllreset"); > + if (IS_ERR(phy->dpll_reset_syscon)) { > + dev_info(&pdev->dev, > + "can't get syscon-pllreset, sata dpll won't idle\n"); > + phy->dpll_reset_syscon = NULL; > + } else { > + if (of_property_read_u32_index(node, > + "syscon-pllreset", 1, > + &phy->dpll_reset_reg)) { > + dev_err(&pdev->dev, > + "couldn't get pllreset reg. offset\n"); > + return -EINVAL; > + } > + } > } > > if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { > @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, phy); > pm_runtime_enable(phy->dev); > - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ > - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) > - clk_prepare_enable(phy->refclk); > + > + /* > + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 > + */ > + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { > + if (!IS_ERR(phy->refclk)) { > + clk_prepare_enable(phy->refclk); > + phy->sata_refclk_enabled = true; > + } > + } > > generic_phy = devm_phy_create(phy->dev, NULL, &ops); > if (IS_ERR(generic_phy)) > @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 *phy) > > static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) > { > - if (!IS_ERR(phy->refclk)) > + if (!IS_ERR(phy->refclk)) { > clk_disable_unprepare(phy->refclk); > + /* > + * SATA refclk needs an additional disable as we left it > + * on in probe to avoid Errata i783 not sure I get this. Why don't we remove it in probe? Thanks Kishon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-20 13:19 ` Kishon Vijay Abraham I @ 2015-05-20 13:47 ` Roger Quadros 2015-05-22 11:34 ` Kishon Vijay Abraham I 2015-06-02 8:17 ` Roger Quadros 1 sibling, 1 reply; 16+ messages in thread From: Roger Quadros @ 2015-05-20 13:47 UTC (permalink / raw) To: Kishon Vijay Abraham I, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Kishon, On 20/05/15 16:19, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >> >> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >> regmap access to this register which sits in the control module. >> >> If the register is not provided we fallback to the old behaviour >> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >> power states. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >> drivers/phy/phy-ti-pipe3.c | 67 >> ++++++++++++++++++++---- >> 2 files changed, 74 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >> b/Documentation/devicetree/bindings/phy/ti-phy.txt >> index 305e3df..f0f5537 100644 >> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >> @@ -82,6 +82,9 @@ Optional properties: >> - id: If there are multiple instance of the same type, in order to >> differentiate between each instance "id" can be used (e.g., >> multi-lane PCIe >> PHY). If "id" is not provided, it is set to default value of '1'. >> + - syscon-pllreset: Handle to system control region that contains the >> + CTRL_CORE_SMA_SW_0 register and register offset to the >> CTRL_CORE_SMA_SW_0 >> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for >> sata_phy. >> >> This is usually a subnode of ocp2scp to which it is connected. >> >> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >> "sysclk", >> "refclk"; >> }; >> + >> +sata_phy: phy@4A096000 { >> + compatible = "ti,phy-pipe3-sata"; >> + reg = <0x4A096000 0x80>, /* phy_rx */ >> + <0x4A096400 0x64>, /* phy_tx */ >> + <0x4A096800 0x40>; /* pll_ctrl */ >> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >> + ctrl-module = <&omap_control_sata>; >> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >> + clock-names = "sysclk", "refclk"; >> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >> + #phy-cells = <0>; >> +}; >> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >> index e13a306..d730142 100644 >> --- a/drivers/phy/phy-ti-pipe3.c >> +++ b/drivers/phy/phy-ti-pipe3.c >> @@ -28,6 +28,8 @@ >> #include <linux/delay.h> >> #include <linux/phy/omap_control_phy.h> >> #include <linux/of_platform.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/regmap.h> >> >> #define PLL_STATUS 0x00000004 >> #define PLL_GO 0x00000008 >> @@ -52,6 +54,8 @@ >> #define PLL_LOCK 0x2 >> #define PLL_IDLE 0x1 >> >> +#define SATA_PLL_SOFT_RESET BIT(18) >> + >> /* >> * This is an Empirical value that works, need to confirm the actual >> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >> @@ -82,6 +86,9 @@ struct ti_pipe3 { >> struct clk *refclk; >> struct clk *div_clk; >> struct pipe3_dpll_map *dpll_map; >> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >> + bool sata_refclk_enabled; >> }; >> >> static struct pipe3_dpll_map dpll_map_usb[] = { >> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >> u32 val; >> unsigned long timeout; >> >> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >> - * does not have internal DPLL >> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >> + * due to Errata i783 >> */ >> - if (of_device_is_compatible(phy->dev->of_node, >> "ti,phy-pipe3-sata") || >> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >> + if (of_device_is_compatible(phy->dev->of_node, >> "ti,phy-pipe3-sata") && >> + !phy->dpll_reset_syscon) >> + return 0; >> + >> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ > > I think it's better to fix it in this patch itself.. to disable clocks > for PCIe. >> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >> return 0; >> >> /* Put DPLL in IDLE mode */ >> @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) >> return -EBUSY; >> } >> >> + /* i783: SATA needs control bit toggle after PLL unlock */ >> + if (of_device_is_compatible(phy->dev->of_node, >> "ti,phy-pipe3-sata")) { >> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >> + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); >> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >> + SATA_PLL_SOFT_RESET, 0); >> + } >> + >> ti_pipe3_disable_clocks(phy); >> >> return 0; >> @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device >> *pdev) >> } >> } else { >> phy->wkupclk = ERR_PTR(-ENODEV); >> + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, >> + "syscon-pllreset"); >> + if (IS_ERR(phy->dpll_reset_syscon)) { >> + dev_info(&pdev->dev, >> + "can't get syscon-pllreset, sata dpll won't idle\n"); >> + phy->dpll_reset_syscon = NULL; >> + } else { >> + if (of_property_read_u32_index(node, >> + "syscon-pllreset", 1, >> + &phy->dpll_reset_reg)) { >> + dev_err(&pdev->dev, >> + "couldn't get pllreset reg. offset\n"); >> + return -EINVAL; >> + } >> + } >> } >> >> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >> @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device >> *pdev) >> >> platform_set_drvdata(pdev, phy); >> pm_runtime_enable(phy->dev); >> - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >> - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >> - clk_prepare_enable(phy->refclk); >> + >> + /* >> + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 >> + */ >> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { >> + if (!IS_ERR(phy->refclk)) { >> + clk_prepare_enable(phy->refclk); >> + phy->sata_refclk_enabled = true; >> + } >> + } >> >> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >> if (IS_ERR(generic_phy)) >> @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >> *phy) >> >> static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) >> { >> - if (!IS_ERR(phy->refclk)) >> + if (!IS_ERR(phy->refclk)) { >> clk_disable_unprepare(phy->refclk); >> + /* >> + * SATA refclk needs an additional disable as we left it >> + * on in probe to avoid Errata i783 > > not sure I get this. Why don't we remove it in probe? Because if we remove it from probe and if AHCI_PLATFORM is built as a module then phy_init won't be called before kernel starts autodisabling unused clocks and so SATA refclk gets disabled causing i783 and breaking SATA. cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-20 13:47 ` Roger Quadros @ 2015-05-22 11:34 ` Kishon Vijay Abraham I 2015-05-22 13:58 ` Roger Quadros 0 siblings, 1 reply; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-05-22 11:34 UTC (permalink / raw) To: Roger Quadros, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Roger, On Wednesday 20 May 2015 07:17 PM, Roger Quadros wrote: > Kishon, > > On 20/05/15 16:19, Kishon Vijay Abraham I wrote: >> Hi Roger, >> >> On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >>> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >>> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >>> >>> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >>> regmap access to this register which sits in the control module. >>> >>> If the register is not provided we fallback to the old behaviour >>> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >>> power states. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >>> drivers/phy/phy-ti-pipe3.c | 67 >>> ++++++++++++++++++++---- >>> 2 files changed, 74 insertions(+), 9 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >>> b/Documentation/devicetree/bindings/phy/ti-phy.txt >>> index 305e3df..f0f5537 100644 >>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >>> @@ -82,6 +82,9 @@ Optional properties: >>> - id: If there are multiple instance of the same type, in order to >>> differentiate between each instance "id" can be used (e.g., >>> multi-lane PCIe >>> PHY). If "id" is not provided, it is set to default value of '1'. >>> + - syscon-pllreset: Handle to system control region that contains the >>> + CTRL_CORE_SMA_SW_0 register and register offset to the >>> CTRL_CORE_SMA_SW_0 >>> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for >>> sata_phy. >>> >>> This is usually a subnode of ocp2scp to which it is connected. >>> >>> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >>> "sysclk", >>> "refclk"; >>> }; >>> + >>> +sata_phy: phy@4A096000 { >>> + compatible = "ti,phy-pipe3-sata"; >>> + reg = <0x4A096000 0x80>, /* phy_rx */ >>> + <0x4A096400 0x64>, /* phy_tx */ >>> + <0x4A096800 0x40>; /* pll_ctrl */ >>> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >>> + ctrl-module = <&omap_control_sata>; >>> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >>> + clock-names = "sysclk", "refclk"; >>> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >>> + #phy-cells = <0>; >>> +}; >>> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >>> index e13a306..d730142 100644 >>> --- a/drivers/phy/phy-ti-pipe3.c >>> +++ b/drivers/phy/phy-ti-pipe3.c >>> @@ -28,6 +28,8 @@ >>> #include <linux/delay.h> >>> #include <linux/phy/omap_control_phy.h> >>> #include <linux/of_platform.h> >>> +#include <linux/mfd/syscon.h> >>> +#include <linux/regmap.h> >>> >>> #define PLL_STATUS 0x00000004 >>> #define PLL_GO 0x00000008 >>> @@ -52,6 +54,8 @@ >>> #define PLL_LOCK 0x2 >>> #define PLL_IDLE 0x1 >>> >>> +#define SATA_PLL_SOFT_RESET BIT(18) >>> + >>> /* >>> * This is an Empirical value that works, need to confirm the actual >>> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >>> @@ -82,6 +86,9 @@ struct ti_pipe3 { >>> struct clk *refclk; >>> struct clk *div_clk; >>> struct pipe3_dpll_map *dpll_map; >>> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >>> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >>> + bool sata_refclk_enabled; >>> }; >>> >>> static struct pipe3_dpll_map dpll_map_usb[] = { >>> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >>> u32 val; >>> unsigned long timeout; >>> >>> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >>> - * does not have internal DPLL >>> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >>> + * due to Errata i783 >>> */ >>> - if (of_device_is_compatible(phy->dev->of_node, >>> "ti,phy-pipe3-sata") || >>> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>> + if (of_device_is_compatible(phy->dev->of_node, >>> "ti,phy-pipe3-sata") && >>> + !phy->dpll_reset_syscon) >>> + return 0; >>> + >>> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ >> >> I think it's better to fix it in this patch itself.. to disable clocks >> for PCIe. >>> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>> return 0; >>> >>> /* Put DPLL in IDLE mode */ >>> @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) >>> return -EBUSY; >>> } >>> >>> + /* i783: SATA needs control bit toggle after PLL unlock */ >>> + if (of_device_is_compatible(phy->dev->of_node, >>> "ti,phy-pipe3-sata")) { >>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>> + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); >>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>> + SATA_PLL_SOFT_RESET, 0); >>> + } >>> + >>> ti_pipe3_disable_clocks(phy); >>> >>> return 0; >>> @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device >>> *pdev) >>> } >>> } else { >>> phy->wkupclk = ERR_PTR(-ENODEV); >>> + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, >>> + "syscon-pllreset"); >>> + if (IS_ERR(phy->dpll_reset_syscon)) { >>> + dev_info(&pdev->dev, >>> + "can't get syscon-pllreset, sata dpll won't idle\n"); >>> + phy->dpll_reset_syscon = NULL; >>> + } else { >>> + if (of_property_read_u32_index(node, >>> + "syscon-pllreset", 1, >>> + &phy->dpll_reset_reg)) { >>> + dev_err(&pdev->dev, >>> + "couldn't get pllreset reg. offset\n"); >>> + return -EINVAL; >>> + } >>> + } >>> } >>> >>> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >>> @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device >>> *pdev) >>> >>> platform_set_drvdata(pdev, phy); >>> pm_runtime_enable(phy->dev); >>> - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >>> - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >>> - clk_prepare_enable(phy->refclk); >>> + >>> + /* >>> + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 >>> + */ >>> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { >>> + if (!IS_ERR(phy->refclk)) { >>> + clk_prepare_enable(phy->refclk); >>> + phy->sata_refclk_enabled = true; >>> + } >>> + } >>> >>> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >>> if (IS_ERR(generic_phy)) >>> @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >>> *phy) >>> >>> static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) >>> { >>> - if (!IS_ERR(phy->refclk)) >>> + if (!IS_ERR(phy->refclk)) { >>> clk_disable_unprepare(phy->refclk); >>> + /* >>> + * SATA refclk needs an additional disable as we left it >>> + * on in probe to avoid Errata i783 >> >> not sure I get this. Why don't we remove it in probe? > > Because if we remove it from probe and if AHCI_PLATFORM is built as a > module then phy_init won't be called before kernel starts autodisabling > unused clocks and so SATA refclk gets disabled causing i783 and > breaking SATA. Just to be clear, this entire thing won't be required if uboot does SATA_PLL_SOFT_RESET no? Thanks Kishon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-22 11:34 ` Kishon Vijay Abraham I @ 2015-05-22 13:58 ` Roger Quadros 2015-05-23 7:22 ` Kishon Vijay Abraham I 0 siblings, 1 reply; 16+ messages in thread From: Roger Quadros @ 2015-05-22 13:58 UTC (permalink / raw) To: Kishon Vijay Abraham I, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Kishon, On 22/05/15 14:34, Kishon Vijay Abraham I wrote: > Roger, > > On Wednesday 20 May 2015 07:17 PM, Roger Quadros wrote: >> Kishon, >> >> On 20/05/15 16:19, Kishon Vijay Abraham I wrote: >>> Hi Roger, >>> >>> On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >>>> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >>>> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >>>> >>>> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >>>> regmap access to this register which sits in the control module. >>>> >>>> If the register is not provided we fallback to the old behaviour >>>> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >>>> power states. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >>>> drivers/phy/phy-ti-pipe3.c | 67 >>>> ++++++++++++++++++++---- >>>> 2 files changed, 74 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>> b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>> index 305e3df..f0f5537 100644 >>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>> @@ -82,6 +82,9 @@ Optional properties: >>>> - id: If there are multiple instance of the same type, in order to >>>> differentiate between each instance "id" can be used (e.g., >>>> multi-lane PCIe >>>> PHY). If "id" is not provided, it is set to default value of '1'. >>>> + - syscon-pllreset: Handle to system control region that contains the >>>> + CTRL_CORE_SMA_SW_0 register and register offset to the >>>> CTRL_CORE_SMA_SW_0 >>>> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for >>>> sata_phy. >>>> >>>> This is usually a subnode of ocp2scp to which it is connected. >>>> >>>> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >>>> "sysclk", >>>> "refclk"; >>>> }; >>>> + >>>> +sata_phy: phy@4A096000 { >>>> + compatible = "ti,phy-pipe3-sata"; >>>> + reg = <0x4A096000 0x80>, /* phy_rx */ >>>> + <0x4A096400 0x64>, /* phy_tx */ >>>> + <0x4A096800 0x40>; /* pll_ctrl */ >>>> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >>>> + ctrl-module = <&omap_control_sata>; >>>> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >>>> + clock-names = "sysclk", "refclk"; >>>> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >>>> + #phy-cells = <0>; >>>> +}; >>>> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >>>> index e13a306..d730142 100644 >>>> --- a/drivers/phy/phy-ti-pipe3.c >>>> +++ b/drivers/phy/phy-ti-pipe3.c >>>> @@ -28,6 +28,8 @@ >>>> #include <linux/delay.h> >>>> #include <linux/phy/omap_control_phy.h> >>>> #include <linux/of_platform.h> >>>> +#include <linux/mfd/syscon.h> >>>> +#include <linux/regmap.h> >>>> >>>> #define PLL_STATUS 0x00000004 >>>> #define PLL_GO 0x00000008 >>>> @@ -52,6 +54,8 @@ >>>> #define PLL_LOCK 0x2 >>>> #define PLL_IDLE 0x1 >>>> >>>> +#define SATA_PLL_SOFT_RESET BIT(18) >>>> + >>>> /* >>>> * This is an Empirical value that works, need to confirm the actual >>>> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >>>> @@ -82,6 +86,9 @@ struct ti_pipe3 { >>>> struct clk *refclk; >>>> struct clk *div_clk; >>>> struct pipe3_dpll_map *dpll_map; >>>> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >>>> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >>>> + bool sata_refclk_enabled; >>>> }; >>>> >>>> static struct pipe3_dpll_map dpll_map_usb[] = { >>>> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >>>> u32 val; >>>> unsigned long timeout; >>>> >>>> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >>>> - * does not have internal DPLL >>>> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >>>> + * due to Errata i783 >>>> */ >>>> - if (of_device_is_compatible(phy->dev->of_node, >>>> "ti,phy-pipe3-sata") || >>>> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>> + if (of_device_is_compatible(phy->dev->of_node, >>>> "ti,phy-pipe3-sata") && >>>> + !phy->dpll_reset_syscon) >>>> + return 0; >>>> + >>>> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ >>> >>> I think it's better to fix it in this patch itself.. to disable clocks >>> for PCIe. >>>> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>> return 0; >>>> >>>> /* Put DPLL in IDLE mode */ >>>> @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) >>>> return -EBUSY; >>>> } >>>> >>>> + /* i783: SATA needs control bit toggle after PLL unlock */ >>>> + if (of_device_is_compatible(phy->dev->of_node, >>>> "ti,phy-pipe3-sata")) { >>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>> + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); >>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>> + SATA_PLL_SOFT_RESET, 0); >>>> + } >>>> + >>>> ti_pipe3_disable_clocks(phy); >>>> >>>> return 0; >>>> @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device >>>> *pdev) >>>> } >>>> } else { >>>> phy->wkupclk = ERR_PTR(-ENODEV); >>>> + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, >>>> + "syscon-pllreset"); >>>> + if (IS_ERR(phy->dpll_reset_syscon)) { >>>> + dev_info(&pdev->dev, >>>> + "can't get syscon-pllreset, sata dpll won't idle\n"); >>>> + phy->dpll_reset_syscon = NULL; >>>> + } else { >>>> + if (of_property_read_u32_index(node, >>>> + "syscon-pllreset", 1, >>>> + &phy->dpll_reset_reg)) { >>>> + dev_err(&pdev->dev, >>>> + "couldn't get pllreset reg. offset\n"); >>>> + return -EINVAL; >>>> + } >>>> + } >>>> } >>>> >>>> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >>>> @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device >>>> *pdev) >>>> >>>> platform_set_drvdata(pdev, phy); >>>> pm_runtime_enable(phy->dev); >>>> - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >>>> - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >>>> - clk_prepare_enable(phy->refclk); >>>> + >>>> + /* >>>> + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 >>>> + */ >>>> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { >>>> + if (!IS_ERR(phy->refclk)) { >>>> + clk_prepare_enable(phy->refclk); >>>> + phy->sata_refclk_enabled = true; >>>> + } >>>> + } >>>> >>>> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >>>> if (IS_ERR(generic_phy)) >>>> @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >>>> *phy) >>>> >>>> static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) >>>> { >>>> - if (!IS_ERR(phy->refclk)) >>>> + if (!IS_ERR(phy->refclk)) { >>>> clk_disable_unprepare(phy->refclk); >>>> + /* >>>> + * SATA refclk needs an additional disable as we left it >>>> + * on in probe to avoid Errata i783 >>> >>> not sure I get this. Why don't we remove it in probe? >> >> Because if we remove it from probe and if AHCI_PLATFORM is built as a >> module then phy_init won't be called before kernel starts autodisabling >> unused clocks and so SATA refclk gets disabled causing i783 and >> breaking SATA. > > Just to be clear, this entire thing won't be required if uboot does SATA_PLL_SOFT_RESET no? > Yes that is right. We don't yet have the u-boot fix and there is no guarantee all users move to the u-boot with the fix, so this is important. cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-22 13:58 ` Roger Quadros @ 2015-05-23 7:22 ` Kishon Vijay Abraham I 2015-05-26 14:20 ` Roger Quadros 0 siblings, 1 reply; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-05-23 7:22 UTC (permalink / raw) To: Roger Quadros, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Roger, On Friday 22 May 2015 07:28 PM, Roger Quadros wrote: > Kishon, > > On 22/05/15 14:34, Kishon Vijay Abraham I wrote: >> Roger, >> >> On Wednesday 20 May 2015 07:17 PM, Roger Quadros wrote: >>> Kishon, >>> >>> On 20/05/15 16:19, Kishon Vijay Abraham I wrote: >>>> Hi Roger, >>>> >>>> On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >>>>> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >>>>> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >>>>> >>>>> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >>>>> regmap access to this register which sits in the control module. >>>>> >>>>> If the register is not provided we fallback to the old behaviour >>>>> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >>>>> power states. >>>>> >>>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>>> --- >>>>> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >>>>> drivers/phy/phy-ti-pipe3.c | 67 >>>>> ++++++++++++++++++++---- >>>>> 2 files changed, 74 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>> b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>> index 305e3df..f0f5537 100644 >>>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>> @@ -82,6 +82,9 @@ Optional properties: >>>>> - id: If there are multiple instance of the same type, in order to >>>>> differentiate between each instance "id" can be used (e.g., >>>>> multi-lane PCIe >>>>> PHY). If "id" is not provided, it is set to default value of '1'. >>>>> + - syscon-pllreset: Handle to system control region that contains the >>>>> + CTRL_CORE_SMA_SW_0 register and register offset to the >>>>> CTRL_CORE_SMA_SW_0 >>>>> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for >>>>> sata_phy. >>>>> >>>>> This is usually a subnode of ocp2scp to which it is connected. >>>>> >>>>> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >>>>> "sysclk", >>>>> "refclk"; >>>>> }; >>>>> + >>>>> +sata_phy: phy@4A096000 { >>>>> + compatible = "ti,phy-pipe3-sata"; >>>>> + reg = <0x4A096000 0x80>, /* phy_rx */ >>>>> + <0x4A096400 0x64>, /* phy_tx */ >>>>> + <0x4A096800 0x40>; /* pll_ctrl */ >>>>> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >>>>> + ctrl-module = <&omap_control_sata>; >>>>> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >>>>> + clock-names = "sysclk", "refclk"; >>>>> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >>>>> + #phy-cells = <0>; >>>>> +}; >>>>> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >>>>> index e13a306..d730142 100644 >>>>> --- a/drivers/phy/phy-ti-pipe3.c >>>>> +++ b/drivers/phy/phy-ti-pipe3.c >>>>> @@ -28,6 +28,8 @@ >>>>> #include <linux/delay.h> >>>>> #include <linux/phy/omap_control_phy.h> >>>>> #include <linux/of_platform.h> >>>>> +#include <linux/mfd/syscon.h> >>>>> +#include <linux/regmap.h> >>>>> >>>>> #define PLL_STATUS 0x00000004 >>>>> #define PLL_GO 0x00000008 >>>>> @@ -52,6 +54,8 @@ >>>>> #define PLL_LOCK 0x2 >>>>> #define PLL_IDLE 0x1 >>>>> >>>>> +#define SATA_PLL_SOFT_RESET BIT(18) >>>>> + >>>>> /* >>>>> * This is an Empirical value that works, need to confirm the actual >>>>> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >>>>> @@ -82,6 +86,9 @@ struct ti_pipe3 { >>>>> struct clk *refclk; >>>>> struct clk *div_clk; >>>>> struct pipe3_dpll_map *dpll_map; >>>>> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >>>>> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >>>>> + bool sata_refclk_enabled; >>>>> }; >>>>> >>>>> static struct pipe3_dpll_map dpll_map_usb[] = { >>>>> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >>>>> u32 val; >>>>> unsigned long timeout; >>>>> >>>>> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >>>>> - * does not have internal DPLL >>>>> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >>>>> + * due to Errata i783 >>>>> */ >>>>> - if (of_device_is_compatible(phy->dev->of_node, >>>>> "ti,phy-pipe3-sata") || >>>>> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>>> + if (of_device_is_compatible(phy->dev->of_node, >>>>> "ti,phy-pipe3-sata") && >>>>> + !phy->dpll_reset_syscon) >>>>> + return 0; >>>>> + >>>>> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ >>>> >>>> I think it's better to fix it in this patch itself.. to disable clocks >>>> for PCIe. >>>>> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>>> return 0; >>>>> >>>>> /* Put DPLL in IDLE mode */ >>>>> @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) >>>>> return -EBUSY; >>>>> } >>>>> >>>>> + /* i783: SATA needs control bit toggle after PLL unlock */ >>>>> + if (of_device_is_compatible(phy->dev->of_node, >>>>> "ti,phy-pipe3-sata")) { >>>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>>> + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); >>>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>>> + SATA_PLL_SOFT_RESET, 0); >>>>> + } >>>>> + >>>>> ti_pipe3_disable_clocks(phy); >>>>> >>>>> return 0; >>>>> @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device >>>>> *pdev) >>>>> } >>>>> } else { >>>>> phy->wkupclk = ERR_PTR(-ENODEV); >>>>> + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, >>>>> + "syscon-pllreset"); >>>>> + if (IS_ERR(phy->dpll_reset_syscon)) { >>>>> + dev_info(&pdev->dev, >>>>> + "can't get syscon-pllreset, sata dpll won't idle\n"); >>>>> + phy->dpll_reset_syscon = NULL; >>>>> + } else { >>>>> + if (of_property_read_u32_index(node, >>>>> + "syscon-pllreset", 1, >>>>> + &phy->dpll_reset_reg)) { >>>>> + dev_err(&pdev->dev, >>>>> + "couldn't get pllreset reg. offset\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + } >>>>> } >>>>> >>>>> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >>>>> @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device >>>>> *pdev) >>>>> >>>>> platform_set_drvdata(pdev, phy); >>>>> pm_runtime_enable(phy->dev); >>>>> - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >>>>> - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >>>>> - clk_prepare_enable(phy->refclk); >>>>> + >>>>> + /* >>>>> + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 >>>>> + */ >>>>> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { >>>>> + if (!IS_ERR(phy->refclk)) { >>>>> + clk_prepare_enable(phy->refclk); >>>>> + phy->sata_refclk_enabled = true; >>>>> + } >>>>> + } >>>>> >>>>> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >>>>> if (IS_ERR(generic_phy)) >>>>> @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >>>>> *phy) >>>>> >>>>> static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) >>>>> { >>>>> - if (!IS_ERR(phy->refclk)) >>>>> + if (!IS_ERR(phy->refclk)) { >>>>> clk_disable_unprepare(phy->refclk); >>>>> + /* >>>>> + * SATA refclk needs an additional disable as we left it >>>>> + * on in probe to avoid Errata i783 >>>> >>>> not sure I get this. Why don't we remove it in probe? >>> >>> Because if we remove it from probe and if AHCI_PLATFORM is built as a >>> module then phy_init won't be called before kernel starts autodisabling >>> unused clocks and so SATA refclk gets disabled causing i783 and >>> breaking SATA. >> >> Just to be clear, this entire thing won't be required if uboot does SATA_PLL_SOFT_RESET no? >> > > Yes that is right. We don't yet have the u-boot fix and there is no guarantee all users move > to the u-boot with the fix, so this is important. Okay. Just thinking if we can do SATA_PLL_SOFT_RESET in probe to begin with. That way we don't have to do this additional clk_enable(). Thanks Kishon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-23 7:22 ` Kishon Vijay Abraham I @ 2015-05-26 14:20 ` Roger Quadros 0 siblings, 0 replies; 16+ messages in thread From: Roger Quadros @ 2015-05-26 14:20 UTC (permalink / raw) To: Kishon Vijay Abraham I, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Kishon, On 23/05/15 10:22, Kishon Vijay Abraham I wrote: > Roger, > > On Friday 22 May 2015 07:28 PM, Roger Quadros wrote: >> Kishon, >> >> On 22/05/15 14:34, Kishon Vijay Abraham I wrote: >>> Roger, >>> >>> On Wednesday 20 May 2015 07:17 PM, Roger Quadros wrote: >>>> Kishon, >>>> >>>> On 20/05/15 16:19, Kishon Vijay Abraham I wrote: >>>>> Hi Roger, >>>>> >>>>> On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >>>>>> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >>>>>> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >>>>>> >>>>>> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >>>>>> regmap access to this register which sits in the control module. >>>>>> >>>>>> If the register is not provided we fallback to the old behaviour >>>>>> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >>>>>> power states. >>>>>> >>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>>>> --- >>>>>> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >>>>>> drivers/phy/phy-ti-pipe3.c | 67 >>>>>> ++++++++++++++++++++---- >>>>>> 2 files changed, 74 insertions(+), 9 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>>> b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>>> index 305e3df..f0f5537 100644 >>>>>> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>>> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >>>>>> @@ -82,6 +82,9 @@ Optional properties: >>>>>> - id: If there are multiple instance of the same type, in order to >>>>>> differentiate between each instance "id" can be used (e.g., >>>>>> multi-lane PCIe >>>>>> PHY). If "id" is not provided, it is set to default value of '1'. >>>>>> + - syscon-pllreset: Handle to system control region that contains the >>>>>> + CTRL_CORE_SMA_SW_0 register and register offset to the >>>>>> CTRL_CORE_SMA_SW_0 >>>>>> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for >>>>>> sata_phy. >>>>>> >>>>>> This is usually a subnode of ocp2scp to which it is connected. >>>>>> >>>>>> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >>>>>> "sysclk", >>>>>> "refclk"; >>>>>> }; >>>>>> + >>>>>> +sata_phy: phy@4A096000 { >>>>>> + compatible = "ti,phy-pipe3-sata"; >>>>>> + reg = <0x4A096000 0x80>, /* phy_rx */ >>>>>> + <0x4A096400 0x64>, /* phy_tx */ >>>>>> + <0x4A096800 0x40>; /* pll_ctrl */ >>>>>> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >>>>>> + ctrl-module = <&omap_control_sata>; >>>>>> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >>>>>> + clock-names = "sysclk", "refclk"; >>>>>> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >>>>>> + #phy-cells = <0>; >>>>>> +}; >>>>>> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >>>>>> index e13a306..d730142 100644 >>>>>> --- a/drivers/phy/phy-ti-pipe3.c >>>>>> +++ b/drivers/phy/phy-ti-pipe3.c >>>>>> @@ -28,6 +28,8 @@ >>>>>> #include <linux/delay.h> >>>>>> #include <linux/phy/omap_control_phy.h> >>>>>> #include <linux/of_platform.h> >>>>>> +#include <linux/mfd/syscon.h> >>>>>> +#include <linux/regmap.h> >>>>>> >>>>>> #define PLL_STATUS 0x00000004 >>>>>> #define PLL_GO 0x00000008 >>>>>> @@ -52,6 +54,8 @@ >>>>>> #define PLL_LOCK 0x2 >>>>>> #define PLL_IDLE 0x1 >>>>>> >>>>>> +#define SATA_PLL_SOFT_RESET BIT(18) >>>>>> + >>>>>> /* >>>>>> * This is an Empirical value that works, need to confirm the actual >>>>>> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >>>>>> @@ -82,6 +86,9 @@ struct ti_pipe3 { >>>>>> struct clk *refclk; >>>>>> struct clk *div_clk; >>>>>> struct pipe3_dpll_map *dpll_map; >>>>>> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >>>>>> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >>>>>> + bool sata_refclk_enabled; >>>>>> }; >>>>>> >>>>>> static struct pipe3_dpll_map dpll_map_usb[] = { >>>>>> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >>>>>> u32 val; >>>>>> unsigned long timeout; >>>>>> >>>>>> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >>>>>> - * does not have internal DPLL >>>>>> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >>>>>> + * due to Errata i783 >>>>>> */ >>>>>> - if (of_device_is_compatible(phy->dev->of_node, >>>>>> "ti,phy-pipe3-sata") || >>>>>> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>>>> + if (of_device_is_compatible(phy->dev->of_node, >>>>>> "ti,phy-pipe3-sata") && >>>>>> + !phy->dpll_reset_syscon) >>>>>> + return 0; >>>>>> + >>>>>> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ >>>>> >>>>> I think it's better to fix it in this patch itself.. to disable clocks >>>>> for PCIe. >>>>>> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >>>>>> return 0; >>>>>> >>>>>> /* Put DPLL in IDLE mode */ >>>>>> @@ -276,6 +287,14 @@ static int ti_pipe3_exit(struct phy *x) >>>>>> return -EBUSY; >>>>>> } >>>>>> >>>>>> + /* i783: SATA needs control bit toggle after PLL unlock */ >>>>>> + if (of_device_is_compatible(phy->dev->of_node, >>>>>> "ti,phy-pipe3-sata")) { >>>>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>>>> + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); >>>>>> + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, >>>>>> + SATA_PLL_SOFT_RESET, 0); >>>>>> + } >>>>>> + >>>>>> ti_pipe3_disable_clocks(phy); >>>>>> >>>>>> return 0; >>>>>> @@ -350,6 +369,21 @@ static int ti_pipe3_probe(struct platform_device >>>>>> *pdev) >>>>>> } >>>>>> } else { >>>>>> phy->wkupclk = ERR_PTR(-ENODEV); >>>>>> + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, >>>>>> + "syscon-pllreset"); >>>>>> + if (IS_ERR(phy->dpll_reset_syscon)) { >>>>>> + dev_info(&pdev->dev, >>>>>> + "can't get syscon-pllreset, sata dpll won't idle\n"); >>>>>> + phy->dpll_reset_syscon = NULL; >>>>>> + } else { >>>>>> + if (of_property_read_u32_index(node, >>>>>> + "syscon-pllreset", 1, >>>>>> + &phy->dpll_reset_reg)) { >>>>>> + dev_err(&pdev->dev, >>>>>> + "couldn't get pllreset reg. offset\n"); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + } >>>>>> } >>>>>> >>>>>> if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { >>>>>> @@ -402,9 +436,16 @@ static int ti_pipe3_probe(struct platform_device >>>>>> *pdev) >>>>>> >>>>>> platform_set_drvdata(pdev, phy); >>>>>> pm_runtime_enable(phy->dev); >>>>>> - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ >>>>>> - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) >>>>>> - clk_prepare_enable(phy->refclk); >>>>>> + >>>>>> + /* >>>>>> + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 >>>>>> + */ >>>>>> + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { >>>>>> + if (!IS_ERR(phy->refclk)) { >>>>>> + clk_prepare_enable(phy->refclk); >>>>>> + phy->sata_refclk_enabled = true; >>>>>> + } >>>>>> + } >>>>>> >>>>>> generic_phy = devm_phy_create(phy->dev, NULL, &ops); >>>>>> if (IS_ERR(generic_phy)) >>>>>> @@ -443,8 +484,17 @@ static int ti_pipe3_enable_refclk(struct ti_pipe3 >>>>>> *phy) >>>>>> >>>>>> static void ti_pipe3_disable_refclk(struct ti_pipe3 *phy) >>>>>> { >>>>>> - if (!IS_ERR(phy->refclk)) >>>>>> + if (!IS_ERR(phy->refclk)) { >>>>>> clk_disable_unprepare(phy->refclk); >>>>>> + /* >>>>>> + * SATA refclk needs an additional disable as we left it >>>>>> + * on in probe to avoid Errata i783 >>>>> >>>>> not sure I get this. Why don't we remove it in probe? >>>> >>>> Because if we remove it from probe and if AHCI_PLATFORM is built as a >>>> module then phy_init won't be called before kernel starts autodisabling >>>> unused clocks and so SATA refclk gets disabled causing i783 and >>>> breaking SATA. >>> >>> Just to be clear, this entire thing won't be required if uboot does SATA_PLL_SOFT_RESET no? >>> >> >> Yes that is right. We don't yet have the u-boot fix and there is no guarantee all users move >> to the u-boot with the fix, so this is important. > > Okay. Just thinking if we can do SATA_PLL_SOFT_RESET in probe to begin with. That way we don't have to do this additional clk_enable(). > I had tried it but it didn't work. cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-05-20 13:19 ` Kishon Vijay Abraham I 2015-05-20 13:47 ` Roger Quadros @ 2015-06-02 8:17 ` Roger Quadros 1 sibling, 0 replies; 16+ messages in thread From: Roger Quadros @ 2015-06-02 8:17 UTC (permalink / raw) To: Kishon Vijay Abraham I, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap Kishon, On 20/05/15 16:19, Kishon Vijay Abraham I wrote: > Hi Roger, > > On Tuesday 12 May 2015 09:37 PM, Roger Quadros wrote: >> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled >> between a SATA DPLL unlock and re-lock to prevent SATA lockup. >> >> Introduce a new DT parameter 'syscon-pllreset' to provide the syscon >> regmap access to this register which sits in the control module. >> >> If the register is not provided we fallback to the old behaviour >> i.e. SATA DPLL refclk will not be disabled and we prevent SoC low >> power states. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ >> drivers/phy/phy-ti-pipe3.c | 67 ++++++++++++++++++++---- >> 2 files changed, 74 insertions(+), 9 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt >> index 305e3df..f0f5537 100644 >> --- a/Documentation/devicetree/bindings/phy/ti-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt >> @@ -82,6 +82,9 @@ Optional properties: >> - id: If there are multiple instance of the same type, in order to >> differentiate between each instance "id" can be used (e.g., multi-lane PCIe >> PHY). If "id" is not provided, it is set to default value of '1'. >> + - syscon-pllreset: Handle to system control region that contains the >> + CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0 >> + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy. >> >> This is usually a subnode of ocp2scp to which it is connected. >> >> @@ -100,3 +103,16 @@ usb3phy@4a084400 { >> "sysclk", >> "refclk"; >> }; >> + >> +sata_phy: phy@4A096000 { >> + compatible = "ti,phy-pipe3-sata"; >> + reg = <0x4A096000 0x80>, /* phy_rx */ >> + <0x4A096400 0x64>, /* phy_tx */ >> + <0x4A096800 0x40>; /* pll_ctrl */ >> + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; >> + ctrl-module = <&omap_control_sata>; >> + clocks = <&sys_clkin1>, <&sata_ref_clk>; >> + clock-names = "sysclk", "refclk"; >> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >> + #phy-cells = <0>; >> +}; >> diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c >> index e13a306..d730142 100644 >> --- a/drivers/phy/phy-ti-pipe3.c >> +++ b/drivers/phy/phy-ti-pipe3.c >> @@ -28,6 +28,8 @@ >> #include <linux/delay.h> >> #include <linux/phy/omap_control_phy.h> >> #include <linux/of_platform.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/regmap.h> >> >> #define PLL_STATUS 0x00000004 >> #define PLL_GO 0x00000008 >> @@ -52,6 +54,8 @@ >> #define PLL_LOCK 0x2 >> #define PLL_IDLE 0x1 >> >> +#define SATA_PLL_SOFT_RESET BIT(18) >> + >> /* >> * This is an Empirical value that works, need to confirm the actual >> * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status >> @@ -82,6 +86,9 @@ struct ti_pipe3 { >> struct clk *refclk; >> struct clk *div_clk; >> struct pipe3_dpll_map *dpll_map; >> + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ >> + unsigned int dpll_reset_reg; /* reg. index within syscon */ >> + bool sata_refclk_enabled; >> }; >> >> static struct pipe3_dpll_map dpll_map_usb[] = { >> @@ -249,11 +256,15 @@ static int ti_pipe3_exit(struct phy *x) >> u32 val; >> unsigned long timeout; >> >> - /* SATA DPLL can't be powered down due to Errata i783 and PCIe >> - * does not have internal DPLL >> + /* If dpll_reset_syscon is not present we wont power down SATA DPLL >> + * due to Errata i783 >> */ >> - if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") || >> - of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-pcie")) >> + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") && >> + !phy->dpll_reset_syscon) >> + return 0; >> + >> + /* PCIe doesn't have DPLL. FIXME: need to disable clocks though */ > > I think it's better to fix it in this patch itself.. to disable clocks for PCIe. I was not sure if it will break PCIe in any way. moreover it is not part of the patch subject. do you still want me to fix it in this patch? cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY 2015-05-12 16:07 [PATCH 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM Roger Quadros 2015-05-12 16:07 ` [PATCH 1/3] phy: ti-pipe3: fix suspend Roger Quadros 2015-05-12 16:07 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Roger Quadros @ 2015-05-12 16:07 ` Roger Quadros 2015-07-14 10:34 ` Tony Lindgren 2 siblings, 1 reply; 16+ messages in thread From: Roger Quadros @ 2015-05-12 16:07 UTC (permalink / raw) To: kishon, tony Cc: nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap, Roger Quadros This register is required to be passed to the SATA PHY driver to workaround errata i783 (SATA Lockup After SATA DPLL Unlock/Relock). Signed-off-by: Roger Quadros <rogerq@ti.com> Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- arch/arm/boot/dts/dra7.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi index f03a091..260f300 100644 --- a/arch/arm/boot/dts/dra7.dtsi +++ b/arch/arm/boot/dts/dra7.dtsi @@ -1135,6 +1135,7 @@ ctrl-module = <&omap_control_sata>; clocks = <&sys_clkin1>, <&sata_ref_clk>; clock-names = "sysclk", "refclk"; + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; #phy-cells = <0>; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY 2015-05-12 16:07 ` [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Roger Quadros @ 2015-07-14 10:34 ` Tony Lindgren 2015-07-14 10:39 ` Roger Quadros 0 siblings, 1 reply; 16+ messages in thread From: Tony Lindgren @ 2015-07-14 10:34 UTC (permalink / raw) To: Roger Quadros Cc: kishon, nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap * Roger Quadros <rogerq@ti.com> [150512 09:08]: > This register is required to be passed to the SATA PHY driver > to workaround errata i783 (SATA Lockup After SATA DPLL Unlock/Relock). > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > arch/arm/boot/dts/dra7.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi > index f03a091..260f300 100644 > --- a/arch/arm/boot/dts/dra7.dtsi > +++ b/arch/arm/boot/dts/dra7.dtsi > @@ -1135,6 +1135,7 @@ > ctrl-module = <&omap_control_sata>; > clocks = <&sys_clkin1>, <&sata_ref_clk>; > clock-names = "sysclk", "refclk"; > + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; > #phy-cells = <0>; > }; > Looks like this one is still pending driver changes, please resend when those are resolved. I'll untag this one for now. Regards, Tony ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY 2015-07-14 10:34 ` Tony Lindgren @ 2015-07-14 10:39 ` Roger Quadros 0 siblings, 0 replies; 16+ messages in thread From: Roger Quadros @ 2015-07-14 10:39 UTC (permalink / raw) To: Tony Lindgren Cc: kishon, nm, nsekhar, balbi, grygorii.strashko, linux-kernel, linux-omap On 14/07/15 13:34, Tony Lindgren wrote: > * Roger Quadros <rogerq@ti.com> [150512 09:08]: >> This register is required to be passed to the SATA PHY driver >> to workaround errata i783 (SATA Lockup After SATA DPLL Unlock/Relock). >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> arch/arm/boot/dts/dra7.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi >> index f03a091..260f300 100644 >> --- a/arch/arm/boot/dts/dra7.dtsi >> +++ b/arch/arm/boot/dts/dra7.dtsi >> @@ -1135,6 +1135,7 @@ >> ctrl-module = <&omap_control_sata>; >> clocks = <&sys_clkin1>, <&sata_ref_clk>; >> clock-names = "sysclk", "refclk"; >> + syscon-pllreset = <&dra7_ctrl_core 0x3fc>; >> #phy-cells = <0>; >> }; >> > > Looks like this one is still pending driver changes, please > resend when those are resolved. I'll untag this one for now. I've sent a v2 of this series here http://thread.gmane.org/gmane.linux.kernel/1967419 cheers, -roger ^ permalink raw reply [flat|nested] 16+ messages in thread
* [GIT PULL 0/3] phy: for 4.2 -rc @ 2015-08-04 16:47 Kishon Vijay Abraham I 2015-08-04 16:47 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Kishon Vijay Abraham I 0 siblings, 1 reply; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-08-04 16:47 UTC (permalink / raw) To: gregkh; +Cc: kishon, linux-kernel Hi Greg, Got a couple of more fixes for this -rc cycle. One fixes a compiler error when the driver is built as module and the other fixes a SATA lockup. The fix also requires a dts patch which I've included in this series after Tony's ACK. Please find the pull request below and consider merging in this -rc cycle. Let me know If I have to modify something. Cheers Kishon The following changes since commit dcb54fcb3483862e993abdae99cec22fb3ae4099: phy: berlin-usb: fix divider for BG2 (2015-07-15 20:02:09 +0530) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git tags/phy-for-4.2-rc6 for you to fetch changes up to 257d5d9a9f19ee6c6801589c74791d5422c374e9: ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY (2015-08-04 21:11:50 +0530) ---------------------------------------------------------------- phy: for 4.2-rc6 *) Fix compiler error when sun4i usb phy driver is built as module *) Fix SATA Lockup issue in dra7 SoC Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> ---------------------------------------------------------------- Hans de Goede (1): phy-sun4i-usb: Add missing EXPORT_SYMBOL_GPL for sun4i_usb_phy_set_squelch_detect Roger Quadros (2): phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ arch/arm/boot/dts/dra7.dtsi | 1 + drivers/phy/phy-sun4i-usb.c | 1 + drivers/phy/phy-ti-pipe3.c | 61 +++++++++++++++++++--- 4 files changed, 73 insertions(+), 6 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock 2015-08-04 16:47 [GIT PULL 0/3] phy: for 4.2 -rc Kishon Vijay Abraham I @ 2015-08-04 16:47 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 16+ messages in thread From: Kishon Vijay Abraham I @ 2015-08-04 16:47 UTC (permalink / raw) To: gregkh; +Cc: kishon, linux-kernel From: Roger Quadros <rogerq@ti.com> SATA_PLL_SOFT_RESET bit of CTRL_CORE_SMA_SW_0 must be toggled between a SATA DPLL unlock and re-lock to prevent SATA lockup. Introduce a new DT parameter 'syscon-pllreset' to provide the syscon regmap access to this register which sits in the control module. If the register is not provided we fallback to the old behaviour i.e. SATA DPLL refclk will not be disabled and we prevent SoC low power states. Signed-off-by: Roger Quadros <rogerq@ti.com> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- Documentation/devicetree/bindings/phy/ti-phy.txt | 16 ++++++ drivers/phy/phy-ti-pipe3.c | 61 +++++++++++++++++++--- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt index 305e3df..9cf9446 100644 --- a/Documentation/devicetree/bindings/phy/ti-phy.txt +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt @@ -82,6 +82,9 @@ Optional properties: - id: If there are multiple instance of the same type, in order to differentiate between each instance "id" can be used (e.g., multi-lane PCIe PHY). If "id" is not provided, it is set to default value of '1'. + - syscon-pllreset: Handle to system control region that contains the + CTRL_CORE_SMA_SW_0 register and register offset to the CTRL_CORE_SMA_SW_0 + register that contains the SATA_PLL_SOFT_RESET bit. Only valid for sata_phy. This is usually a subnode of ocp2scp to which it is connected. @@ -100,3 +103,16 @@ usb3phy@4a084400 { "sysclk", "refclk"; }; + +sata_phy: phy@4A096000 { + compatible = "ti,phy-pipe3-sata"; + reg = <0x4A096000 0x80>, /* phy_rx */ + <0x4A096400 0x64>, /* phy_tx */ + <0x4A096800 0x40>; /* pll_ctrl */ + reg-names = "phy_rx", "phy_tx", "pll_ctrl"; + ctrl-module = <&omap_control_sata>; + clocks = <&sys_clkin1>, <&sata_ref_clk>; + clock-names = "sysclk", "refclk"; + syscon-pllreset = <&scm_conf 0x3fc>; + #phy-cells = <0>; +}; diff --git a/drivers/phy/phy-ti-pipe3.c b/drivers/phy/phy-ti-pipe3.c index 3510b81..08020dc 100644 --- a/drivers/phy/phy-ti-pipe3.c +++ b/drivers/phy/phy-ti-pipe3.c @@ -28,6 +28,8 @@ #include <linux/delay.h> #include <linux/phy/omap_control_phy.h> #include <linux/of_platform.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #define PLL_STATUS 0x00000004 #define PLL_GO 0x00000008 @@ -52,6 +54,8 @@ #define PLL_LOCK 0x2 #define PLL_IDLE 0x1 +#define SATA_PLL_SOFT_RESET BIT(18) + /* * This is an Empirical value that works, need to confirm the actual * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status @@ -82,6 +86,9 @@ struct ti_pipe3 { struct clk *refclk; struct clk *div_clk; struct pipe3_dpll_map *dpll_map; + struct regmap *dpll_reset_syscon; /* ctrl. reg. acces */ + unsigned int dpll_reset_reg; /* reg. index within syscon */ + bool sata_refclk_enabled; }; static struct pipe3_dpll_map dpll_map_usb[] = { @@ -249,8 +256,11 @@ static int ti_pipe3_exit(struct phy *x) u32 val; unsigned long timeout; - /* SATA DPLL can't be powered down due to Errata i783 */ - if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) + /* If dpll_reset_syscon is not present we wont power down SATA DPLL + * due to Errata i783 + */ + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata") && + !phy->dpll_reset_syscon) return 0; /* PCIe doesn't have internal DPLL */ @@ -276,6 +286,14 @@ static int ti_pipe3_exit(struct phy *x) } } + /* i783: SATA needs control bit toggle after PLL unlock */ + if (of_device_is_compatible(phy->dev->of_node, "ti,phy-pipe3-sata")) { + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, + SATA_PLL_SOFT_RESET, SATA_PLL_SOFT_RESET); + regmap_update_bits(phy->dpll_reset_syscon, phy->dpll_reset_reg, + SATA_PLL_SOFT_RESET, 0); + } + ti_pipe3_disable_clocks(phy); return 0; @@ -350,6 +368,21 @@ static int ti_pipe3_probe(struct platform_device *pdev) } } else { phy->wkupclk = ERR_PTR(-ENODEV); + phy->dpll_reset_syscon = syscon_regmap_lookup_by_phandle(node, + "syscon-pllreset"); + if (IS_ERR(phy->dpll_reset_syscon)) { + dev_info(&pdev->dev, + "can't get syscon-pllreset, sata dpll won't idle\n"); + phy->dpll_reset_syscon = NULL; + } else { + if (of_property_read_u32_index(node, + "syscon-pllreset", 1, + &phy->dpll_reset_reg)) { + dev_err(&pdev->dev, + "couldn't get pllreset reg. offset\n"); + return -EINVAL; + } + } } if (of_device_is_compatible(node, "ti,phy-pipe3-pcie")) { @@ -402,10 +435,16 @@ static int ti_pipe3_probe(struct platform_device *pdev) platform_set_drvdata(pdev, phy); pm_runtime_enable(phy->dev); - /* Prevent auto-disable of refclk for SATA PHY due to Errata i783 */ - if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) - if (!IS_ERR(phy->refclk)) + + /* + * Prevent auto-disable of refclk for SATA PHY due to Errata i783 + */ + if (of_device_is_compatible(node, "ti,phy-pipe3-sata")) { + if (!IS_ERR(phy->refclk)) { clk_prepare_enable(phy->refclk); + phy->sata_refclk_enabled = true; + } + } generic_phy = devm_phy_create(phy->dev, NULL, &ops); if (IS_ERR(generic_phy)) @@ -472,8 +511,18 @@ static void ti_pipe3_disable_clocks(struct ti_pipe3 *phy) { if (!IS_ERR(phy->wkupclk)) clk_disable_unprepare(phy->wkupclk); - if (!IS_ERR(phy->refclk)) + if (!IS_ERR(phy->refclk)) { clk_disable_unprepare(phy->refclk); + /* + * SATA refclk needs an additional disable as we left it + * on in probe to avoid Errata i783 + */ + if (phy->sata_refclk_enabled) { + clk_disable_unprepare(phy->refclk); + phy->sata_refclk_enabled = false; + } + } + if (!IS_ERR(phy->div_clk)) clk_disable_unprepare(phy->div_clk); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-08-04 16:48 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-12 16:07 [PATCH 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM Roger Quadros 2015-05-12 16:07 ` [PATCH 1/3] phy: ti-pipe3: fix suspend Roger Quadros 2015-05-20 13:04 ` Kishon Vijay Abraham I 2015-05-20 13:48 ` Roger Quadros 2015-05-12 16:07 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Roger Quadros 2015-05-20 13:19 ` Kishon Vijay Abraham I 2015-05-20 13:47 ` Roger Quadros 2015-05-22 11:34 ` Kishon Vijay Abraham I 2015-05-22 13:58 ` Roger Quadros 2015-05-23 7:22 ` Kishon Vijay Abraham I 2015-05-26 14:20 ` Roger Quadros 2015-06-02 8:17 ` Roger Quadros 2015-05-12 16:07 ` [PATCH 3/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY Roger Quadros 2015-07-14 10:34 ` Tony Lindgren 2015-07-14 10:39 ` Roger Quadros 2015-08-04 16:47 [GIT PULL 0/3] phy: for 4.2 -rc Kishon Vijay Abraham I 2015-08-04 16:47 ` [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock Kishon Vijay Abraham I
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).