[PATCH v3 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM (core-retention)

[PATCH v3 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM (core-retention)

[Roger Quadros]

Hi,

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 without preventing
core-retention.

Depends on http://article.gmane.org/gmane.linux.ports.arm.omap/126670.

Changelog:
v3:
- used scm_conf node in dra7.dtsi for control register access.

v2:
- Fixed pcie disable_clocks. Addressed review comments.

cheers,
-roger

Roger Quadros (3):
  phy: ti-pipe3: i783 workaround for SATA lockup after dpll
    unlock/relock
  ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY
  ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

 Documentation/devicetree/bindings/phy/ti-phy.txt | 16 +++++++
 arch/arm/boot/dts/dra7.dtsi                      | 20 ++++----
 drivers/phy/phy-ti-pipe3.c                       | 61 +++++++++++++++++++++---
 3 files changed, 80 insertions(+), 17 deletions(-)

-- 
2.1.4

[PATCH v3 1/3] phy: ti-pipe3: i783 workaround for SATA lockup after dpll unlock/relock

[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                       | 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);
 }
-- 
2.1.4

[PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[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>
---
 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 8f1e25b..4a0718c 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1140,6 +1140,7 @@
 				ctrl-module = <&omap_control_sata>;
 				clocks = <&sys_clkin1>, <&sata_ref_clk>;
 				clock-names = "sysclk", "refclk";
+				syscon-pllreset = <&scm_conf 0x3fc>;
 				#phy-cells = <0>;
 			};
 
-- 
2.1.4

Re: [PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[Tero Kristo]

On 07/17/2015 04:47 PM, Roger Quadros wrote:
> 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>
> ---
>   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 8f1e25b..4a0718c 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1140,6 +1140,7 @@
>   				ctrl-module = <&omap_control_sata>;
>   				clocks = <&sys_clkin1>, <&sata_ref_clk>;
>   				clock-names = "sysclk", "refclk";
> +				syscon-pllreset = <&scm_conf 0x3fc>;
>   				#phy-cells = <0>;
>   			};
>
>

Looks fine to me.

Make sure you use this register via regmap_update_bits only, seeing 
there is another potential user for the same register.

-Tero

Re: [PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[Roger Quadros]

On 20/07/15 15:08, Tero Kristo wrote:
> On 07/17/2015 04:47 PM, Roger Quadros wrote:
>> 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>
>> ---
>>   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 8f1e25b..4a0718c 100644
>> --- a/arch/arm/boot/dts/dra7.dtsi
>> +++ b/arch/arm/boot/dts/dra7.dtsi
>> @@ -1140,6 +1140,7 @@
>>                   ctrl-module = <&omap_control_sata>;
>>                   clocks = <&sys_clkin1>, <&sata_ref_clk>;
>>                   clock-names = "sysclk", "refclk";
>> +                syscon-pllreset = <&scm_conf 0x3fc>;
>>                   #phy-cells = <0>;
>>               };
>>
>>
> 
> Looks fine to me.
> 
> Make sure you use this register via regmap_update_bits only, seeing there is another potential user for the same register.

Yes. Patch 1 is the user using regmap_update_bits.

cheers,
-roger

Re: [PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[Roger Quadros]

Tony,

On 17/07/15 16:47, Roger Quadros wrote:
> 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>

Can you please Ack or pick this for -fixes.
Kishon has already picked patch 1 in this series. Thanks.

cheers,
-roger

> ---
>  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 8f1e25b..4a0718c 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1140,6 +1140,7 @@
>  				ctrl-module = <&omap_control_sata>;
>  				clocks = <&sys_clkin1>, <&sata_ref_clk>;
>  				clock-names = "sysclk", "refclk";
> +				syscon-pllreset = <&scm_conf 0x3fc>;
>  				#phy-cells = <0>;
>  			};
>  
> 

Re: [PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[Tony Lindgren]

* Roger Quadros <rogerq@ti.com> [150804 01:22]:
> Tony,
> 
> On 17/07/15 16:47, Roger Quadros wrote:
> > 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>
> 
> Can you please Ack or pick this for -fixes.
> Kishon has already picked patch 1 in this series. Thanks.

Best that Kishon takes both then:

Acked-by: Tony Lindgren <tony@atomide.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 8f1e25b..4a0718c 100644
> > --- a/arch/arm/boot/dts/dra7.dtsi
> > +++ b/arch/arm/boot/dts/dra7.dtsi
> > @@ -1140,6 +1140,7 @@
> >  				ctrl-module = <&omap_control_sata>;
> >  				clocks = <&sys_clkin1>, <&sata_ref_clk>;
> >  				clock-names = "sysclk", "refclk";
> > +				syscon-pllreset = <&scm_conf 0x3fc>;
> >  				#phy-cells = <0>;
> >  			};
> >  
> > 

Re: [PATCH v3 2/3] ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY

[Kishon Vijay Abraham I]

On Tuesday 04 August 2015 02:11 PM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [150804 01:22]:
>> Tony,
>>
>> On 17/07/15 16:47, Roger Quadros wrote:
>>> 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>
>>
>> Can you please Ack or pick this for -fixes.
>> Kishon has already picked patch 1 in this series. Thanks.
> 
> Best that Kishon takes both then:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

merged, thanks.

-Kishon
>  
>>> ---
>>>  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 8f1e25b..4a0718c 100644
>>> --- a/arch/arm/boot/dts/dra7.dtsi
>>> +++ b/arch/arm/boot/dts/dra7.dtsi
>>> @@ -1140,6 +1140,7 @@
>>>  				ctrl-module = <&omap_control_sata>;
>>>  				clocks = <&sys_clkin1>, <&sata_ref_clk>;
>>>  				clock-names = "sysclk", "refclk";
>>> +				syscon-pllreset = <&scm_conf 0x3fc>;
>>>  				#phy-cells = <0>;
>>>  			};
>>>  
>>>

[PATCH v3 3/3] ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

[Roger Quadros]

scm_conf1 maps the control register address space after the
padconf till the end.

Fix the scm_conf and pmx_core resource lengths. We need to add
4 bytes to include the last 32-bit register space.

Remove the redundant dra7_ctrl_core and dra7_ctrl_general nodes.
They are not used by anyone and no longer needed as they are
covered by scm_conf and scm_conf1.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 arch/arm/boot/dts/dra7.dtsi | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 4a0718c..d07c34c 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -141,7 +141,7 @@
 				dra7_pmx_core: pinmux@1400 {
 					compatible = "ti,dra7-padconf",
 						     "pinctrl-single";
-					reg = <0x1400 0x0464>;
+					reg = <0x1400 0x0468>;
 					#address-cells = <1>;
 					#size-cells = <0>;
 					#interrupt-cells = <1>;
@@ -149,6 +149,13 @@
 					pinctrl-single,register-width = <32>;
 					pinctrl-single,function-mask = <0x3fffffff>;
 				};
+
+				scm_conf1: scm_conf@1 {
+					compatible = "syscon";
+					reg = <0x1868 0x03e0>;
+					#address-cells = <1>;
+					#size-cells = <1>;
+				};
 			};
 
 			cm_core_aon: cm_core_aon@5000 {
@@ -286,16 +293,6 @@
 				#thermal-sensor-cells = <1>;
 		};
 
