Hi Chunfeng,

Thank you for the patch!

> From: Chunfeng Yun, Sent: Thursday, September 10, 2020 5:22 PM
> 
> Use readl_poll_timeout_atomic() to simplify code
> 
> Cc: Mathias Nyman <mathias.ny...@linux.intel.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Signed-off-by: Chunfeng Yun <chunfeng....@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  drivers/usb/host/xhci-rcar.c | 43 ++++++++++++-------------------------------
>  1 file changed, 12 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
> index c1025d3..74f836f 100644
> --- a/drivers/usb/host/xhci-rcar.c
> +++ b/drivers/usb/host/xhci-rcar.c
> @@ -6,6 +6,7 @@
>   */
> 
>  #include <linux/firmware.h>
> +#include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/of.h>
> @@ -127,8 +128,7 @@ static int xhci_rcar_download_firmware(struct usb_hcd 
> *hcd)
>       void __iomem *regs = hcd->regs;
>       struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
>       const struct firmware *fw;
> -     int retval, index, j, time;
> -     int timeout = 10000;
> +     int retval, index, j;
>       u32 data, val, temp;
>       u32 quirks = 0;
>       const struct soc_device_attribute *attr;
> @@ -166,32 +166,19 @@ static int xhci_rcar_download_firmware(struct usb_hcd 
> *hcd)
>               temp |= RCAR_USB3_DL_CTRL_FW_SET_DATA0;
>               writel(temp, regs + RCAR_USB3_DL_CTRL);
> 
> -             for (time = 0; time < timeout; time++) {
> -                     val = readl(regs + RCAR_USB3_DL_CTRL);
> -                     if ((val & RCAR_USB3_DL_CTRL_FW_SET_DATA0) == 0)
> -                             break;
> -                     udelay(1);
> -             }
> -             if (time == timeout) {
> -                     retval = -ETIMEDOUT;
> +             retval = readl_poll_timeout_atomic(regs + RCAR_USB3_DL_CTRL,
> +                             val, !(val & RCAR_USB3_DL_CTRL_FW_SET_DATA0),
> +                             1, 10000);
> +             if (retval < 0)
>                       break;
> -             }
>       }
> 
>       temp = readl(regs + RCAR_USB3_DL_CTRL);
>       temp &= ~RCAR_USB3_DL_CTRL_ENABLE;
>       writel(temp, regs + RCAR_USB3_DL_CTRL);
> 
> -     for (time = 0; time < timeout; time++) {
> -             val = readl(regs + RCAR_USB3_DL_CTRL);
> -             if (val & RCAR_USB3_DL_CTRL_FW_SUCCESS) {
> -                     retval = 0;
> -                     break;
> -             }
> -             udelay(1);
> -     }
> -     if (time == timeout)
> -             retval = -ETIMEDOUT;
> +     retval = readl_poll_timeout_atomic((regs + RCAR_USB3_DL_CTRL),
> +                     val, (val & RCAR_USB3_DL_CTRL_FW_SUCCESS), 1, 10000);

Some parentheses are not needed like below:

        retval = readl_poll_timeout_atomic(regs + RCAR_USB3_DL_CTRL,
                        val, val & RCAR_USB3_DL_CTRL_FW_SUCCESS, 1, 10000);


>       release_firmware(fw);
> 
> @@ -200,18 +187,12 @@ static int xhci_rcar_download_firmware(struct usb_hcd 
> *hcd)
> 
>  static bool xhci_rcar_wait_for_pll_active(struct usb_hcd *hcd)
>  {
> -     int timeout = 1000;
> +     int retval;
>       u32 val, mask = RCAR_USB3_AXH_STA_PLL_ACTIVE_MASK;
> 
> -     while (timeout > 0) {
> -             val = readl(hcd->regs + RCAR_USB3_AXH_STA);
> -             if ((val & mask) == mask)
> -                     return true;
> -             udelay(1);
> -             timeout--;
> -     }
> -
> -     return false;
> +     retval = readl_poll_timeout_atomic(hcd->regs + RCAR_USB3_AXH_STA,
> +                     val, ((val & mask) == mask), 1, 1000);

Likewise above:
        retval = readl_poll_timeout_atomic(hcd->regs + RCAR_USB3_AXH_STA,
                        val, (val & mask) == mask, 1, 1000);

> +     return !!retval;

This breaks the code. If I changed this to "return retval < 0 ? false : true;",
it works again.

Best regards,
Yoshihiro Shimoda

Reply via email to