All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error
@ 2021-04-26 13:26 Anand Moon
  2021-04-26 13:26 ` [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning Anand Moon
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Anand Moon @ 2021-04-26 13:26 UTC (permalink / raw)
  To: u-boot

Use the Error values that may be returned by PCI functions
Added the error macro from linux/include/linux/pci.h

drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_read':
drivers/pci/pcie_dw_rockchip.c:70:10: error: 'PCIBIOS_UNSUPPORTED'
			undeclared (first use in this function)
   70 |   return PCIBIOS_UNSUPPORTED;
      |          ^~~~~~~~~~~~~~~~~~~
drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_write':
drivers/pci/pcie_dw_rockchip.c:90:10: error: 'PCIBIOS_UNSUPPORTED'
			undeclared (first use in this function)
   90 |   return PCIBIOS_UNSUPPORTED;
      |          ^~~~~~~~~~~~~~~~~~~

Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/pci/pcie_dw_common.h   | 9 +++++++++
 drivers/pci/pcie_dw_rockchip.c | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_common.h b/drivers/pci/pcie_dw_common.h
index 6b701645af..ba5feb5b51 100644
--- a/drivers/pci/pcie_dw_common.h
+++ b/drivers/pci/pcie_dw_common.h
@@ -90,6 +90,15 @@
 #define PCIE_MISC_CONTROL_1_OFF		0x8bc
 #define PCIE_DBI_RO_WR_EN		BIT(0)
 
+/* Error values that may be returned by PCI functions */
+#define PCIBIOS_SUCCESSFUL              0x00
+#define PCIBIOS_FUNC_NOT_SUPPORTED      0x81
+#define PCIBIOS_BAD_VENDOR_ID           0x83
+#define PCIBIOS_DEVICE_NOT_FOUND        0x86
+#define PCIBIOS_BAD_REGISTER_NUMBER     0x87
+#define PCIBIOS_SET_FAILED              0x88
+#define PCIBIOS_BUFFER_TOO_SMALL        0x89
+
 /* Parameters for the waiting for iATU enabled routine */
 #define LINK_WAIT_MAX_IATU_RETRIES	5
 #define LINK_WAIT_IATU			10000
diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index bc22af4230..9702b40019 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -67,7 +67,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
 {
 	if ((uintptr_t)addr & (size - 1)) {
 		*val = 0;
-		return PCIBIOS_UNSUPPORTED;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 	}
 
 	if (size == 4) {
@@ -87,7 +87,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
 static int rk_pcie_write(void __iomem *addr, int size, u32 val)
 {
 	if ((uintptr_t)addr & (size - 1))
-		return PCIBIOS_UNSUPPORTED;
+		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	if (size == 4)
 		writel(val, addr);
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning
  2021-04-26 13:26 [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Anand Moon
@ 2021-04-26 13:26 ` Anand Moon
  2021-05-21 13:21   ` Kever Yang
  2021-04-26 13:26 ` [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep Anand Moon
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Anand Moon @ 2021-04-26 13:26 UTC (permalink / raw)
  To: u-boot

Drop the unused variable warning below.

drivers/pci/pcie_dw_rockchip.c:161:6: warning: unused variable
			'val' [-Wunused-variable]
  161 |  u32 val;
      |      ^~~

Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/pci/pcie_dw_rockchip.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index 9702b40019..e7f42604ab 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -158,8 +158,6 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg,
  */
 static void rk_pcie_configure(struct rk_pcie *pci, u32 cap_speed)
 {
-	u32 val;
-
 	dw_pcie_dbi_write_enable(&pci->dw, true);
 
 	clrsetbits_le32(pci->dw.dbi_base + PCIE_LINK_CAPABILITY,
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-26 13:26 [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Anand Moon
  2021-04-26 13:26 ` [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning Anand Moon
@ 2021-04-26 13:26 ` Anand Moon
  2021-04-26 20:08   ` Patrick Wildt
  2021-05-21 13:20   ` Kever Yang
  2021-04-26 20:10 ` [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Patrick Wildt
  2021-05-21 13:21 ` Kever Yang
  3 siblings, 2 replies; 13+ messages in thread
From: Anand Moon @ 2021-04-26 13:26 UTC (permalink / raw)
  To: u-boot

Use udelay instead of msleep fix the below warning.

drivers/pci/pcie_dw_rockchip.c:254:3: warning: implicit
	declaration of function 'msleep' [-Wimplicit-function-declaration]

Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Kever Yang <kever.yang@rock-chips.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/pci/pcie_dw_rockchip.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
index e7f42604ab..6c87ee1ea0 100644
--- a/drivers/pci/pcie_dw_rockchip.c
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -249,7 +249,7 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
 		 * some wired devices need much more, such as 600ms.
 		 * Add a enough delay to cover all cases.
 		 */
-		msleep(PERST_WAIT_MS);
+		udelay(PERST_WAIT_MS);
 		dm_gpio_set_value(&priv->rst_gpio, 1);
 	}
 
@@ -271,12 +271,12 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
 		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
 			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
 		rk_pcie_debug_dump(priv);
-		msleep(1000);
+		udelay(1000);
 	}
 
 	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
 	/* Link maybe in Gen switch recovery but we need to wait more 1s */
-	msleep(1000);
+	udelay(1000);
 	return -EIO;
 }
 
@@ -296,7 +296,7 @@ static int rockchip_pcie_init_port(struct udevice *dev)
 		}
 	}
 
