diff mbox series

[2/2] pcie: pcie_advk: move setting of reference clock to the pcie driver

Message ID 20200831032538.467336-2-a.heider@gmail.com
State Superseded
Delegated to: Stefan Roese
Headers show
Series [1/2] xenon_sdhci: support for HS200 mode | expand

Commit Message

Andre Heider Aug. 31, 2020, 3:25 a.m. UTC 
From: Grzegorz Jaszczyk <jaz@semihalf.com>

The settings of reference clock is done via pcie address space and not
comphy address space - move the settings in appropriate place.

This aligns hw initialization of pcie advk with Linux and also will allow
to get rid of not comphy related operation from the comphy driver.

Change-Id: I9cc2e8f3e415a880dfb01d10cc8db73b7e81a605
Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Reviewed-on: http://vgitil04.il.marvell.com:8080/59619
Reviewed-by: Igal Liberman <igall@marvell.com>
Tested-by: iSoC Platform CI <ykjenk@marvell.com>
[a.heider: adapt to mainline]
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
Missing downstream patch, noticed while diffing branches:
https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/e3d9c015d434f755578a86a4b6b3acd17d69238a

 drivers/pci/pci-aardvark.c         | 8 ++++++++
 drivers/phy/marvell/comphy_a3700.c | 4 ++--
 drivers/phy/marvell/comphy_a3700.h | 5 -----
 3 files changed, 10 insertions(+), 7 deletions(-)

Comments

Pali Rohár Aug. 31, 2020, 7:52 a.m. UTC | #1 
On Monday 31 August 2020 05:25:38 Andre Heider wrote:
> From: Grzegorz Jaszczyk <jaz@semihalf.com>
> 
> The settings of reference clock is done via pcie address space and not
> comphy address space - move the settings in appropriate place.
> 
> This aligns hw initialization of pcie advk with Linux and also will allow
> to get rid of not comphy related operation from the comphy driver.
> 
> Change-Id: I9cc2e8f3e415a880dfb01d10cc8db73b7e81a605
> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> Reviewed-on: http://vgitil04.il.marvell.com:8080/59619
> Reviewed-by: Igal Liberman <igall@marvell.com>
> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> [a.heider: adapt to mainline]
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> Missing downstream patch, noticed while diffing branches:
> https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/e3d9c015d434f755578a86a4b6b3acd17d69238a

It looks like this patch only moves initialization code from one file to
another. It is fixing some issue? I have tested PCIe on both MOX and
Espressobin and it worked fine. If there is no issue I would rather wait
for Marek comphy patches which should completely remove current marvell
comphy driver from u-boot.

