[PATCH 0/4] Various R-Car PCIe patches

[PATCH 0/4] Various R-Car PCIe patches

[Phil Edworthy]

The first patch removes code that is no longer used. The next two allow us to
use runtim PM. The last patch is for PHY setup in order to get PCIe compliance
apparently. For that last patch, I've been told that this is what I must set.

Phil Edworthy (4):
  PCI: rcar: remove unused pci_sys_data structure in pcie-rcar
  PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar
  PCI: rcar: Add runtime PM support to pcie-rcar
  PCI: rcar: Add Gen2 PHY setup to pcie-rcar

 drivers/pci/host/pcie-rcar.c | 105 +++++++++++++++++++++++++++++++++----------
 1 file changed, 81 insertions(+), 24 deletions(-)

-- 
2.5.0

[PATCH 0/4] Various R-Car PCIe patches

[Phil Edworthy]

The first patch removes code that is no longer used. The next two allow us to
use runtim PM. The last patch is for PHY setup in order to get PCIe compliance
apparently. For that last patch, I've been told that this is what I must set.

Phil Edworthy (4):
  PCI: rcar: remove unused pci_sys_data structure in pcie-rcar
  PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar
  PCI: rcar: Add runtime PM support to pcie-rcar
  PCI: rcar: Add Gen2 PHY setup to pcie-rcar

 drivers/pci/host/pcie-rcar.c | 105 +++++++++++++++++++++++++++++++++----------
 1 file changed, 81 insertions(+), 24 deletions(-)

-- 
2.5.0

[PATCH 1/4] PCI: rcar: remove unused pci_sys_data structure in pcie-rcar

[Phil Edworthy]

Commit b3a72384fe29 ("ARM/PCI: Replace pci_sys_data->align_resource
with global function pointer") removed the struct pci_sys_data
dependency from the ARM pcibios functions, so remove it from this
driver.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 5c29626..c72c0ae 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -124,16 +124,7 @@ static inline struct rcar_msi *to_rcar_msi(struct msi_controller *chip)
 }
 
 /* Structure representing the PCIe interface */