-	msleep(1000);
+	udelay(1000);
 
 	ret = generic_phy_init(&priv->phy);
 	if (ret) {
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-26 13:26 ` [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep Anand Moon
@ 2021-04-26 20:08   ` Patrick Wildt
  2021-04-27  5:41     ` Anand Moon
  2021-05-21 13:20   ` Kever Yang
  1 sibling, 1 reply; 13+ messages in thread
From: Patrick Wildt @ 2021-04-26 20:08 UTC (permalink / raw)
  To: u-boot

Am Mon, Apr 26, 2021 at 01:26:32PM +0000 schrieb Anand Moon:
> Use udelay instead of msleep fix the below warning.

You sure that's correct? the m in msleep means milli, while the u
in udelay means micro.  That's a factor of 1000 of a difference.

> drivers/pci/pcie_dw_rockchip.c:254:3: warning: implicit
> 	declaration of function 'msleep' [-Wimplicit-function-declaration]
> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/pci/pcie_dw_rockchip.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index e7f42604ab..6c87ee1ea0 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -249,7 +249,7 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>  		 * some wired devices need much more, such as 600ms.
>  		 * Add a enough delay to cover all cases.
>  		 */
> -		msleep(PERST_WAIT_MS);
> +		udelay(PERST_WAIT_MS);
>  		dm_gpio_set_value(&priv->rst_gpio, 1);
>  	}
>  
> @@ -271,12 +271,12 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>  		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
>  			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
>  		rk_pcie_debug_dump(priv);
> -		msleep(1000);
> +		udelay(1000);
>  	}
>  
>  	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
>  	/* Link maybe in Gen switch recovery but we need to wait more 1s */
> -	msleep(1000);
> +	udelay(1000);
>  	return -EIO;
>  }
>  
> @@ -296,7 +296,7 @@ static int rockchip_pcie_init_port(struct udevice *dev)
>  		}
>  	}
>  
> -	msleep(1000);
> +	udelay(1000);
>  
>  	ret = generic_phy_init(&priv->phy);
>  	if (ret) {
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error
  2021-04-26 13:26 [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Anand Moon
  2021-04-26 13:26 ` [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning Anand Moon
  2021-04-26 13:26 ` [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep Anand Moon
@ 2021-04-26 20:10 ` Patrick Wildt
  2021-04-27  5:40   ` Anand Moon
  2021-05-21 13:21 ` Kever Yang
  3 siblings, 1 reply; 13+ messages in thread
From: Patrick Wildt @ 2021-04-26 20:10 UTC (permalink / raw)
  To: u-boot

Am Mon, Apr 26, 2021 at 01:26:30PM +0000 schrieb Anand Moon:
> Use the Error values that may be returned by PCI functions
> Added the error macro from linux/include/linux/pci.h
> 
> drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_read':
> drivers/pci/pcie_dw_rockchip.c:70:10: error: 'PCIBIOS_UNSUPPORTED'
> 			undeclared (first use in this function)
>    70 |   return PCIBIOS_UNSUPPORTED;
>       |          ^~~~~~~~~~~~~~~~~~~
> drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_write':
> drivers/pci/pcie_dw_rockchip.c:90:10: error: 'PCIBIOS_UNSUPPORTED'
> 			undeclared (first use in this function)
>    90 |   return PCIBIOS_UNSUPPORTED;
>       |          ^~~~~~~~~~~~~~~~~~~
> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/pci/pcie_dw_common.h   | 9 +++++++++
>  drivers/pci/pcie_dw_rockchip.c | 4 ++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie_dw_common.h b/drivers/pci/pcie_dw_common.h
> index 6b701645af..ba5feb5b51 100644
> --- a/drivers/pci/pcie_dw_common.h
> +++ b/drivers/pci/pcie_dw_common.h
> @@ -90,6 +90,15 @@
>  #define PCIE_MISC_CONTROL_1_OFF		0x8bc
>  #define PCIE_DBI_RO_WR_EN		BIT(0)
>  
> +/* Error values that may be returned by PCI functions */
> +#define PCIBIOS_SUCCESSFUL              0x00
> +#define PCIBIOS_FUNC_NOT_SUPPORTED      0x81
> +#define PCIBIOS_BAD_VENDOR_ID           0x83
> +#define PCIBIOS_DEVICE_NOT_FOUND        0x86
> +#define PCIBIOS_BAD_REGISTER_NUMBER     0x87
> +#define PCIBIOS_SET_FAILED              0x88
> +#define PCIBIOS_BUFFER_TOO_SMALL        0x89
> +
>  /* Parameters for the waiting for iATU enabled routine */
>  #define LINK_WAIT_MAX_IATU_RETRIES	5
>  #define LINK_WAIT_IATU			10000
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index bc22af4230..9702b40019 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -67,7 +67,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>  {
>  	if ((uintptr_t)addr & (size - 1)) {
>  		*val = 0;
> -		return PCIBIOS_UNSUPPORTED;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;

Since this function seems to return normal error code, why not use
-EINVAL?  Or even better -EOPNOTSUP?  The callers only check for != 0
anyway.

>  	}
>  
>  	if (size == 4) {
> @@ -87,7 +87,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>  static int rk_pcie_write(void __iomem *addr, int size, u32 val)
>  {
>  	if ((uintptr_t)addr & (size - 1))
> -		return PCIBIOS_UNSUPPORTED;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (size == 4)
>  		writel(val, addr);
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error
  2021-04-26 20:10 ` [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Patrick Wildt
@ 2021-04-27  5:40   ` Anand Moon
  0 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2021-04-27  5:40 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On Tue, 27 Apr 2021 at 01:40, Patrick Wildt <patrick@blueri.se> wrote:
>
> Am Mon, Apr 26, 2021 at 01:26:30PM +0000 schrieb Anand Moon:
> > Use the Error values that may be returned by PCI functions
> > Added the error macro from linux/include/linux/pci.h
> >
> > drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_read':
> > drivers/pci/pcie_dw_rockchip.c:70:10: error: 'PCIBIOS_UNSUPPORTED'
> >                       undeclared (first use in this function)
> >    70 |   return PCIBIOS_UNSUPPORTED;
> >       |          ^~~~~~~~~~~~~~~~~~~
> > drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_write':
> > drivers/pci/pcie_dw_rockchip.c:90:10: error: 'PCIBIOS_UNSUPPORTED'
> >                       undeclared (first use in this function)
> >    90 |   return PCIBIOS_UNSUPPORTED;
> >       |          ^~~~~~~~~~~~~~~~~~~
> >
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Kever Yang <kever.yang@rock-chips.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  drivers/pci/pcie_dw_common.h   | 9 +++++++++
> >  drivers/pci/pcie_dw_rockchip.c | 4 ++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pcie_dw_common.h b/drivers/pci/pcie_dw_common.h
> > index 6b701645af..ba5feb5b51 100644
> > --- a/drivers/pci/pcie_dw_common.h
> > +++ b/drivers/pci/pcie_dw_common.h
> > @@ -90,6 +90,15 @@
> >  #define PCIE_MISC_CONTROL_1_OFF              0x8bc
> >  #define PCIE_DBI_RO_WR_EN            BIT(0)
> >
> > +/* Error values that may be returned by PCI functions */
> > +#define PCIBIOS_SUCCESSFUL              0x00
> > +#define PCIBIOS_FUNC_NOT_SUPPORTED      0x81
> > +#define PCIBIOS_BAD_VENDOR_ID           0x83
> > +#define PCIBIOS_DEVICE_NOT_FOUND        0x86
> > +#define PCIBIOS_BAD_REGISTER_NUMBER     0x87
> > +#define PCIBIOS_SET_FAILED              0x88
> > +#define PCIBIOS_BUFFER_TOO_SMALL        0x89
> > +
> >  /* Parameters for the waiting for iATU enabled routine */
> >  #define LINK_WAIT_MAX_IATU_RETRIES   5
> >  #define LINK_WAIT_IATU                       10000
> > diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> > index bc22af4230..9702b40019 100644
> > --- a/drivers/pci/pcie_dw_rockchip.c
> > +++ b/drivers/pci/pcie_dw_rockchip.c
> > @@ -67,7 +67,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
> >  {
> >       if ((uintptr_t)addr & (size - 1)) {
> >               *val = 0;
> > -             return PCIBIOS_UNSUPPORTED;
> > +             return PCIBIOS_BAD_REGISTER_NUMBER;
>
> Since this function seems to return normal error code, why not use
> -EINVAL?  Or even better -EOPNOTSUP?  The callers only check for != 0
> anyway.
>

Thanks for your review comments.
-EOPNOTSUPP It seems to be the correct return code over here.


-Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-26 20:08   ` Patrick Wildt
@ 2021-04-27  5:41     ` Anand Moon
  2021-04-27 19:27       ` Patrick Wildt
  0 siblings, 1 reply; 13+ messages in thread
From: Anand Moon @ 2021-04-27  5:41 UTC (permalink / raw)
  To: u-boot

hi Patrick,

On Tue, 27 Apr 2021 at 01:38, Patrick Wildt <patrick@blueri.se> wrote:
>
> Am Mon, Apr 26, 2021 at 01:26:32PM +0000 schrieb Anand Moon:
> > Use udelay instead of msleep fix the below warning.
>
> You sure that's correct? the m in msleep means milli, while the u
> in udelay means micro.  That's a factor of 1000 of a difference.
>
Thanks for your review comments.

Most of the u-boot driver prefers udelay and usleep_range internally
calls udelay.

I don't have the HW to test and verify.



-Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-27  5:41     ` Anand Moon
@ 2021-04-27 19:27       ` Patrick Wildt
  2021-05-06 18:40         ` Anand Moon
  0 siblings, 1 reply; 13+ messages in thread
From: Patrick Wildt @ 2021-04-27 19:27 UTC (permalink / raw)
  To: u-boot

Am Tue, Apr 27, 2021 at 11:11:19AM +0530 schrieb Anand Moon:
> hi Patrick,
> 
> On Tue, 27 Apr 2021 at 01:38, Patrick Wildt <patrick@blueri.se> wrote:
> >
> > Am Mon, Apr 26, 2021 at 01:26:32PM +0000 schrieb Anand Moon:
> > > Use udelay instead of msleep fix the below warning.
> >
> > You sure that's correct? the m in msleep means milli, while the u
> > in udelay means micro.  That's a factor of 1000 of a difference.
> >
> Thanks for your review comments.
> 
> Most of the u-boot driver prefers udelay and usleep_range internally
> calls udelay.
> 
> I don't have the HW to test and verify.
> 
> -Anand

Sure, I'm not complaining about that.  My point is that if you pass
e. g. 8 milliseconds to a function that takes microseconds, you need
to add the factor.

Not good: msleep(1000) -> udelay(1000)
Much better: msleep(1000) -> udelay(1000 * 1000)

Which also means that you either have to rename PERST_WAIT_MS and change
its value, or do udelay(PERST_WAIT_MS * 1000)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-27 19:27       ` Patrick Wildt
@ 2021-05-06 18:40         ` Anand Moon
  0 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2021-05-06 18:40 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On Wed, 28 Apr 2021 at 00:57, Patrick Wildt <patrick@blueri.se> wrote:
>
> Am Tue, Apr 27, 2021 at 11:11:19AM +0530 schrieb Anand Moon:
> > hi Patrick,
> >
> > On Tue, 27 Apr 2021 at 01:38, Patrick Wildt <patrick@blueri.se> wrote:
> > >
> > > Am Mon, Apr 26, 2021 at 01:26:32PM +0000 schrieb Anand Moon:
> > > > Use udelay instead of msleep fix the below warning.
> > >
> > > You sure that's correct? the m in msleep means milli, while the u
> > > in udelay means micro.  That's a factor of 1000 of a difference.
> > >
> > Thanks for your review comments.
> >
> > Most of the u-boot driver prefers udelay and usleep_range internally
> > calls udelay.
> >
> > I don't have the HW to test and verify.
> >
> > -Anand
>
> Sure, I'm not complaining about that.  My point is that if you pass
> e. g. 8 milliseconds to a function that takes microseconds, you need
> to add the factor.
>
> Not good: msleep(1000) -> udelay(1000)
> Much better: msleep(1000) -> udelay(1000 * 1000)
>
> Which also means that you either have to rename PERST_WAIT_MS and change
> its value, or do udelay(PERST_WAIT_MS * 1000)

Thanks for this tip, can we use mdelay as sugged above.
.
static inline void mdelay(unsigned long msec)
{
        udelay(1000 * msec);
}

Thanks
-Anand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep
  2021-04-26 13:26 ` [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep Anand Moon
  2021-04-26 20:08   ` Patrick Wildt
@ 2021-05-21 13:20   ` Kever Yang
  1 sibling, 0 replies; 13+ messages in thread
From: Kever Yang @ 2021-05-21 13:20 UTC (permalink / raw)
  To: Anand Moon, u-boot; +Cc: Neil Armstrong, Bin Meng

Hi Anand,

It's OK to update the API, but please keep the delay duration.


Thanks,

- Kever

On 2021/4/26 下午9:26, Anand Moon wrote:
> Use udelay instead of msleep fix the below warning.
>
> drivers/pci/pcie_dw_rockchip.c:254:3: warning: implicit
> 	declaration of function 'msleep' [-Wimplicit-function-declaration]
>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>   drivers/pci/pcie_dw_rockchip.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index e7f42604ab..6c87ee1ea0 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -249,7 +249,7 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>   		 * some wired devices need much more, such as 600ms.
>   		 * Add a enough delay to cover all cases.
>   		 */
> -		msleep(PERST_WAIT_MS);
> +		udelay(PERST_WAIT_MS);
>   		dm_gpio_set_value(&priv->rst_gpio, 1);
>   	}
>   
> @@ -271,12 +271,12 @@ static int rk_pcie_link_up(struct rk_pcie *priv, u32 cap_speed)
>   		dev_info(priv->dw.dev, "PCIe Linking... LTSSM is 0x%x\n",
>   			 rk_pcie_readl_apb(priv, PCIE_CLIENT_LTSSM_STATUS));
>   		rk_pcie_debug_dump(priv);
> -		msleep(1000);
> +		udelay(1000);
>   	}
>   
>   	dev_err(priv->dw.dev, "PCIe-%d Link Fail\n", dev_seq(priv->dw.dev));
>   	/* Link maybe in Gen switch recovery but we need to wait more 1s */
> -	msleep(1000);
> +	udelay(1000);
>   	return -EIO;
>   }
>   
> @@ -296,7 +296,7 @@ static int rockchip_pcie_init_port(struct udevice *dev)
>   		}
>   	}
>   
> -	msleep(1000);
> +	udelay(1000);
>   
>   	ret = generic_phy_init(&priv->phy);
>   	if (ret) {



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error
  2021-04-26 13:26 [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Anand Moon
                   ` (2 preceding siblings ...)
  2021-04-26 20:10 ` [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Patrick Wildt
@ 2021-05-21 13:21 ` Kever Yang
  2021-05-21 16:58   ` Anand Moon
  3 siblings, 1 reply; 13+ messages in thread
From: Kever Yang @ 2021-05-21 13:21 UTC (permalink / raw)
  To: Anand Moon, u-boot; +Cc: Neil Armstrong, Bin Meng


On 2021/4/26 下午9:26, Anand Moon wrote:
> Use the Error values that may be returned by PCI functions
> Added the error macro from linux/include/linux/pci.h
>
> drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_read':
> drivers/pci/pcie_dw_rockchip.c:70:10: error: 'PCIBIOS_UNSUPPORTED'
> 			undeclared (first use in this function)
>     70 |   return PCIBIOS_UNSUPPORTED;
>        |          ^~~~~~~~~~~~~~~~~~~
> drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_write':
> drivers/pci/pcie_dw_rockchip.c:90:10: error: 'PCIBIOS_UNSUPPORTED'
> 			undeclared (first use in this function)
>     90 |   return PCIBIOS_UNSUPPORTED;
>        |          ^~~~~~~~~~~~~~~~~~~
>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,

- Kever

> ---
>   drivers/pci/pcie_dw_common.h   | 9 +++++++++
>   drivers/pci/pcie_dw_rockchip.c | 4 ++--
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_common.h b/drivers/pci/pcie_dw_common.h
> index 6b701645af..ba5feb5b51 100644
> --- a/drivers/pci/pcie_dw_common.h
> +++ b/drivers/pci/pcie_dw_common.h
> @@ -90,6 +90,15 @@
>   #define PCIE_MISC_CONTROL_1_OFF		0x8bc
>   #define PCIE_DBI_RO_WR_EN		BIT(0)
>   
> +/* Error values that may be returned by PCI functions */
> +#define PCIBIOS_SUCCESSFUL              0x00
> +#define PCIBIOS_FUNC_NOT_SUPPORTED      0x81
> +#define PCIBIOS_BAD_VENDOR_ID           0x83
> +#define PCIBIOS_DEVICE_NOT_FOUND        0x86
> +#define PCIBIOS_BAD_REGISTER_NUMBER     0x87
> +#define PCIBIOS_SET_FAILED              0x88
> +#define PCIBIOS_BUFFER_TOO_SMALL        0x89
> +
>   /* Parameters for the waiting for iATU enabled routine */
>   #define LINK_WAIT_MAX_IATU_RETRIES	5
>   #define LINK_WAIT_IATU			10000
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index bc22af4230..9702b40019 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -67,7 +67,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>   {
>   	if ((uintptr_t)addr & (size - 1)) {
>   		*val = 0;
> -		return PCIBIOS_UNSUPPORTED;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>   	}
>   
>   	if (size == 4) {
> @@ -87,7 +87,7 @@ static int rk_pcie_read(void __iomem *addr, int size, u32 *val)
>   static int rk_pcie_write(void __iomem *addr, int size, u32 val)
>   {
>   	if ((uintptr_t)addr & (size - 1))
> -		return PCIBIOS_UNSUPPORTED;
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
>   
>   	if (size == 4)
>   		writel(val, addr);



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning
  2021-04-26 13:26 ` [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning Anand Moon
@ 2021-05-21 13:21   ` Kever Yang
  0 siblings, 0 replies; 13+ messages in thread
From: Kever Yang @ 2021-05-21 13:21 UTC (permalink / raw)
  To: Anand Moon, u-boot; +Cc: Neil Armstrong, Bin Meng


On 2021/4/26 下午9:26, Anand Moon wrote:
> Drop the unused variable warning below.
>
> drivers/pci/pcie_dw_rockchip.c:161:6: warning: unused variable
> 			'val' [-Wunused-variable]
>    161 |  u32 val;
>        |      ^~~
>
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Cc: Kever Yang <kever.yang@rock-chips.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,

- Kever

> ---
>   drivers/pci/pcie_dw_rockchip.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> index 9702b40019..e7f42604ab 100644
> --- a/drivers/pci/pcie_dw_rockchip.c
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -158,8 +158,6 @@ static inline void rk_pcie_writel_apb(struct rk_pcie *rk_pcie, u32 reg,
>    */
>   static void rk_pcie_configure(struct rk_pcie *pci, u32 cap_speed)
>   {
> -	u32 val;
> -
>   	dw_pcie_dbi_write_enable(&pci->dw, true);
>   
>   	clrsetbits_le32(pci->dw.dbi_base + PCIE_LINK_CAPABILITY,



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error
  2021-05-21 13:21 ` Kever Yang
@ 2021-05-21 16:58   ` Anand Moon
  0 siblings, 0 replies; 13+ messages in thread
From: Anand Moon @ 2021-05-21 16:58 UTC (permalink / raw)
  To: Kever Yang; +Cc: U-Boot Mailing List, Neil Armstrong, Bin Meng

H Kever,

On Fri, 21 May 2021 at 18:51, Kever Yang <kever.yang@rock-chips.com> wrote:
>
>
> On 2021/4/26 下午9:26, Anand Moon wrote:
> > Use the Error values that may be returned by PCI functions
> > Added the error macro from linux/include/linux/pci.h
> >
> > drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_read':
> > drivers/pci/pcie_dw_rockchip.c:70:10: error: 'PCIBIOS_UNSUPPORTED'
> >                       undeclared (first use in this function)
> >     70 |   return PCIBIOS_UNSUPPORTED;
> >        |          ^~~~~~~~~~~~~~~~~~~
> > drivers/pci/pcie_dw_rockchip.c: In function 'rk_pcie_write':
> > drivers/pci/pcie_dw_rockchip.c:90:10: error: 'PCIBIOS_UNSUPPORTED'
> >                       undeclared (first use in this function)
> >     90 |   return PCIBIOS_UNSUPPORTED;
> >        |          ^~~~~~~~~~~~~~~~~~~
> >
> > Cc: Neil Armstrong <narmstrong@baylibre.com>
> > Cc: Kever Yang <kever.yang@rock-chips.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>

Oops, I forgot to send the revised version of these patches.

> Thanks,
>
> - Kever

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-05-21 16:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 13:26 [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Anand Moon
2021-04-26 13:26 ` [PATCH 2/3] pci: pcie_dw_rockchip: Drop the unused variable warning Anand Moon
2021-05-21 13:21   ` Kever Yang
2021-04-26 13:26 ` [PATCH 3/3] pci: pcie_dw_rockchip: Use udelay instead of msleep Anand Moon
2021-04-26 20:08   ` Patrick Wildt
2021-04-27  5:41     ` Anand Moon
2021-04-27 19:27       ` Patrick Wildt
2021-05-06 18:40         ` Anand Moon
2021-05-21 13:20   ` Kever Yang
2021-04-26 20:10 ` [PATCH 1/3] pci: pcie_dw_rockchip: Fixed the below compilation error Patrick Wildt
2021-04-27  5:40   ` Anand Moon
2021-05-21 13:21 ` Kever Yang
2021-05-21 16:58   ` Anand Moon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.