| Message ID | 35e8fb4a-4b43-dac9-7a6f-cc7083c06453@cogentembedded.com |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add R8A77980 PCIe support & some driver cleanups | expand |
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 >
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. ...
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 > > >
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
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
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(+)