-/*
- * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
- * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
- * that when we use a gen_pci pointer as sysdata, it is also a pointer to
- * a struct pci_sys_data.
- */
 struct rcar_pcie {
-#ifdef CONFIG_ARM
-	struct pci_sys_data	sys;
-#endif
 	struct device		*dev;
 	void __iomem		*base;
 	struct list_head	resources;
-- 
2.5.0

[PATCH 1/4] PCI: rcar: remove unused pci_sys_data structure in pcie-rcar

[Phil Edworthy]

Commit b3a72384fe29 ("ARM/PCI: Replace pci_sys_data->align_resource
with global function pointer") removed the struct pci_sys_data
dependency from the ARM pcibios functions, so remove it from this
driver.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 5c29626..c72c0ae 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -124,16 +124,7 @@ static inline struct rcar_msi *to_rcar_msi(struct msi_controller *chip)
 }
 
 /* Structure representing the PCIe interface */
-/*
- * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
- * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
- * that when we use a gen_pci pointer as sysdata, it is also a pointer to
- * a struct pci_sys_data.
- */
 struct rcar_pcie {
-#ifdef CONFIG_ARM
-	struct pci_sys_data	sys;
-#endif
 	struct device		*dev;
 	void __iomem		*base;
 	struct list_head	resources;
-- 
2.5.0

[PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

The R-Car PCIe host controller does not handle L1 ASPM. Instead, the
hardware needs assistance to transition to L1. When the controller
has received a PM_ENTER_L1 DLLP, we can't access a card's config regs
until we have got it out of L1 link state. The host controller will
handle this as long as it has also been transitioned to L1 link state.

So, when attempting a config access, check to see if the card has gone
into L1, and if so, do the same for the host controller.

This is based on a patch by Hien Dang <hien.dang.eb@rvc.renesas.com>

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index c72c0ae..4a4f8e1 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -83,6 +83,14 @@
 #define MACSR			0x011054
 #define MACCTLR			0x011058
 #define  SCRAMBLE_DISABLE	(1 << 27)
+#define PMSR			0x01105c
+#define  L1FAEG			(1 << 31)
+#define  PM_ENTER_L1RX		(1 << 23)
+#define  PMSTATE		(7 << 16)
+#define  PMSTATE_L1		(3 << 16)
+#define PMCTLR			0x011060
+#define  L1_INIT		(1 << 31)
+
 
 /* R-Car H1 PHY */
 #define H1_PCIEPHYADRR		0x04000c
@@ -175,6 +183,7 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
 		unsigned int devfn, int where, u32 *data)
 {
 	int dev, func, reg, index;
+	u32 val;
 
 	dev = PCI_SLOT(devfn);
 	func = PCI_FUNC(devfn);
@@ -216,6 +225,22 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
 	if (pcie->root_bus_nr < 0)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
+	/*
+	 * If we are not in L1 link state but have received PM_ENTER_L1 DLLP,
+	 * transition to L1 link state. The HW will handle coming out of L1.
+	 */
+	val = rcar_pci_read_reg(pcie, PMSR);
+	if ((val & PM_ENTER_L1RX) && ((val & PMSTATE) != PMSTATE_L1)) {
+		rcar_pci_write_reg(pcie, L1_INIT, PMCTLR);
+
+		/* Wait until we are in L1 */
+		while (!(val & L1FAEG))
+			val = rcar_pci_read_reg(pcie, PMSR);
+
+		/* Clear flags indicating link has transitioned to L1 */
+		rcar_pci_write_reg(pcie, L1FAEG | PM_ENTER_L1RX, PMSR);
+	}
+
 	/* Clear errors */
 	rcar_pci_write_reg(pcie, rcar_pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
 
-- 
2.5.0

[PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

The R-Car PCIe host controller does not handle L1 ASPM. Instead, the
hardware needs assistance to transition to L1. When the controller
has received a PM_ENTER_L1 DLLP, we can't access a card's config regs
until we have got it out of L1 link state. The host controller will
handle this as long as it has also been transitioned to L1 link state.

So, when attempting a config access, check to see if the card has gone
into L1, and if so, do the same for the host controller.

This is based on a patch by Hien Dang <hien.dang.eb@rvc.renesas.com>

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index c72c0ae..4a4f8e1 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -83,6 +83,14 @@
 #define MACSR			0x011054
 #define MACCTLR			0x011058
 #define  SCRAMBLE_DISABLE	(1 << 27)
+#define PMSR			0x01105c
+#define  L1FAEG			(1 << 31)
+#define  PM_ENTER_L1RX		(1 << 23)
+#define  PMSTATE		(7 << 16)
+#define  PMSTATE_L1		(3 << 16)
+#define PMCTLR			0x011060
+#define  L1_INIT		(1 << 31)
+
 
 /* R-Car H1 PHY */
 #define H1_PCIEPHYADRR		0x04000c
@@ -175,6 +183,7 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
 		unsigned int devfn, int where, u32 *data)
 {
 	int dev, func, reg, index;
+	u32 val;
 
 	dev = PCI_SLOT(devfn);
 	func = PCI_FUNC(devfn);
@@ -216,6 +225,22 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
 	if (pcie->root_bus_nr < 0)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
+	/*
+	 * If we are not in L1 link state but have received PM_ENTER_L1 DLLP,
+	 * transition to L1 link state. The HW will handle coming out of L1.
+	 */
+	val = rcar_pci_read_reg(pcie, PMSR);
+	if ((val & PM_ENTER_L1RX) && ((val & PMSTATE) != PMSTATE_L1)) {
+		rcar_pci_write_reg(pcie, L1_INIT, PMCTLR);
+
+		/* Wait until we are in L1 */
+		while (!(val & L1FAEG))
+			val = rcar_pci_read_reg(pcie, PMSR);
+
+		/* Clear flags indicating link has transitioned to L1 */
+		rcar_pci_write_reg(pcie, L1FAEG | PM_ENTER_L1RX, PMSR);
+	}
+
 	/* Clear errors */
 	rcar_pci_write_reg(pcie, rcar_pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
 
-- 
2.5.0

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Hi Phil,

> +		/* Wait until we are in L1 */
> +		while (!(val & L1FAEG))
> +			val = rcar_pci_read_reg(pcie, PMSR);

No timeout?

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Hi Phil,

> +		/* Wait until we are in L1 */
> +		while (!(val & L1FAEG))
> +			val = rcar_pci_read_reg(pcie, PMSR);

No timeout?

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

On Thu, Dec 17, 2015 at 2:30 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> +             /* Wait until we are in L1 */
>> +             while (!(val & L1FAEG))
>> +                     val = rcar_pci_read_reg(pcie, PMSR);
>
> No timeout?

And no cpu_relax() in each iteration.

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

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

On Thu, Dec 17, 2015 at 2:30 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> +             /* Wait until we are in L1 */
>> +             while (!(val & L1FAEG))
>> +                     val = rcar_pci_read_reg(pcie, PMSR);
>
> No timeout?

And no cpu_relax() in each iteration.

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

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

T24gMTcgRGVjZW1iZXIgMjAxNSAxMzo0MSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3RlOg0KPiBP
biBUaHUsIERlYyAxNywgMjAxNSBhdCAyOjMwIFBNLCBXb2xmcmFtIFNhbmcgPHdzYUB0aGUtZHJl
YW1zLmRlPiB3cm90ZToNCj4gPj4gKyAgICAgICAgICAgICAvKiBXYWl0IHVudGlsIHdlIGFyZSBp
biBMMSAqLw0KPiA+PiArICAgICAgICAgICAgIHdoaWxlICghKHZhbCAmIEwxRkFFRykpDQo+ID4+
ICsgICAgICAgICAgICAgICAgICAgICB2YWwgPSByY2FyX3BjaV9yZWFkX3JlZyhwY2llLCBQTVNS
KTsNCj4gPg0KPiA+IE5vIHRpbWVvdXQ/DQo+IA0KPiBBbmQgbm8gY3B1X3JlbGF4KCkgaW4gZWFj
aCBpdGVyYXRpb24uDQpTdXJlLCBJJ2xsIGZpeCB0aGF0Lg0KDQpUaGFua3MNClBoaWwNCg0KIA0K
PiBHcntvZXRqZSxlZXRpbmd9cywNCj4gDQo+ICAgICAgICAgICAgICAgICAgICAgICAgIEdlZXJ0
DQo+IA0KPiAtLQ0KPiBHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhlcmUncyBsb3RzIG9mIExpbnV4
IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQo+IA0KPiBJbiBwZXJzb25hbCBj
b252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBjYWxsIG15c2VsZiBhIGhhY2tl
ci4gQnV0DQo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlzdHMgSSBqdXN0IHNheSAicHJv
Z3JhbW1lciIgb3Igc29tZXRoaW5nIGxpa2UgdGhhdC4NCj4gICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0K

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 906 bytes --]

On 17 December 2015 13:41, Geert Uytterhoeven wrote:
> On Thu, Dec 17, 2015 at 2:30 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> >> +             /* Wait until we are in L1 */
> >> +             while (!(val & L1FAEG))
> >> +                     val = rcar_pci_read_reg(pcie, PMSR);
> >
> > No timeout?
> 
> And no cpu_relax() in each iteration.
Sure, I'll fix that.

Thanks
Phil

 
> 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
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

Hi Wolfram,

On 17 December 2015 13:31, Wolfram Sang wrote:
> Hi Phil,
> 
> > +		/* Wait until we are in L1 */
> > +		while (!(val & L1FAEG))
> > +			val = rcar_pci_read_reg(pcie, PMSR);
> 
> No timeout?
Since the hardware doesn't support hot plug, I believe this loop will
always exit very quickly. Unless someone has taken a hammer to the HW
of course.
However, point taken. I'll add a timeout.

> Regards,
> 
>    Wolfram

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]


> Since the hardware doesn't support hot plug, I believe this loop will
> always exit very quickly. Unless someone has taken a hammer to the HW
> of course.

I know what you mean. But since readl_poll_timeout() makes it easy, we
should better be safe than sorry.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]


> Since the hardware doesn't support hot plug, I believe this loop will
> always exit very quickly. Unless someone has taken a hammer to the HW
> of course.

I know what you mean. But since readl_poll_timeout() makes it easy, we
should better be safe than sorry.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

Hi Wolfram,

On 18 December 2015 14:04, Wolfram Sang wrote:
> > Since the hardware doesn't support hot plug, I believe this loop will
> > always exit very quickly. Unless someone has taken a hammer to the HW
> > of course.
> 
> I know what you mean. But since readl_poll_timeout() makes it easy, we
> should better be safe than sorry.
I haven't see that one before, very handy!

Thanks
Phil

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

Hi Wolfram,

On 18 December 2015 14:04, Wolfram Sang wrote:
> > Since the hardware doesn't support hot plug, I believe this loop will
> > always exit very quickly. Unless someone has taken a hammer to the HW
> > of course.
> 
> I know what you mean. But since readl_poll_timeout() makes it easy, we
> should better be safe than sorry.
Hmm, I changed the code, but now it doesn't come out of suspend unless
sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?

To test, I am just using:
Build the kernel with CONFIG_PM_DEBUG. 
echo platform > /sys/power/pm_test
echo N > /sys/module/printk/parameters/console_suspend
echo mem > /sys/power/state

Thanks
Phil

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

Hi Phil,

(this time with full CC list)

On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> On 18 December 2015 14:04, Wolfram Sang wrote:
>> > Since the hardware doesn't support hot plug, I believe this loop will
>> > always exit very quickly. Unless someone has taken a hammer to the HW
>> > of course.
>>
>> I know what you mean. But since readl_poll_timeout() makes it easy, we
>> should better be safe than sorry.
> Hmm, I changed the code, but now it doesn't come out of suspend unless
> sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?

Timers or interrupts disabled?

Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y?

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

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

Hi Phil,

(this time with full CC list)

On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> On 18 December 2015 14:04, Wolfram Sang wrote:
>> > Since the hardware doesn't support hot plug, I believe this loop will
>> > always exit very quickly. Unless someone has taken a hammer to the HW
>> > of course.
>>
>> I know what you mean. But since readl_poll_timeout() makes it easy, we
>> should better be safe than sorry.
> Hmm, I changed the code, but now it doesn't come out of suspend unless
> sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?

Timers or interrupts disabled?

Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y?

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

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

SGkgR2VlcnQsDQoNCk9uIDIxIERlY2VtYmVyIDIwMTUgMTM6MTcsIEdlZXJ0IFV5dHRlcmhvZXZl
biB3cm90ZToNCj4gT24gTW9uLCBEZWMgMjEsIDIwMTUgYXQgMTE6NTIgQU0sIFBoaWwgRWR3b3J0
aHkNCj4gPHBoaWwuZWR3b3J0aHlAcmVuZXNhcy5jb20+IHdyb3RlOg0KPiA+IE9uIDE4IERlY2Vt
YmVyIDIwMTUgMTQ6MDQsIFdvbGZyYW0gU2FuZyB3cm90ZToNCj4gPj4gPiBTaW5jZSB0aGUgaGFy
ZHdhcmUgZG9lc24ndCBzdXBwb3J0IGhvdCBwbHVnLCBJIGJlbGlldmUgdGhpcyBsb29wIHdpbGwN
Cj4gPj4gPiBhbHdheXMgZXhpdCB2ZXJ5IHF1aWNrbHkuIFVubGVzcyBzb21lb25lIGhhcyB0YWtl
biBhIGhhbW1lciB0byB0aGUgSFcNCj4gPj4gPiBvZiBjb3Vyc2UuDQo+ID4+DQo+ID4+IEkga25v
dyB3aGF0IHlvdSBtZWFuLiBCdXQgc2luY2UgcmVhZGxfcG9sbF90aW1lb3V0KCkgbWFrZXMgaXQg
ZWFzeSwgd2UNCj4gPj4gc2hvdWxkIGJldHRlciBiZSBzYWZlIHRoYW4gc29ycnkuDQo+ID4gSG1t
LCBJIGNoYW5nZWQgdGhlIGNvZGUsIGJ1dCBub3cgaXQgZG9lc24ndCBjb21lIG91dCBvZiBzdXNw
ZW5kIHVubGVzcw0KPiA+IHNsZWVwX3VzIHBhc3NlZCB0byByZWFkbF9wb2xsX3RpbWVvdXQgaXMg
MC4gQW55IHJlYXNvbiB5b3UgY2FuIHRoaW5rIG9mPw0KPiANCj4gVGltZXJzIG9yIGludGVycnVw
dHMgZGlzYWJsZWQ/DQo+IA0KPiBEb2VzIHRoZSBtaWdodF9zbGVlcF9pZigpIHNjcmVhbSBpZiBD
T05GSUdfREVCVUdfQVRPTUlDX1NMRUVQPXk/DQpZZXMsIGl0IGRvZXMgaW5kZWVkIHNjcmVhbS4g
V291bGQgeW91IHJlY29tbWVuZCB0byBzdGlsbCB1c2UgcmVhZGxfcG9sbF90aW1lb3V0DQp3aXRo
IHNsZWVwX3VzIHNldCB0byAwPw0KDQpUaGFua3MNClBoaWwNCg=

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

SGkgR2VlcnQsDQoNCk9uIDIxIERlY2VtYmVyIDIwMTUgMTM6MTcsIEdlZXJ0IFV5dHRlcmhvZXZl
biB3cm90ZToNCj4gT24gTW9uLCBEZWMgMjEsIDIwMTUgYXQgMTE6NTIgQU0sIFBoaWwgRWR3b3J0
aHkNCj4gPHBoaWwuZWR3b3J0aHlAcmVuZXNhcy5jb20+IHdyb3RlOg0KPiA+IE9uIDE4IERlY2Vt
YmVyIDIwMTUgMTQ6MDQsIFdvbGZyYW0gU2FuZyB3cm90ZToNCj4gPj4gPiBTaW5jZSB0aGUgaGFy
ZHdhcmUgZG9lc24ndCBzdXBwb3J0IGhvdCBwbHVnLCBJIGJlbGlldmUgdGhpcyBsb29wIHdpbGwN
Cj4gPj4gPiBhbHdheXMgZXhpdCB2ZXJ5IHF1aWNrbHkuIFVubGVzcyBzb21lb25lIGhhcyB0YWtl
biBhIGhhbW1lciB0byB0aGUgSFcNCj4gPj4gPiBvZiBjb3Vyc2UuDQo+ID4+DQo+ID4+IEkga25v
dyB3aGF0IHlvdSBtZWFuLiBCdXQgc2luY2UgcmVhZGxfcG9sbF90aW1lb3V0KCkgbWFrZXMgaXQg
ZWFzeSwgd2UNCj4gPj4gc2hvdWxkIGJldHRlciBiZSBzYWZlIHRoYW4gc29ycnkuDQo+ID4gSG1t
LCBJIGNoYW5nZWQgdGhlIGNvZGUsIGJ1dCBub3cgaXQgZG9lc24ndCBjb21lIG91dCBvZiBzdXNw
ZW5kIHVubGVzcw0KPiA+IHNsZWVwX3VzIHBhc3NlZCB0byByZWFkbF9wb2xsX3RpbWVvdXQgaXMg
MC4gQW55IHJlYXNvbiB5b3UgY2FuIHRoaW5rIG9mPw0KPiANCj4gVGltZXJzIG9yIGludGVycnVw
dHMgZGlzYWJsZWQ/DQo+IA0KPiBEb2VzIHRoZSBtaWdodF9zbGVlcF9pZigpIHNjcmVhbSBpZiBD
T05GSUdfREVCVUdfQVRPTUlDX1NMRUVQPXk/DQpZZXMsIGl0IGRvZXMgaW5kZWVkIHNjcmVhbS4g
V291bGQgeW91IHJlY29tbWVuZCB0byBzdGlsbCB1c2UgcmVhZGxfcG9sbF90aW1lb3V0DQp3aXRo
IHNsZWVwX3VzIHNldCB0byAwPw0KDQpUaGFua3MNClBoaWwNCg==

RE: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Phil Edworthy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1029 bytes --]

Hi Geert,

On 21 December 2015 13:17, Geert Uytterhoeven wrote:
> On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > On 18 December 2015 14:04, Wolfram Sang wrote:
> >> > Since the hardware doesn't support hot plug, I believe this loop will
> >> > always exit very quickly. Unless someone has taken a hammer to the HW
> >> > of course.
> >>
> >> I know what you mean. But since readl_poll_timeout() makes it easy, we
> >> should better be safe than sorry.
> > Hmm, I changed the code, but now it doesn't come out of suspend unless
> > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?
> 
> Timers or interrupts disabled?
> 
> Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y?
Yes, it does indeed scream. Would you recommend to still use readl_poll_timeout
with sleep_us set to 0?

Thanks
Phil
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

Hi Phil,

On Mon, Jan 4, 2016 at 3:18 PM, Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 21 December 2015 13:17, Geert Uytterhoeven wrote:
>> On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > On 18 December 2015 14:04, Wolfram Sang wrote:
>> >> > Since the hardware doesn't support hot plug, I believe this loop will
>> >> > always exit very quickly. Unless someone has taken a hammer to the HW
>> >> > of course.
>> >>
>> >> I know what you mean. But since readl_poll_timeout() makes it easy, we
>> >> should better be safe than sorry.
>> > Hmm, I changed the code, but now it doesn't come out of suspend unless
>> > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?
>>
>> Timers or interrupts disabled?
>>
>> Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y?
> Yes, it does indeed scream. Would you recommend to still use readl_poll_timeout
> with sleep_us set to 0?

Seems like people hit this before: use readl_poll_timeout_atomic().

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

Re: [PATCH 2/4] PCI: rcar: Support runtime PM link state L1 handling in pcie-rcar

[Geert Uytterhoeven]

Hi Phil,

On Mon, Jan 4, 2016 at 3:18 PM, Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 21 December 2015 13:17, Geert Uytterhoeven wrote:
>> On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > On 18 December 2015 14:04, Wolfram Sang wrote:
>> >> > Since the hardware doesn't support hot plug, I believe this loop will
>> >> > always exit very quickly. Unless someone has taken a hammer to the HW
>> >> > of course.
>> >>
>> >> I know what you mean. But since readl_poll_timeout() makes it easy, we
>> >> should better be safe than sorry.
>> > Hmm, I changed the code, but now it doesn't come out of suspend unless
>> > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of?
>>
>> Timers or interrupts disabled?
>>
>> Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y?
> Yes, it does indeed scream. Would you recommend to still use readl_poll_timeout
> with sleep_us set to 0?

Seems like people hit this before: use readl_poll_timeout_atomic().

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

[PATCH 3/4] PCI: rcar: Add runtime PM support to pcie-rcar

[Phil Edworthy]

If runtime PM is enabled in the kernel config, simply enable the
clocks once during probe.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 4a4f8e1..02a5993 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -26,6 +26,7 @@
 #include <linux/of_platform.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #define DRV_NAME "rcar-pcie"
@@ -1019,32 +1020,51 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	 if (err)
 		return err;
 
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		err = rcar_pcie_enable_msi(pcie);
-		if (err < 0) {
-			dev_err(&pdev->dev,
-				"failed to enable MSI support: %d\n",
-				err);
-			return err;
-		}
-	}
-
 	of_id = of_match_device(rcar_pcie_of_match, pcie->dev);
 	if (!of_id || !of_id->data)
 		return -EINVAL;
 	hw_init_fn = of_id->data;
 
+	pm_runtime_enable(pcie->dev);
+	err = pm_runtime_get_sync(pcie->dev);
+	if (err < 0) {
+		dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
+		goto err_pm_disable;
+	}
+
 	/* Failure to get a link might just be that no cards are inserted */
 	err = hw_init_fn(pcie);
 	if (err) {
 		dev_info(&pdev->dev, "PCIe link down\n");
-		return 0;
+		err = 0;
+		goto err_pm_put;
 	}
 
 	data = rcar_pci_read_reg(pcie, MACSR);
 	dev_info(&pdev->dev, "PCIe x%d: link up\n", (data >> 20) & 0x3f);
 
-	return rcar_pcie_enable(pcie);
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		err = rcar_pcie_enable_msi(pcie);
+		if (err < 0) {
+			dev_err(&pdev->dev,
+				"failed to enable MSI support: %d\n",
+				err);
+			goto err_pm_put;
+		}
+	}
+
+	err = rcar_pcie_enable(pcie);
+	if (err)
+		goto err_pm_put;
+
+	return 0;
+
+err_pm_put:
+	pm_runtime_put(pcie->dev);
+
+err_pm_disable:
+	pm_runtime_disable(pcie->dev);
+	return err;
 }
 
 static struct platform_driver rcar_pcie_driver = {
-- 
2.5.0

[PATCH 3/4] PCI: rcar: Add runtime PM support to pcie-rcar

[Phil Edworthy]

If runtime PM is enabled in the kernel config, simply enable the
clocks once during probe.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 4a4f8e1..02a5993 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -26,6 +26,7 @@
 #include <linux/of_platform.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #define DRV_NAME "rcar-pcie"
@@ -1019,32 +1020,51 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	 if (err)
 		return err;
 
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
-		err = rcar_pcie_enable_msi(pcie);
-		if (err < 0) {
-			dev_err(&pdev->dev,
-				"failed to enable MSI support: %d\n",
-				err);
-			return err;
-		}
-	}
-
 	of_id = of_match_device(rcar_pcie_of_match, pcie->dev);
 	if (!of_id || !of_id->data)
 		return -EINVAL;
 	hw_init_fn = of_id->data;
 
