Dear Peter Chen,

[...]

> +static void ci_wait_vbus_stable(struct ci13xxx *ci, bool low)
> +{
> +     unsigned long timeout;
> +     u32 otgsc = hw_read(ci, OP_OTGSC, ~0);
> +
> +     timeout = jiffies + CI_WAIT_VBUS_STABLE_TIMEOUT;
> +
> +     if (low) {
> +             while (otgsc & OTGSC_BSV) {
> +                     if (time_after(jiffies, timeout)) {
> +                             dev_err(ci->dev, "wait vbus lower than\
> +                                             B_SESS_VALID timeout!\n");
> +                             return;
> +                     }
> +                     msleep(20);
> +                     otgsc = hw_read(ci, OP_OTGSC, ~0);
> +             }
> +     } else {
> +             while (!(otgsc & OTGSC_AVV)) {
> +                     if (time_after(jiffies, timeout)) {
> +                             dev_err(ci->dev, "wait vbus higher than\
> +                                             A_VBUS_VALID timeout!\n");
> +                             return;
> +                     }
> +                     msleep(20);
> +                     otgsc = hw_read(ci, OP_OTGSC, ~0);
> +             }

Just a nitpick really, but what about parametrizing this code so we won't have 
two copies of the same loop?

Say:

uint32_t bit = low ? OTGSC_BSV : OTGSC_AWW;
const char *name = low ? "B_SESS_VALID" : "A_VBUS_VALID";

while (!(otgsc & bit)) {
        if (time_after(jiffies, timeout)) {
                dev_err(ci->dev, "wait vbus higher than\
                                %s timeout!\n", name);
                return;
        }
        msleep(20);
        otgsc = hw_read(ci, OP_OTGSC, ~0);
}

And shall we not be more careful about endless loops?

[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to