Re: [PATCH 4.19.y-cip 1/6] PCI: rcar: Move shareable code to a common file

Lad Prabhakar

Hi Pavel,

Thank you for the review.

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 23 October 2020 20:41
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das
Subject: Re: [PATCH 4.19.y-cip 1/6] PCI: rcar: Move shareable code to a common file


Move shareable code to common file pcie-rcar.c and the #defines to
pcie-rcar.h so that the common code can be reused with endpoint driver.
There are no functional changes with this patch for the host controller
I have merged the patches, but this should be cleaned up IMO...

+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;
Kind of fun. We test the PHYRDY, not ready, so we wait (now phy
becomes ready).. and we time out.

What about

while (1) {
if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
return 0;
if (!timeout)
return -ETIMEDOUT;

Agreed, Ill get this upstream first and then backport.

+int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
+ unsigned int timeout = 10000;
+ while (timeout--) {
+ if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
+ return 0;
+ udelay(5);
+ cpu_relax();
+ }
+ return -ETIMEDOUT;
This has same problem. Plus, I don't believe cpu_relax() is good idea
there. Relaxing CPU every 5usec will not change anything.
Agreed (as above). wrt cpu_relax() I believe this was added to improve the performance of other HW threads.


Best regards,

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Join to automatically receive all group messages.