+	pm_runtime_enable(pcie->dev);
+	err = pm_runtime_get_sync(pcie->dev);
+	if (err < 0) {
+		dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
+		goto err_pm_disable;
+	}
+
 	/* Failure to get a link might just be that no cards are inserted */
 	err = hw_init_fn(pcie);
 	if (err) {
 		dev_info(&pdev->dev, "PCIe link down\n");
-		return 0;
+		err = 0;
+		goto err_pm_put;
 	}
 
 	data = rcar_pci_read_reg(pcie, MACSR);
 	dev_info(&pdev->dev, "PCIe x%d: link up\n", (data >> 20) & 0x3f);
 
-	return rcar_pcie_enable(pcie);
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		err = rcar_pcie_enable_msi(pcie);
+		if (err < 0) {
+			dev_err(&pdev->dev,
+				"failed to enable MSI support: %d\n",
+				err);
+			goto err_pm_put;
+		}
+	}
+
+	err = rcar_pcie_enable(pcie);
+	if (err)
+		goto err_pm_put;
+
+	return 0;
+
+err_pm_put:
+	pm_runtime_put(pcie->dev);
+
+err_pm_disable:
+	pm_runtime_disable(pcie->dev);
+	return err;
 }
 
 static struct platform_driver rcar_pcie_driver = {
-- 
2.5.0

[PATCH 4/4] PCI: rcar: Add Gen2 PHY setup to pcie-rcar

[Phil Edworthy]

For PCIe compliance, the PHY registers need setting as per the
manual.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 02a5993..61e112e 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -103,6 +103,11 @@
 #define H1_PCIEPHYDOUTR		0x040014
 #define H1_PCIEPHYSR		0x040018
 
+/* R-Car Gen2 PHY */
+#define GEN2_PCIEPHYADDR	0x780
+#define GEN2_PCIEPHYDATA	0x784
+#define GEN2_PCIEPHYCTRL	0x78c
+
 #define INT_PCI_MSI_NR	32
 
 #define RCONF(x)	(PCICONF(0)+(x))
@@ -593,6 +598,22 @@ static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
 	return -ETIMEDOUT;
 }
 
