| Message ID | 20210426132632.10221-3-linux.amoon@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | [1/3] pci: pcie_dw_rockchip: Fixed the below compilation error | expand |
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 >
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
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)
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
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) {
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) {
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(-)