* [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
* 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
* [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 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
* [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
* 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 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