>  drivers/pci/pci-aardvark.c         | 8 ++++++++
>  drivers/phy/marvell/comphy_a3700.c | 4 ++--
>  drivers/phy/marvell/comphy_a3700.h | 5 -----
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index b2c417701f..8d1748ec0a 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -95,6 +95,11 @@
>  #define     PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE	BIT(5)
>  #define     PCIE_CORE_CTRL2_ADDRWIN_MAP_ENABLE	BIT(6)
>  
> +#define PCIE_PHY_REF_CLOCK			(CONTROL_BASE_ADDR + 0x14)
> +#define     PCIE_PHY_CTRL_OFF			16
> +#define     PCIE_PHY_BUF_CTRL_OFF		0
> +#define     PCIE_PHY_BUF_CTRL_INIT_VAL		0x1342
> +
>  /* LMI registers base address and register offsets */
>  #define LMI_BASE_ADDR				0x6000
>  #define CFG_REG					(LMI_BASE_ADDR + 0x0)
> @@ -515,6 +520,9 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>  {
>  	u32 reg;
>  
> +	/* Set HW Reference Clock Buffer Control */
> +	advk_writel(pcie, PCIE_PHY_BUF_CTRL_INIT_VAL, PCIE_PHY_REF_CLOCK);
> +
>  	/* Set to Direct mode */
>  	reg = advk_readl(pcie, CTRL_CONFIG_REG);
>  	reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 4606de6f48..d460dc345a 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -202,9 +202,9 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
>  		  0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
>  
>  	/*
> -	 * 7. Enable TX
> +	 * 7. Enable TX, PCIE global register, 0xd0074814, it is done in
> +	 * PCI-E driver
>  	 */
> -	reg_set(PCIE_REF_CLK_ADDR, 0x1342, 0xFFFFFFFF);
>  
>  	/*
>  	 * 8. Check crystal jumper setting and program the Power and PLL
> diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
> index b0941ffb37..db69393c1b 100644
> --- a/drivers/phy/marvell/comphy_a3700.h
> +++ b/drivers/phy/marvell/comphy_a3700.h
> @@ -45,9 +45,6 @@
>  /*
>   * PCIe/USB/SGMII definitions
>   */
> -#define PCIE_BASE			MVEBU_REG(0x070000)
> -#define PCIETOP_BASE			MVEBU_REG(0x080000)
> -#define PCIE_RAMBASE			MVEBU_REG(0x08C000)
>  #define PCIEPHY_BASE			MVEBU_REG(0x01F000)
>  #define PCIEPHY_SHFT			2
>  
> @@ -166,8 +163,6 @@ static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
>  
>  #define PWR_MGM_TIM1			0x1d0
>  
> -#define PCIE_REF_CLK_ADDR		(PCIE_BASE + 0x4814)
> -
>  #define USB3_CTRPUL_VAL_REG		(0x20 + USB32_BASE)
>  #define USB3H_CTRPUL_VAL_REG		(0x3454 + USB32H_BASE)
>  #define rb_usb3_ctr_100ns		0xff000000
> -- 
> 2.28.0
>
Andre Heider Aug. 31, 2020, 8:10 a.m. UTC | #2 
On 31/08/2020 09:52, Pali Rohár wrote:
> On Monday 31 August 2020 05:25:38 Andre Heider wrote:
>> From: Grzegorz Jaszczyk <jaz@semihalf.com>
>>
>> The settings of reference clock is done via pcie address space and not
>> comphy address space - move the settings in appropriate place.
>>
>> This aligns hw initialization of pcie advk with Linux and also will allow
>> to get rid of not comphy related operation from the comphy driver.
>>
>> Change-Id: I9cc2e8f3e415a880dfb01d10cc8db73b7e81a605
>> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/59619
>> Reviewed-by: Igal Liberman <igall@marvell.com>
>> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
>> [a.heider: adapt to mainline]
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>> Missing downstream patch, noticed while diffing branches:
>> https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/e3d9c015d434f755578a86a4b6b3acd17d69238a
> 
> It looks like this patch only moves initialization code from one file to
> another. It is fixing some issue? I have tested PCIe on both MOX and
> Espressobin and it worked fine. If there is no issue I would rather wait
> for Marek comphy patches which should completely remove current marvell
> comphy driver from u-boot.

As with the other patch, this reduces the diff against the downstream fork.

PCIe works fine here too, this is just a preparation step for removing 
the comphy driver (which downstream has done too).

Thanks,
Andre

>>   drivers/pci/pci-aardvark.c         | 8 ++++++++
>>   drivers/phy/marvell/comphy_a3700.c | 4 ++--
>>   drivers/phy/marvell/comphy_a3700.h | 5 -----
>>   3 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
>> index b2c417701f..8d1748ec0a 100644
>> --- a/drivers/pci/pci-aardvark.c
>> +++ b/drivers/pci/pci-aardvark.c
>> @@ -95,6 +95,11 @@
>>   #define     PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE	BIT(5)
>>   #define     PCIE_CORE_CTRL2_ADDRWIN_MAP_ENABLE	BIT(6)
>>   
>> +#define PCIE_PHY_REF_CLOCK			(CONTROL_BASE_ADDR + 0x14)
>> +#define     PCIE_PHY_CTRL_OFF			16
>> +#define     PCIE_PHY_BUF_CTRL_OFF		0
>> +#define     PCIE_PHY_BUF_CTRL_INIT_VAL		0x1342
>> +
>>   /* LMI registers base address and register offsets */
>>   #define LMI_BASE_ADDR				0x6000
>>   #define CFG_REG					(LMI_BASE_ADDR + 0x0)
>> @@ -515,6 +520,9 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>>   {
>>   	u32 reg;
>>   
>> +	/* Set HW Reference Clock Buffer Control */
>> +	advk_writel(pcie, PCIE_PHY_BUF_CTRL_INIT_VAL, PCIE_PHY_REF_CLOCK);
>> +
>>   	/* Set to Direct mode */
>>   	reg = advk_readl(pcie, CTRL_CONFIG_REG);
>>   	reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
>> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
>> index 4606de6f48..d460dc345a 100644
>> --- a/drivers/phy/marvell/comphy_a3700.c
>> +++ b/drivers/phy/marvell/comphy_a3700.c
>> @@ -202,9 +202,9 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
>>   		  0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
>>   
>>   	/*
>> -	 * 7. Enable TX
>> +	 * 7. Enable TX, PCIE global register, 0xd0074814, it is done in
>> +	 * PCI-E driver
>>   	 */
>> -	reg_set(PCIE_REF_CLK_ADDR, 0x1342, 0xFFFFFFFF);
>>   
>>   	/*
>>   	 * 8. Check crystal jumper setting and program the Power and PLL
>> diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
>> index b0941ffb37..db69393c1b 100644
>> --- a/drivers/phy/marvell/comphy_a3700.h
>> +++ b/drivers/phy/marvell/comphy_a3700.h
>> @@ -45,9 +45,6 @@
>>   /*
>>    * PCIe/USB/SGMII definitions
>>    */
>> -#define PCIE_BASE			MVEBU_REG(0x070000)
>> -#define PCIETOP_BASE			MVEBU_REG(0x080000)
>> -#define PCIE_RAMBASE			MVEBU_REG(0x08C000)
>>   #define PCIEPHY_BASE			MVEBU_REG(0x01F000)
>>   #define PCIEPHY_SHFT			2
>>   
>> @@ -166,8 +163,6 @@ static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
>>   
>>   #define PWR_MGM_TIM1			0x1d0
>>   
>> -#define PCIE_REF_CLK_ADDR		(PCIE_BASE + 0x4814)
>> -
>>   #define USB3_CTRPUL_VAL_REG		(0x20 + USB32_BASE)
>>   #define USB3H_CTRPUL_VAL_REG		(0x3454 + USB32H_BASE)
>>   #define rb_usb3_ctr_100ns		0xff000000
>> -- 
>> 2.28.0
>>
Andre Heider Sept. 2, 2020, 7:11 a.m. UTC | #3 
On 31/08/2020 09:52, Pali Rohár wrote:
> On Monday 31 August 2020 05:25:38 Andre Heider wrote:
>> From: Grzegorz Jaszczyk <jaz@semihalf.com>
>>
>> The settings of reference clock is done via pcie address space and not
>> comphy address space - move the settings in appropriate place.
>>
>> This aligns hw initialization of pcie advk with Linux and also will allow
>> to get rid of not comphy related operation from the comphy driver.
>>
>> Change-Id: I9cc2e8f3e415a880dfb01d10cc8db73b7e81a605
>> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/59619
>> Reviewed-by: Igal Liberman <igall@marvell.com>
>> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
>> [a.heider: adapt to mainline]
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>> Missing downstream patch, noticed while diffing branches:
>> https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/e3d9c015d434f755578a86a4b6b3acd17d69238a
> 
> It looks like this patch only moves initialization code from one file to
> another. It is fixing some issue? I have tested PCIe on both MOX and
> Espressobin and it worked fine. If there is no issue I would rather wait
> for Marek comphy patches which should completely remove current marvell
> comphy driver from u-boot.

I just found out that Marek's patches cover this, so this patch can be 
dropped:
https://patchwork.ozlabs.org/project/uboot/patch/20200419154850.25868-8-marek.behun@nic.cz/

Thanks,
Andre
Marek Behún Sept. 2, 2020, 12:35 p.m. UTC | #4 
On Wed, 2 Sep 2020 09:11:52 +0200
Andre Heider <a.heider@gmail.com> wrote:

> On 31/08/2020 09:52, Pali Rohár wrote:
> > On Monday 31 August 2020 05:25:38 Andre Heider wrote:  
> >> From: Grzegorz Jaszczyk <jaz@semihalf.com>
> >>
> >> The settings of reference clock is done via pcie address space and
> >> not comphy address space - move the settings in appropriate place.
> >>
> >> This aligns hw initialization of pcie advk with Linux and also
> >> will allow to get rid of not comphy related operation from the
> >> comphy driver.
> >>
> >> Change-Id: I9cc2e8f3e415a880dfb01d10cc8db73b7e81a605
> >> Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
> >> Reviewed-on: http://vgitil04.il.marvell.com:8080/59619
> >> Reviewed-by: Igal Liberman <igall@marvell.com>
> >> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> >> [a.heider: adapt to mainline]
> >> Signed-off-by: Andre Heider <a.heider@gmail.com>
> >> ---
> >> Missing downstream patch, noticed while diffing branches:
> >> https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/commit/e3d9c015d434f755578a86a4b6b3acd17d69238a
> >>  
> > 
> > It looks like this patch only moves initialization code from one
> > file to another. It is fixing some issue? I have tested PCIe on
> > both MOX and Espressobin and it worked fine. If there is no issue I
> > would rather wait for Marek comphy patches which should completely
> > remove current marvell comphy driver from u-boot.  
> 
> I just found out that Marek's patches cover this, so this patch can
> be dropped:
> https://patchwork.ozlabs.org/project/uboot/patch/20200419154850.25868-8-marek.behun@nic.cz/
> 
> Thanks,
> Andre

Hi,
that patch series wasn't accepted though, it needs some work. I will
try to this ASAP.
Marek
diff mbox series

Patch

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index b2c417701f..8d1748ec0a 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -95,6 +95,11 @@ 
 #define     PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE	BIT(5)
 #define     PCIE_CORE_CTRL2_ADDRWIN_MAP_ENABLE	BIT(6)
 
+#define PCIE_PHY_REF_CLOCK			(CONTROL_BASE_ADDR + 0x14)
+#define     PCIE_PHY_CTRL_OFF			16
+#define     PCIE_PHY_BUF_CTRL_OFF		0
+#define     PCIE_PHY_BUF_CTRL_INIT_VAL		0x1342
+
 /* LMI registers base address and register offsets */
 #define LMI_BASE_ADDR				0x6000
 #define CFG_REG					(LMI_BASE_ADDR + 0x0)
@@ -515,6 +520,9 @@  static int pcie_advk_setup_hw(struct pcie_advk *pcie)
 {
 	u32 reg;
 
+	/* Set HW Reference Clock Buffer Control */
+	advk_writel(pcie, PCIE_PHY_BUF_CTRL_INIT_VAL, PCIE_PHY_REF_CLOCK);
+
 	/* Set to Direct mode */
 	reg = advk_readl(pcie, CTRL_CONFIG_REG);
 	reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
index 4606de6f48..d460dc345a 100644
--- a/drivers/phy/marvell/comphy_a3700.c
+++ b/drivers/phy/marvell/comphy_a3700.c
@@ -202,9 +202,9 @@  static int comphy_pcie_power_up(u32 speed, u32 invert)
 		  0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
 
 	/*
-	 * 7. Enable TX
+	 * 7. Enable TX, PCIE global register, 0xd0074814, it is done in
+	 * PCI-E driver
 	 */
-	reg_set(PCIE_REF_CLK_ADDR, 0x1342, 0xFFFFFFFF);
 
 	/*
 	 * 8. Check crystal jumper setting and program the Power and PLL
diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
index b0941ffb37..db69393c1b 100644
--- a/drivers/phy/marvell/comphy_a3700.h
+++ b/drivers/phy/marvell/comphy_a3700.h
@@ -45,9 +45,6 @@ 
 /*
  * PCIe/USB/SGMII definitions
  */
-#define PCIE_BASE			MVEBU_REG(0x070000)
-#define PCIETOP_BASE			MVEBU_REG(0x080000)
-#define PCIE_RAMBASE			MVEBU_REG(0x08C000)
 #define PCIEPHY_BASE			MVEBU_REG(0x01F000)
 #define PCIEPHY_SHFT			2
 
@@ -166,8 +163,6 @@  static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
 
 #define PWR_MGM_TIM1			0x1d0
 
-#define PCIE_REF_CLK_ADDR		(PCIE_BASE + 0x4814)
-
 #define USB3_CTRPUL_VAL_REG		(0x20 + USB32_BASE)
 #define USB3H_CTRPUL_VAL_REG		(0x3454 + USB32H_BASE)
 #define rb_usb3_ctr_100ns		0xff000000