-		dra7_ctrl_core: ctrl_core@4a002000 {
-			compatible = "syscon";
-			reg = <0x4a002000 0x6d0>;
-		};
-
-		dra7_ctrl_general: tisyscon@4a002e00 {
-			compatible = "syscon";
-			reg = <0x4a002e00 0x7c>;
-		};
-
 		sdma: dma-controller@4a056000 {
 			compatible = "ti,omap4430-sdma";
 			reg = <0x4a056000 0x1000>;
-- 
2.1.4

Re: [PATCH v3 3/3] ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

[Tero Kristo]

On 07/17/2015 04:47 PM, Roger Quadros wrote:
> scm_conf1 maps the control register address space after the
> padconf till the end.
>
> Fix the scm_conf and pmx_core resource lengths. We need to add
> 4 bytes to include the last 32-bit register space.
>
> Remove the redundant dra7_ctrl_core and dra7_ctrl_general nodes.
> They are not used by anyone and no longer needed as they are
> covered by scm_conf and scm_conf1.

Looks like you are doing three things in this patch, maybe split it up 
as such?

>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>   arch/arm/boot/dts/dra7.dtsi | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 4a0718c..d07c34c 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -141,7 +141,7 @@
>   				dra7_pmx_core: pinmux@1400 {
>   					compatible = "ti,dra7-padconf",
>   						     "pinctrl-single";
> -					reg = <0x1400 0x0464>;
> +					reg = <0x1400 0x0468>;
>   					#address-cells = <1>;
>   					#size-cells = <0>;
>   					#interrupt-cells = <1>;
> @@ -149,6 +149,13 @@
>   					pinctrl-single,register-width = <32>;
>   					pinctrl-single,function-mask = <0x3fffffff>;
>   				};
> +
> +				scm_conf1: scm_conf@1 {

Should be ... scm_conf@1868?

Are there any users for this area anyway? I don't think we should map 
this area just for fun of it. Mostly it looks like this contains efuse 
values for OPPs, which should be mapped from the OPP layer, not as a 
generic syscon.

> +					compatible = "syscon";
> +					reg = <0x1868 0x03e0>;
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +				};
>   			};
>
>   			cm_core_aon: cm_core_aon@5000 {
> @@ -286,16 +293,6 @@
>   				#thermal-sensor-cells = <1>;
>   		};
>
> -		dra7_ctrl_core: ctrl_core@4a002000 {
> -			compatible = "syscon";
> -			reg = <0x4a002000 0x6d0>;
> -		};
> -
> -		dra7_ctrl_general: tisyscon@4a002e00 {
> -			compatible = "syscon";
> -			reg = <0x4a002e00 0x7c>;
> -		};

