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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
