Messages in this thread | | | Date | Sat, 23 May 2015 12:52:21 +0530 | From | Kishon Vijay Abraham I <> | Subject | Re: [PATCH 2/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock |
| |
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
| |