[1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()

Message ID 35e8fb4a-4b43-dac9-7a6f-cc7083c06453@cogentembedded.com
State New
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • Add R8A77980 PCIe support & some driver cleanups
Related show

Commit Message

Sergei Shtylyov April 6, 2018, 11:02 a.m.
In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
the driver only does this on R-Car H1 (polling a PHY specific register).
Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
special PHY driver on the R-Car V3H the PCIEC initialization just freezes
the kernel --  adding the PHYRDY polling allows the init code to exit
gracefully on timeout (PHY starts powered down after reset on this SoC).

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/pci/host/pcie-rcar.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Simon Horman April 9, 2018, 10:54 a.m. | #1
On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> the driver only does this on R-Car H1 (polling a PHY specific register).

Is the R-Car H1 specific code still needed with this patch in place?

If so can we consider a helper. rcar_pcie_wait_for_phyrdy() looks
very similar to that R-Car H1 code.

> Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
> special PHY driver on the R-Car V3H the PCIEC initialization just freezes
> the kernel --  adding the PHYRDY polling allows the init code to exit
> gracefully on timeout (PHY starts powered down after reset on this SoC).

How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.

> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  drivers/pci/host/pcie-rcar.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> Index: pci/drivers/pci/host/pcie-rcar.c
> ===================================================================
> --- pci.orig/drivers/pci/host/pcie-rcar.c
> +++ pci/drivers/pci/host/pcie-rcar.c
> @@ -36,6 +36,8 @@
>  #define PCIECDR			0x000020
>  #define PCIEMSR			0x000028
>  #define PCIEINTXR		0x000400
> +#define PCIEPHYSR		0x0007f0
> +#define  PHYRDY			1

Can we start using the BIT() macro in this driver?

>  #define PCIEMSITXR		0x000840
>  
>  /* Transfer control */
> @@ -527,6 +529,20 @@ static void phy_write_reg(struct rcar_pc
>  	phy_wait_for_ack(pcie);
>  }
>  
> +static int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
> +{
> +	unsigned int timeout = 10;
> +
> +	while (timeout--) {
> +		if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
> +			return 0;
> +
> +		msleep(5);
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>  {
>  	unsigned int timeout = 10;
> @@ -551,6 +567,10 @@ static int rcar_pcie_hw_init(struct rcar
>  	/* Set mode */
>  	rcar_pci_write_reg(pcie, 1, PCIEMSR);
>  
> +	err = rcar_pcie_wait_for_phyrdy(pcie);
> +	if (err)
> +		return err;
> +
>  	/*
>  	 * Initial header for port config space is type 1, set the device
>  	 * class to match. Hardware takes care of propagating the IDSETR
>
Simon Horman April 9, 2018, 10:55 a.m. | #2
On Mon, Apr 09, 2018 at 12:54:18PM +0200, Simon Horman wrote:
> On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> > In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> > for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> > the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?

Sorry, I now see you addressed that by removing the R-Car H1 code in patch 2/4.

...
Simon Horman April 9, 2018, 11:24 a.m. | #3
On Mon, Apr 09, 2018 at 12:54:18PM +0200, Simon Horman wrote:
> On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> > In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> > for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> > the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?
> 
> If so can we consider a helper. rcar_pcie_wait_for_phyrdy() looks
> very similar to that R-Car H1 code.
> 
> > Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
> > special PHY driver on the R-Car V3H the PCIEC initialization just freezes
> > the kernel --  adding the PHYRDY polling allows the init code to exit
> > gracefully on timeout (PHY starts powered down after reset on this SoC).
> 
> How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.
> 
> > 
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> > 
> > ---
> >  drivers/pci/host/pcie-rcar.c |   20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > Index: pci/drivers/pci/host/pcie-rcar.c
> > ===================================================================
> > --- pci.orig/drivers/pci/host/pcie-rcar.c
> > +++ pci/drivers/pci/host/pcie-rcar.c
> > @@ -36,6 +36,8 @@
> >  #define PCIECDR			0x000020
> >  #define PCIEMSR			0x000028
> >  #define PCIEINTXR		0x000400
> > +#define PCIEPHYSR		0x0007f0
> > +#define  PHYRDY			1
> 
> Can we start using the BIT() macro in this driver?

I see this is handled by

[PATCH V3] PCI: rcar: Clean up the macros

> 
> >  #define PCIEMSITXR		0x000840
> >  
> >  /* Transfer control */
> > @@ -527,6 +529,20 @@ static void phy_write_reg(struct rcar_pc
> >  	phy_wait_for_ack(pcie);
> >  }
> >  
> > +static int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
> > +{
> > +	unsigned int timeout = 10;
> > +
> > +	while (timeout--) {
> > +		if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
> > +			return 0;
> > +
> > +		msleep(5);
> > +	}
> > +
> > +	return -ETIMEDOUT;
> > +}
> > +
> >  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
> >  {
> >  	unsigned int timeout = 10;
> > @@ -551,6 +567,10 @@ static int rcar_pcie_hw_init(struct rcar
> >  	/* Set mode */
> >  	rcar_pci_write_reg(pcie, 1, PCIEMSR);
> >  
> > +	err = rcar_pcie_wait_for_phyrdy(pcie);
> > +	if (err)
> > +		return err;
> > +
> >  	/*
> >  	 * Initial header for port config space is type 1, set the device
> >  	 * class to match. Hardware takes care of propagating the IDSETR
> > 
>
Sergei Shtylyov April 9, 2018, 3:45 p.m. | #4
On 04/09/2018 01:54 PM, Simon Horman wrote:

>> In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
>> for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
>> the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?

   No, it's removed in the next patch.

[...]
>> Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
>> special PHY driver on the R-Car V3H the PCIEC initialization just freezes
>> the kernel --  adding the PHYRDY polling allows the init code to exit
>> gracefully on timeout (PHY starts powered down after reset on this SoC).
> 
> How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.

   Tested on R8A7791/Porter (unfortunately, I have no spare PCIe card) and
R8A77980/Condor (tried couple PCIe cards).

>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>>  drivers/pci/host/pcie-rcar.c |   20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> Index: pci/drivers/pci/host/pcie-rcar.c
>> ===================================================================
>> --- pci.orig/drivers/pci/host/pcie-rcar.c
>> +++ pci/drivers/pci/host/pcie-rcar.c
>> @@ -36,6 +36,8 @@
>>  #define PCIECDR			0x000020
>>  #define PCIEMSR			0x000028
>>  #define PCIEINTXR		0x000400
>> +#define PCIEPHYSR		0x0007f0
>> +#define  PHYRDY			1
> 
> Can we start using the BIT() macro in this driver?

   Done by Marek for the other regs... not sure whose patch would hit the kernel
first tho...

[...]

MBR, Sergei

Patch

Index: pci/drivers/pci/host/pcie-rcar.c
===================================================================
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c
@@ -36,6 +36,8 @@ 
 #define PCIECDR			0x000020
 #define PCIEMSR			0x000028
 #define PCIEINTXR		0x000400
+#define PCIEPHYSR		0x0007f0
+#define  PHYRDY			1
 #define PCIEMSITXR		0x000840
 
 /* Transfer control */
@@ -527,6 +529,20 @@  static void phy_write_reg(struct rcar_pc
 	phy_wait_for_ack(pcie);
 }
 
+static int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
+{
+	unsigned int timeout = 10;
+
+	while (timeout--) {
+		if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
+			return 0;
+
+		msleep(5);
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
 {
 	unsigned int timeout = 10;
@@ -551,6 +567,10 @@  static int rcar_pcie_hw_init(struct rcar
 	/* Set mode */
 	rcar_pci_write_reg(pcie, 1, PCIEMSR);
 
+	err = rcar_pcie_wait_for_phyrdy(pcie);
+	if (err)
+		return err;
+
 	/*
 	 * Initial header for port config space is type 1, set the device
 	 * class to match. Hardware takes care of propagating the IDSETR