I thought I had removed these already and yes... this was done in commit 
d919501feffa8715147582c3ffce96fad0c7016f already, but seems they were 
added back in a resolution for a merge conflict later.

-Tero

> -
>   		sdma: dma-controller@4a056000 {
>   			compatible = "ti,omap4430-sdma";
>   			reg = <0x4a056000 0x1000>;
>

Re: [PATCH v3 3/3] ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

[Kishon Vijay Abraham I]

Hi,

On Monday 20 July 2015 05:34 PM, Tero Kristo wrote:
> On 07/17/2015 04:47 PM, Roger Quadros wrote:
>> scm_conf1 maps the control register address space after the
>> padconf till the end.
>>
>> Fix the scm_conf and pmx_core resource lengths. We need to add
>> 4 bytes to include the last 32-bit register space.
>>
>> Remove the redundant dra7_ctrl_core and dra7_ctrl_general nodes.
>> They are not used by anyone and no longer needed as they are
>> covered by scm_conf and scm_conf1.
> 
> Looks like you are doing three things in this patch, maybe split it up 
> as such?
> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   arch/arm/boot/dts/dra7.dtsi | 19 ++++++++-----------
>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>> index 4a0718c..d07c34c 100644
>> --- a/arch/arm/boot/dts/dra7.dtsi
>> +++ b/arch/arm/boot/dts/dra7.dtsi
>> @@ -141,7 +141,7 @@
>>   				dra7_pmx_core: pinmux@1400 {
>>   					compatible = "ti,dra7-padconf",
>>   						     "pinctrl-single";
>> -					reg = <0x1400 0x0464>;
>> +					reg = <0x1400 0x0468>;
>>   					#address-cells = <1>;
>>   					#size-cells = <0>;
>>   					#interrupt-cells = <1>;
>> @@ -149,6 +149,13 @@
>>   					pinctrl-single,register-width = <32>;
>>   					pinctrl-single,function-mask = <0x3fffffff>;
>>   				};
>> +
>> +				scm_conf1: scm_conf@1 {
> 
> Should be ... scm_conf@1868?
> 
> Are there any users for this area anyway? I don't think we should map 
> this area just for fun of it. Mostly it looks like this contains efuse 
> values for OPPs, which should be mapped from the OPP layer, not as a 
> generic syscon.

The last few registers are used for PCIe PHY and I'll be needing it for the
next version of my patch series.

Thanks
Kishon

Re: [PATCH v3 3/3] ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

[Roger Quadros]

On 21/07/15 08:11, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Monday 20 July 2015 05:34 PM, Tero Kristo wrote:
>> On 07/17/2015 04:47 PM, Roger Quadros wrote:
>>> scm_conf1 maps the control register address space after the
>>> padconf till the end.
>>>
>>> Fix the scm_conf and pmx_core resource lengths. We need to add
>>> 4 bytes to include the last 32-bit register space.
>>>
>>> Remove the redundant dra7_ctrl_core and dra7_ctrl_general nodes.
>>> They are not used by anyone and no longer needed as they are
>>> covered by scm_conf and scm_conf1.
>>
>> Looks like you are doing three things in this patch, maybe split it up 
>> as such?
>>
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>   arch/arm/boot/dts/dra7.dtsi | 19 ++++++++-----------
>>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>>> index 4a0718c..d07c34c 100644
>>> --- a/arch/arm/boot/dts/dra7.dtsi
>>> +++ b/arch/arm/boot/dts/dra7.dtsi
>>> @@ -141,7 +141,7 @@
>>>   				dra7_pmx_core: pinmux@1400 {
>>>   					compatible = "ti,dra7-padconf",
>>>   						     "pinctrl-single";
>>> -					reg = <0x1400 0x0464>;
>>> +					reg = <0x1400 0x0468>;
>>>   					#address-cells = <1>;
>>>   					#size-cells = <0>;
>>>   					#interrupt-cells = <1>;
>>> @@ -149,6 +149,13 @@
>>>   					pinctrl-single,register-width = <32>;
>>>   					pinctrl-single,function-mask = <0x3fffffff>;
>>>   				};
>>> +
>>> +				scm_conf1: scm_conf@1 {
>>
>> Should be ... scm_conf@1868?
>>
>> Are there any users for this area anyway? I don't think we should map 
>> this area just for fun of it. Mostly it looks like this contains efuse 
>> values for OPPs, which should be mapped from the OPP layer, not as a 
>> generic syscon.
> 
> The last few registers are used for PCIe PHY and I'll be needing it for the
> next version of my patch series.

OK noted. Will exclude the PCIe registers from this region.

cheers,
-roger

Re: [PATCH v3 3/3] ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes

[Roger Quadros]

On 20/07/15 15:04, Tero Kristo wrote:
> On 07/17/2015 04:47 PM, Roger Quadros wrote:
>> scm_conf1 maps the control register address space after the
>> padconf till the end.
>>
>> Fix the scm_conf and pmx_core resource lengths. We need to add
>> 4 bytes to include the last 32-bit register space.
>>
>> Remove the redundant dra7_ctrl_core and dra7_ctrl_general nodes.
>> They are not used by anyone and no longer needed as they are
>> covered by scm_conf and scm_conf1.
> 
> Looks like you are doing three things in this patch, maybe split it up as such?

OK.

> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>   arch/arm/boot/dts/dra7.dtsi | 19 ++++++++-----------
>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>> index 4a0718c..d07c34c 100644
>> --- a/arch/arm/boot/dts/dra7.dtsi
>> +++ b/arch/arm/boot/dts/dra7.dtsi
>> @@ -141,7 +141,7 @@
>>                   dra7_pmx_core: pinmux@1400 {
>>                       compatible = "ti,dra7-padconf",
>>                                "pinctrl-single";
>> -                    reg = <0x1400 0x0464>;
>> +                    reg = <0x1400 0x0468>;
>>                       #address-cells = <1>;
>>                       #size-cells = <0>;
>>                       #interrupt-cells = <1>;
>> @@ -149,6 +149,13 @@
>>                       pinctrl-single,register-width = <32>;
>>                       pinctrl-single,function-mask = <0x3fffffff>;
>>                   };
>> +
>> +                scm_conf1: scm_conf@1 {
> 
> Should be ... scm_conf@1868?

OK.

> 
> Are there any users for this area anyway? I don't think we should map this area just for fun of it. Mostly it looks like this contains efuse values for OPPs, which should be mapped from the OPP layer, not as a generic syscon.

OK then, I'll only include the CTRL_CORE_SMA_SW_2..9 registers in this region.


cheers,
-roger

> 
>> +                    compatible = "syscon";
>> +                    reg = <0x1868 0x03e0>;
>> +                    #address-cells = <1>;
>> +                    #size-cells = <1>;
>> +                };
>>               };
>>
>>               cm_core_aon: cm_core_aon@5000 {
>> @@ -286,16 +293,6 @@
>>                   #thermal-sensor-cells = <1>;
>>           };
>>
>> -        dra7_ctrl_core: ctrl_core@4a002000 {
>> -            compatible = "syscon";
>> -            reg = <0x4a002000 0x6d0>;
>> -        };
>> -
>> -        dra7_ctrl_general: tisyscon@4a002e00 {
>> -            compatible = "syscon";
>> -            reg = <0x4a002e00 0x7c>;
>> -        };
> 
> I thought I had removed these already and yes... this was done in commit d919501feffa8715147582c3ffce96fad0c7016f already, but seems they were added back in a resolution for a merge conflict later.
> 
> -Tero
> 
>> -
>>           sdma: dma-controller@4a056000 {
>>               compatible = "ti,omap4430-sdma";
>>               reg = <0x4a056000 0x1000>;
>>
> 

Re: [PATCH v3 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM (core-retention)

[Roger Quadros]

Kishon & Tony,

You can please pick the first 2 patches for -next.

The 3rd patch is not required for SATA and I will send it as
a separate series.

cheers,
-roger

On 17/07/15 16:47, Roger Quadros wrote:
> Hi,
> 
> 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 without preventing
> core-retention.
> 
> Depends on http://article.gmane.org/gmane.linux.ports.arm.omap/126670.
> 
> Changelog:
> v3:
> - used scm_conf node in dra7.dtsi for control register access.
> 
> v2:
> - Fixed pcie disable_clocks. Addressed review comments.
> 
> cheers,
> -roger
> 
> Roger Quadros (3):
>   phy: ti-pipe3: i783 workaround for SATA lockup after dpll
>     unlock/relock
>   ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY
>   ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes
> 
>  Documentation/devicetree/bindings/phy/ti-phy.txt | 16 +++++++
>  arch/arm/boot/dts/dra7.dtsi                      | 20 ++++----
>  drivers/phy/phy-ti-pipe3.c                       | 61 +++++++++++++++++++++---
>  3 files changed, 80 insertions(+), 17 deletions(-)
> 

Re: [PATCH v3 0/3] phy: ti-pipe3: dra7: sata: allow suspend to RAM (core-retention)

[Roger Quadros]

On 27/07/15 13:21, Roger Quadros wrote:
> Kishon & Tony,
> 
> You can please pick the first 2 patches for -next.

Sorry, I meant for -rc.

> 
> The 3rd patch is not required for SATA and I will send it as
> a separate series.

This can go in -next.

cheers,
-roger

> 
> cheers,
> -roger
> 
> On 17/07/15 16:47, Roger Quadros wrote:
>> Hi,
>>
>> 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 without preventing
>> core-retention.
>>
>> Depends on http://article.gmane.org/gmane.linux.ports.arm.omap/126670.
>>
>> Changelog:
>> v3:
>> - used scm_conf node in dra7.dtsi for control register access.
>>
>> v2:
>> - Fixed pcie disable_clocks. Addressed review comments.
>>
>> cheers,
>> -roger
>>
>> Roger Quadros (3):
>>   phy: ti-pipe3: i783 workaround for SATA lockup after dpll
>>     unlock/relock
>>   ARM: dts: dra7: Add syscon-pllreset syscon to SATA PHY
>>   ARM: dts: dra7: Add scm_conf1 node and remove redundant nodes
>>
>>  Documentation/devicetree/bindings/phy/ti-phy.txt | 16 +++++++
>>  arch/arm/boot/dts/dra7.dtsi                      | 20 ++++----
>>  drivers/phy/phy-ti-pipe3.c                       | 61 +++++++++++++++++++++---
>>  3 files changed, 80 insertions(+), 17 deletions(-)
>>