+static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
+{
+	rcar_pci_write_reg(pcie, 0x000f0030, GEN2_PCIEPHYADDR);
+	rcar_pci_write_reg(pcie, 0x00381203, GEN2_PCIEPHYDATA);
+	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
+	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
+
+	rcar_pci_write_reg(pcie, 0x000f0054, GEN2_PCIEPHYADDR);
+	/* The following value is for DC connection, no termination resistor */
+	rcar_pci_write_reg(pcie, 0x13802007, GEN2_PCIEPHYDATA);
+	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
+	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
+
+	return rcar_pcie_hw_init(pcie);
+}
+
 static int rcar_msi_alloc(struct rcar_msi *chip)
 {
 	int msi;
@@ -932,9 +953,9 @@ static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
 
 static const struct of_device_id rcar_pcie_of_match[] = {
 	{ .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
-	{ .compatible = "renesas,pcie-rcar-gen2", .data = rcar_pcie_hw_init },
-	{ .compatible = "renesas,pcie-r8a7790", .data = rcar_pcie_hw_init },
-	{ .compatible = "renesas,pcie-r8a7791", .data = rcar_pcie_hw_init },
+	{ .compatible = "renesas,pcie-rcar-gen2", .data = rcar_pcie_hw_init_gen2 },
+	{ .compatible = "renesas,pcie-r8a7790", .data = rcar_pcie_hw_init_gen2 },
+	{ .compatible = "renesas,pcie-r8a7791", .data = rcar_pcie_hw_init_gen2 },
 	{ .compatible = "renesas,pcie-r8a7795", .data = rcar_pcie_hw_init },
 	{},
 };
-- 
2.5.0

[PATCH 4/4] PCI: rcar: Add Gen2 PHY setup to pcie-rcar

[Phil Edworthy]

For PCIe compliance, the PHY registers need setting as per the
manual.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 02a5993..61e112e 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -103,6 +103,11 @@
 #define H1_PCIEPHYDOUTR		0x040014
 #define H1_PCIEPHYSR		0x040018
 
+/* R-Car Gen2 PHY */
+#define GEN2_PCIEPHYADDR	0x780
+#define GEN2_PCIEPHYDATA	0x784
+#define GEN2_PCIEPHYCTRL	0x78c
+
 #define INT_PCI_MSI_NR	32
 
 #define RCONF(x)	(PCICONF(0)+(x))
@@ -593,6 +598,22 @@ static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
 	return -ETIMEDOUT;
 }
 
+static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
+{
+	rcar_pci_write_reg(pcie, 0x000f0030, GEN2_PCIEPHYADDR);
+	rcar_pci_write_reg(pcie, 0x00381203, GEN2_PCIEPHYDATA);
+	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
+	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
+
+	rcar_pci_write_reg(pcie, 0x000f0054, GEN2_PCIEPHYADDR);
+	/* The following value is for DC connection, no termination resistor */
+	rcar_pci_write_reg(pcie, 0x13802007, GEN2_PCIEPHYDATA);
+	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
+	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
+
+	return rcar_pcie_hw_init(pcie);
+}
+
 static int rcar_msi_alloc(struct rcar_msi *chip)
 {
 	int msi;
@@ -932,9 +953,9 @@ static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
 
 static const struct of_device_id rcar_pcie_of_match[] = {
 	{ .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
-	{ .compatible = "renesas,pcie-rcar-gen2", .data = rcar_pcie_hw_init },
-	{ .compatible = "renesas,pcie-r8a7790", .data = rcar_pcie_hw_init },
-	{ .compatible = "renesas,pcie-r8a7791", .data = rcar_pcie_hw_init },
+	{ .compatible = "renesas,pcie-rcar-gen2", .data = rcar_pcie_hw_init_gen2 },
+	{ .compatible = "renesas,pcie-r8a7790", .data = rcar_pcie_hw_init_gen2 },
+	{ .compatible = "renesas,pcie-r8a7791", .data = rcar_pcie_hw_init_gen2 },
 	{ .compatible = "renesas,pcie-r8a7795", .data = rcar_pcie_hw_init },
 	{},
 };
-- 
2.5.0

Re: [PATCH 4/4] PCI: rcar: Add Gen2 PHY setup to pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]


> +	rcar_pci_write_reg(pcie, 0x000f0030, GEN2_PCIEPHYADDR);
> +	rcar_pci_write_reg(pcie, 0x00381203, GEN2_PCIEPHYDATA);
> +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
> +
> +	rcar_pci_write_reg(pcie, 0x000f0054, GEN2_PCIEPHYADDR);
> +	/* The following value is for DC connection, no termination resistor */
> +	rcar_pci_write_reg(pcie, 0x13802007, GEN2_PCIEPHYDATA);
> +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);

I'd vote for a comment saying where these magic values come from, i.e.
which manual, which chapter, etc...


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/4] PCI: rcar: Add Gen2 PHY setup to pcie-rcar

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]


