* [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups
@ 2018-04-06 10:58 Sergei Shtylyov
2018-04-06 11:02 ` [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init() Sergei Shtylyov
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 10:58 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
Hello!
Here's a set of 4 patches against the 'pci.rcar' branch of Lorenzo Pieralisi's
'pci.git' repo. These are the changes needed for better R-Car gen3 support
(namely for R8A77980 support) plus some PCIe driver re-factoring done in
the process...
[1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
[2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
[3/4] pcie-rcar: add R-Car gen3 PHY support
[4/4] pcie-rcar: factor out rcar_pcie_hw_init() call
MBR, Sergei
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
@ 2018-04-06 11:02 ` Sergei Shtylyov
2018-04-09 10:54 ` Simon Horman
2018-04-06 11:04 ` [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1() Sergei Shtylyov
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 11:02 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
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(+)
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
2018-04-06 11:02 ` [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init() Sergei Shtylyov
@ 2018-04-06 11:04 ` Sergei Shtylyov
2018-04-09 10:56 ` Simon Horman
2018-04-06 11:08 ` [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support Sergei Shtylyov
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 11:04 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
Now that we've added PCIEPHYSR.PHYRDY polling to rcar_pcie_hw_init(),
there is no need anymore for polling the PHY specific register in
rcar_pcie_hw_init_h1() -- remove it.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/pci/host/pcie-rcar.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
Index: pci/drivers/pci/host/pcie-rcar.c
===================================================================
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c
@@ -102,7 +102,6 @@
#define LANE_POS 8
#define ADR_POS 0
#define H1_PCIEPHYDOUTR 0x040014
-#define H1_PCIEPHYSR 0x040018
/* R-Car Gen2 PHY */
#define GEN2_PCIEPHYADDR 0x780
@@ -627,8 +626,6 @@ static int rcar_pcie_hw_init(struct rcar
static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
{
- unsigned int timeout = 10;
-
/* Initialize the phy */
phy_write_reg(pcie, 0, 0x42, 0x1, 0x0EC34191);
phy_write_reg(pcie, 1, 0x42, 0x1, 0x0EC34180);
@@ -647,14 +644,7 @@ static int rcar_pcie_hw_init_h1(struct r
phy_write_reg(pcie, 0, 0x64, 0x1, 0x3F0F1F0F);
phy_write_reg(pcie, 0, 0x66, 0x1, 0x00008000);
- while (timeout--) {
- if (rcar_pci_read_reg(pcie, H1_PCIEPHYSR))
- return rcar_pcie_hw_init(pcie);
-
- msleep(5);
- }
-
- return -ETIMEDOUT;
+ return rcar_pcie_hw_init(pcie);
}
static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
2018-04-06 11:02 ` [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init() Sergei Shtylyov
2018-04-06 11:04 ` [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1() Sergei Shtylyov
@ 2018-04-06 11:08 ` Sergei Shtylyov
2018-04-07 18:35 ` Sergei Shtylyov
2018-04-09 11:00 ` Simon Horman
2018-04-06 11:10 ` [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call Sergei Shtylyov
2018-04-06 11:24 ` [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
4 siblings, 2 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 11:08 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On R-Car gen3 SoCs the PCIe PHY has its own register region -- and I have
written a generic PHY driver for it, thus we need to add the corresponding
code in rcar_pcie_hw_init_gen3() and call devm_phy_optional_get() at the
driver's probing time, so that the existing R-Car gen3 device trees (not
having a PHY node) would still work (we only need to power up the PHY on
R-Car V3H).
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/pci/host/pcie-rcar.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
Index: pci/drivers/pci/host/pcie-rcar.c
===================================================================
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c
@@ -24,6 +24,7 @@
#include <linux/of_pci.h>
#include <linux/of_platform.h>
#include <linux/pci.h>
+#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
@@ -140,6 +141,7 @@ static inline struct rcar_msi *to_rcar_m
/* Structure representing the PCIe interface */
struct rcar_pcie {
struct device *dev;
+ struct phy *phy;
void __iomem *base;
struct list_head resources;
int root_bus_nr;
@@ -667,6 +669,21 @@ static int rcar_pcie_hw_init_gen2(struct
return rcar_pcie_hw_init(pcie);
}
+static int rcar_pcie_hw_init_gen3(struct rcar_pcie *pcie)
+{
+ int err;
+
+ err = phy_init(pcie->phy);
+ if (err)
+ return err;
+
+ err = phy_power_on(pcie->phy);
+ if (err)
+ return err;
+
+ return rcar_pcie_hw_init(pcie);
+}
+
static int rcar_msi_alloc(struct rcar_msi *chip)
{
int msi;
@@ -916,6 +933,10 @@ static int rcar_pcie_get_resources(struc
struct resource res;
int err, i;
+ pcie->phy = devm_phy_optional_get(dev, "pcie");
+ if (IS_ERR(pcie->phy))
+ return PTR_ERR(pcie->phy);
+
err = of_address_to_resource(dev->of_node, 0, &res);
if (err)
return err;
@@ -1068,8 +1089,10 @@ static const struct of_device_id rcar_pc
.data = rcar_pcie_hw_init_gen2 },
{ .compatible = "renesas,pcie-rcar-gen2",
.data = rcar_pcie_hw_init_gen2 },
- { .compatible = "renesas,pcie-r8a7795", .data = rcar_pcie_hw_init },
- { .compatible = "renesas,pcie-rcar-gen3", .data = rcar_pcie_hw_init },
+ { .compatible = "renesas,pcie-r8a7795",
+ .data = rcar_pcie_hw_init_gen3 },
+ { .compatible = "renesas,pcie-rcar-gen3",
+ .data = rcar_pcie_hw_init_gen3 },
{},
};
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
` (2 preceding siblings ...)
2018-04-06 11:08 ` [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support Sergei Shtylyov
@ 2018-04-06 11:10 ` Sergei Shtylyov
2018-04-09 8:34 ` Geert Uytterhoeven
2018-04-09 11:04 ` Simon Horman
2018-04-06 11:24 ` [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
4 siblings, 2 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 11:10 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
makes sense to move that call into the driver's probe() method and then
rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/pci/host/pcie-rcar.c | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
Index: pci/drivers/pci/host/pcie-rcar.c
===================================================================
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c
@@ -626,7 +626,7 @@ static int rcar_pcie_hw_init(struct rcar
return 0;
}
-static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
+static int rcar_pcie_phy_init_h1(struct rcar_pcie *pcie)
{
/* Initialize the phy */
phy_write_reg(pcie, 0, 0x42, 0x1, 0x0EC34191);
@@ -646,10 +646,10 @@ static int rcar_pcie_hw_init_h1(struct r
phy_write_reg(pcie, 0, 0x64, 0x1, 0x3F0F1F0F);
phy_write_reg(pcie, 0, 0x66, 0x1, 0x00008000);
- return rcar_pcie_hw_init(pcie);
+ return 0;
}
-static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
+static int rcar_pcie_phy_init_gen2(struct rcar_pcie *pcie)
{
/*
* These settings come from the R-Car Series, 2nd Generation User's
@@ -666,10 +666,10 @@ static int rcar_pcie_hw_init_gen2(struct
rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
- return rcar_pcie_hw_init(pcie);
+ return 0;
}
-static int rcar_pcie_hw_init_gen3(struct rcar_pcie *pcie)
+static int rcar_pcie_phy_init_gen3(struct rcar_pcie *pcie)
{
int err;
@@ -677,11 +677,7 @@ static int rcar_pcie_hw_init_gen3(struct
if (err)
return err;
- err = phy_power_on(pcie->phy);
- if (err)
- return err;
-
- return rcar_pcie_hw_init(pcie);
+ return phy_power_on(pcie->phy);
}
static int rcar_msi_alloc(struct rcar_msi *chip)
@@ -1082,17 +1078,18 @@ static int rcar_pcie_parse_map_dma_range
}
static const struct of_device_id rcar_pcie_of_match[] = {
- { .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
+ { .compatible = "renesas,pcie-r8a7779",
+ .data = rcar_pcie_phy_init_h1 },
{ .compatible = "renesas,pcie-r8a7790",
- .data = rcar_pcie_hw_init_gen2 },
+ .data = rcar_pcie_phy_init_gen2 },
{ .compatible = "renesas,pcie-r8a7791",
- .data = rcar_pcie_hw_init_gen2 },
+ .data = rcar_pcie_phy_init_gen2 },
{ .compatible = "renesas,pcie-rcar-gen2",
- .data = rcar_pcie_hw_init_gen2 },
+ .data = rcar_pcie_phy_init_gen2 },
{ .compatible = "renesas,pcie-r8a7795",
- .data = rcar_pcie_hw_init_gen3 },
+ .data = rcar_pcie_phy_init_gen3 },
{ .compatible = "renesas,pcie-rcar-gen3",
- .data = rcar_pcie_hw_init_gen3 },
+ .data = rcar_pcie_phy_init_gen3 },
{},
};
@@ -1140,7 +1137,7 @@ static int rcar_pcie_probe(struct platfo
struct rcar_pcie *pcie;
unsigned int data;
int err;
- int (*hw_init_fn)(struct rcar_pcie *);
+ int (*phy_init_fn)(struct rcar_pcie *);
struct pci_host_bridge *bridge;
bridge = pci_alloc_host_bridge(sizeof(*pcie));
@@ -1174,10 +1171,15 @@ static int rcar_pcie_probe(struct platfo
goto err_pm_disable;
}
- /* Failure to get a link might just be that no cards are inserted */
- hw_init_fn = of_device_get_match_data(dev);
- err = hw_init_fn(pcie);
+ phy_init_fn = of_device_get_match_data(dev);
+ err = phy_init_fn(pcie);
if (err) {
+ dev_err(dev, "failed to init PCIe PHY\n");
+ goto err_pm_put;
+ }
+
+ /* Failure to get a link might just be that no cards are inserted */
+ if (rcar_pcie_hw_init(pcie)) {
dev_info(dev, "PCIe link down\n");
err = -ENODEV;
goto err_pm_put;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
` (3 preceding siblings ...)
2018-04-06 11:10 ` [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call Sergei Shtylyov
@ 2018-04-06 11:24 ` Sergei Shtylyov
4 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-06 11:24 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On 04/06/2018 01:58 PM, Sergei Shtylyov wrote:
> Hello!
>
> Here's a set of 4 patches against the 'pci.rcar' branch of Lorenzo Pieralisi's
I meant to type 'pci/rcar'. :-)
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support
2018-04-06 11:08 ` [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support Sergei Shtylyov
@ 2018-04-07 18:35 ` Sergei Shtylyov
2018-04-09 11:00 ` Simon Horman
1 sibling, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-07 18:35 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
Hello!
On 04/06/2018 02:08 PM, Sergei Shtylyov wrote:
> On R-Car gen3 SoCs the PCIe PHY has its own register region -- and I have
> written a generic PHY driver for it, thus we need to add the corresponding
> code in rcar_pcie_hw_init_gen3() and call devm_phy_optional_get() at the
> driver's probing time, so that the existing R-Car gen3 device trees (not
> having a PHY node) would still work (we only need to power up the PHY on
> R-Car V3H).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> drivers/pci/host/pcie-rcar.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> Index: pci/drivers/pci/host/pcie-rcar.c
> ===================================================================
> --- pci.orig/drivers/pci/host/pcie-rcar.c
> +++ pci/drivers/pci/host/pcie-rcar.c
[...]
> @@ -916,6 +933,10 @@ static int rcar_pcie_get_resources(struc
> struct resource res;
> int err, i;
>
> + pcie->phy = devm_phy_optional_get(dev, "pcie");
> + if (IS_ERR(pcie->phy))
> + return PTR_ERR(pcie->phy);
> +
Forgot to update the bindings. Sigh... :-(
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call
2018-04-06 11:10 ` [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call Sergei Shtylyov
@ 2018-04-09 8:34 ` Geert Uytterhoeven
2018-04-09 11:04 ` Simon Horman
1 sibling, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2018-04-09 8:34 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Simon Horman, Bjorn Helgaas, linux-pci, Linux-Renesas, Lorenzo Pieralisi
On Fri, Apr 6, 2018 at 1:10 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
> makes sense to move that call into the driver's probe() method and then
> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
2018-04-06 11:02 ` [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init() Sergei Shtylyov
@ 2018-04-09 10:54 ` Simon Horman
2018-04-09 10:55 ` Simon Horman
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Simon Horman @ 2018-04-09 10:54 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
2018-04-09 10:54 ` Simon Horman
@ 2018-04-09 10:55 ` Simon Horman
2018-04-09 11:24 ` Simon Horman
2018-04-09 15:45 ` Sergei Shtylyov
2 siblings, 0 replies; 17+ messages in thread
From: Simon Horman @ 2018-04-09 10:55 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
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.
...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
2018-04-06 11:04 ` [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1() Sergei Shtylyov
@ 2018-04-09 10:56 ` Simon Horman
2018-04-09 15:33 ` Sergei Shtylyov
0 siblings, 1 reply; 17+ messages in thread
From: Simon Horman @ 2018-04-09 10:56 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On Fri, Apr 06, 2018 at 02:04:52PM +0300, Sergei Shtylyov wrote:
> Now that we've added PCIEPHYSR.PHYRDY polling to rcar_pcie_hw_init(),
> there is no need anymore for polling the PHY specific register in
> rcar_pcie_hw_init_h1() -- remove it.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
This looks good, but has it been tested?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support
2018-04-06 11:08 ` [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support Sergei Shtylyov
2018-04-07 18:35 ` Sergei Shtylyov
@ 2018-04-09 11:00 ` Simon Horman
1 sibling, 0 replies; 17+ messages in thread
From: Simon Horman @ 2018-04-09 11:00 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On Fri, Apr 06, 2018 at 02:08:12PM +0300, Sergei Shtylyov wrote:
> On R-Car gen3 SoCs the PCIe PHY has its own register region -- and I have
> written a generic PHY driver for it, thus we need to add the corresponding
> code in rcar_pcie_hw_init_gen3() and call devm_phy_optional_get() at the
> driver's probing time, so that the existing R-Car gen3 device trees (not
> having a PHY node) would still work (we only need to power up the PHY on
> R-Car V3H).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call
2018-04-06 11:10 ` [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call Sergei Shtylyov
2018-04-09 8:34 ` Geert Uytterhoeven
@ 2018-04-09 11:04 ` Simon Horman
2018-04-09 15:21 ` Sergei Shtylyov
1 sibling, 1 reply; 17+ messages in thread
From: Simon Horman @ 2018-04-09 11:04 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On Fri, Apr 06, 2018 at 02:10:22PM +0300, Sergei Shtylyov wrote:
> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
> makes sense to move that call into the driver's probe() method and then
> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
I'm not sure the churn is worth it, but if you do then that is find by me.
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> drivers/pci/host/pcie-rcar.c | 42 ++++++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> Index: pci/drivers/pci/host/pcie-rcar.c
> ===================================================================
> --- pci.orig/drivers/pci/host/pcie-rcar.c
> +++ pci/drivers/pci/host/pcie-rcar.c
> @@ -626,7 +626,7 @@ static int rcar_pcie_hw_init(struct rcar
> return 0;
> }
>
> -static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_h1(struct rcar_pcie *pcie)
> {
> /* Initialize the phy */
> phy_write_reg(pcie, 0, 0x42, 0x1, 0x0EC34191);
> @@ -646,10 +646,10 @@ static int rcar_pcie_hw_init_h1(struct r
> phy_write_reg(pcie, 0, 0x64, 0x1, 0x3F0F1F0F);
> phy_write_reg(pcie, 0, 0x66, 0x1, 0x00008000);
>
> - return rcar_pcie_hw_init(pcie);
> + return 0;
> }
>
> -static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_gen2(struct rcar_pcie *pcie)
> {
> /*
> * These settings come from the R-Car Series, 2nd Generation User's
> @@ -666,10 +666,10 @@ static int rcar_pcie_hw_init_gen2(struct
> rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
>
> - return rcar_pcie_hw_init(pcie);
> + return 0;
> }
>
> -static int rcar_pcie_hw_init_gen3(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_gen3(struct rcar_pcie *pcie)
> {
> int err;
>
> @@ -677,11 +677,7 @@ static int rcar_pcie_hw_init_gen3(struct
> if (err)
> return err;
>
> - err = phy_power_on(pcie->phy);
> - if (err)
> - return err;
> -
> - return rcar_pcie_hw_init(pcie);
> + return phy_power_on(pcie->phy);
> }
>
> static int rcar_msi_alloc(struct rcar_msi *chip)
> @@ -1082,17 +1078,18 @@ static int rcar_pcie_parse_map_dma_range
> }
>
> static const struct of_device_id rcar_pcie_of_match[] = {
> - { .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
> + { .compatible = "renesas,pcie-r8a7779",
> + .data = rcar_pcie_phy_init_h1 },
> { .compatible = "renesas,pcie-r8a7790",
> - .data = rcar_pcie_hw_init_gen2 },
> + .data = rcar_pcie_phy_init_gen2 },
> { .compatible = "renesas,pcie-r8a7791",
> - .data = rcar_pcie_hw_init_gen2 },
> + .data = rcar_pcie_phy_init_gen2 },
> { .compatible = "renesas,pcie-rcar-gen2",
> - .data = rcar_pcie_hw_init_gen2 },
> + .data = rcar_pcie_phy_init_gen2 },
> { .compatible = "renesas,pcie-r8a7795",
> - .data = rcar_pcie_hw_init_gen3 },
> + .data = rcar_pcie_phy_init_gen3 },
> { .compatible = "renesas,pcie-rcar-gen3",
> - .data = rcar_pcie_hw_init_gen3 },
> + .data = rcar_pcie_phy_init_gen3 },
> {},
I would avoid the line wrapping here, but its up to you.
> };
>
> @@ -1140,7 +1137,7 @@ static int rcar_pcie_probe(struct platfo
> struct rcar_pcie *pcie;
> unsigned int data;
> int err;
> - int (*hw_init_fn)(struct rcar_pcie *);
> + int (*phy_init_fn)(struct rcar_pcie *);
Looking at this I wonder if we also need a phy_cleanup() code or
similar.
> struct pci_host_bridge *bridge;
>
> bridge = pci_alloc_host_bridge(sizeof(*pcie));
> @@ -1174,10 +1171,15 @@ static int rcar_pcie_probe(struct platfo
> goto err_pm_disable;
> }
>
> - /* Failure to get a link might just be that no cards are inserted */
> - hw_init_fn = of_device_get_match_data(dev);
> - err = hw_init_fn(pcie);
> + phy_init_fn = of_device_get_match_data(dev);
> + err = phy_init_fn(pcie);
> if (err) {
> + dev_err(dev, "failed to init PCIe PHY\n");
> + goto err_pm_put;
> + }
> +
> + /* Failure to get a link might just be that no cards are inserted */
> + if (rcar_pcie_hw_init(pcie)) {
> dev_info(dev, "PCIe link down\n");
> err = -ENODEV;
> goto err_pm_put;
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
2018-04-09 10:54 ` Simon Horman
2018-04-09 10:55 ` Simon Horman
@ 2018-04-09 11:24 ` Simon Horman
2018-04-09 15:45 ` Sergei Shtylyov
2 siblings, 0 replies; 17+ messages in thread
From: Simon Horman @ 2018-04-09 11:24 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
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
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call
2018-04-09 11:04 ` Simon Horman
@ 2018-04-09 15:21 ` Sergei Shtylyov
0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-09 15:21 UTC (permalink / raw)
To: Simon Horman; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On 04/09/2018 02:04 PM, Simon Horman wrote:
>> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
>> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
>> makes sense to move that call into the driver's probe() method and then
>> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
>> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
>
> I'm not sure the churn is worth it, but if you do then that is find by me.
s/find/fine/? :-)
I think it's worth it -- makes the code follow more closely the manuals
where the only gen1/2/3 specific init is PHY related.
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> drivers/pci/host/pcie-rcar.c | 42 ++++++++++++++++++++++--------------------
>> 1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> Index: pci/drivers/pci/host/pcie-rcar.c
>> ===================================================================
>> --- pci.orig/drivers/pci/host/pcie-rcar.c
>> +++ pci/drivers/pci/host/pcie-rcar.c
[...]
>> @@ -1082,17 +1078,18 @@ static int rcar_pcie_parse_map_dma_range
>> }
>>
>> static const struct of_device_id rcar_pcie_of_match[] = {
>> - { .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
>> + { .compatible = "renesas,pcie-r8a7779",
>> + .data = rcar_pcie_phy_init_h1 },
>> { .compatible = "renesas,pcie-r8a7790",
>> - .data = rcar_pcie_hw_init_gen2 },
>> + .data = rcar_pcie_phy_init_gen2 },
>> { .compatible = "renesas,pcie-r8a7791",
>> - .data = rcar_pcie_hw_init_gen2 },
>> + .data = rcar_pcie_phy_init_gen2 },
>> { .compatible = "renesas,pcie-rcar-gen2",
>> - .data = rcar_pcie_hw_init_gen2 },
>> + .data = rcar_pcie_phy_init_gen2 },
>> { .compatible = "renesas,pcie-r8a7795",
>> - .data = rcar_pcie_hw_init_gen3 },
>> + .data = rcar_pcie_phy_init_gen3 },
>> { .compatible = "renesas,pcie-rcar-gen3",
>> - .data = rcar_pcie_hw_init_gen3 },
>> + .data = rcar_pcie_phy_init_gen3 },
>> {},
>
> I would avoid the line wrapping here, but its up to you.
I didn't want to break the 80-colums limit; and then again, wanted to keep the initializers alike...
>> @@ -1140,7 +1137,7 @@ static int rcar_pcie_probe(struct platfo
>> struct rcar_pcie *pcie;
>> unsigned int data;
>> int err;
>> - int (*hw_init_fn)(struct rcar_pcie *);
>> + int (*phy_init_fn)(struct rcar_pcie *);
>
> Looking at this I wonder if we also need a phy_cleanup() code or
> similar.
Makes sense -- iff we start supporting PM?..
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
2018-04-09 10:56 ` Simon Horman
@ 2018-04-09 15:33 ` Sergei Shtylyov
0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-09 15:33 UTC (permalink / raw)
To: Simon Horman; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
On 04/09/2018 01:56 PM, Simon Horman wrote:
>> Now that we've added PCIEPHYSR.PHYRDY polling to rcar_pcie_hw_init(),
>> there is no need anymore for polling the PHY specific register in
>> rcar_pcie_hw_init_h1() -- remove it.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> This looks good, but has it been tested?
Unfortunately, no. Yet it fully matches the R-Car H1 manual v1.00.
MBR, Sergei
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
2018-04-09 10:54 ` Simon Horman
2018-04-09 10:55 ` Simon Horman
2018-04-09 11:24 ` Simon Horman
@ 2018-04-09 15:45 ` Sergei Shtylyov
2 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2018-04-09 15:45 UTC (permalink / raw)
To: Simon Horman; +Cc: bhelgaas, linux-pci, linux-renesas-soc, Lorenzo Pieralisi
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-04-09 15:45 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-06 10:58 [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
2018-04-06 11:02 ` [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init() Sergei Shtylyov
2018-04-09 10:54 ` Simon Horman
2018-04-09 10:55 ` Simon Horman
2018-04-09 11:24 ` Simon Horman
2018-04-09 15:45 ` Sergei Shtylyov
2018-04-06 11:04 ` [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1() Sergei Shtylyov
2018-04-09 10:56 ` Simon Horman
2018-04-09 15:33 ` Sergei Shtylyov
2018-04-06 11:08 ` [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support Sergei Shtylyov
2018-04-07 18:35 ` Sergei Shtylyov
2018-04-09 11:00 ` Simon Horman
2018-04-06 11:10 ` [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call Sergei Shtylyov
2018-04-09 8:34 ` Geert Uytterhoeven
2018-04-09 11:04 ` Simon Horman
2018-04-09 15:21 ` Sergei Shtylyov
2018-04-06 11:24 ` [PATCH 0/4] Add R8A77980 PCIe support & some driver cleanups Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).