> +	rcar_pci_write_reg(pcie, 0x000f0030, GEN2_PCIEPHYADDR);
> +	rcar_pci_write_reg(pcie, 0x00381203, GEN2_PCIEPHYDATA);
> +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
> +
> +	rcar_pci_write_reg(pcie, 0x000f0054, GEN2_PCIEPHYADDR);
> +	/* The following value is for DC connection, no termination resistor */
> +	rcar_pci_write_reg(pcie, 0x13802007, GEN2_PCIEPHYDATA);
> +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);

I'd vote for a comment saying where these magic values come from, i.e.
which manual, which chapter, etc...


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH 4/4] PCI: rcar: Add Gen2 PHY setup to pcie-rcar

[Phil Edworthy]

Hi Wolfram,

On 17 December 2015 13:33, Wolfram Sang wrote:
> > +	rcar_pci_write_reg(pcie, 0x000f0030, GEN2_PCIEPHYADDR);
> > +	rcar_pci_write_reg(pcie, 0x00381203, GEN2_PCIEPHYDATA);
> > +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> > +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
> > +
> > +	rcar_pci_write_reg(pcie, 0x000f0054, GEN2_PCIEPHYADDR);
> > +	/* The following value is for DC connection, no termination resistor */
> > +	rcar_pci_write_reg(pcie, 0x13802007, GEN2_PCIEPHYDATA);
> > +	rcar_pci_write_reg(pcie, 0x00000001, GEN2_PCIEPHYCTRL);
> > +	rcar_pci_write_reg(pcie, 0x00000006, GEN2_PCIEPHYCTRL);
> 
> I'd vote for a comment saying where these magic values come from, i.e.
> which manual, which chapter, etc...
Ok, will do.

Thanks
Phil