Re: [PATCH] USB: Fix OTG HNP for hub.c

David Brownell
2007-05-29T19:21:20Z

On Tuesday 29 May 2007, Felipe Balbi wrote:
> From: Tony Lindgren <tony@atomide.com>
>
> This patch makes makes OTG Host Negotiation Protocol (HNP)
> to behave.

Not really. It makes it strongly *MIS* behave in fact,
since it always kicks off HNP ... even when it should not
do so, because the device is in the whitelist. (Called
the "Targeted Peripherals List", TPL, in jargonese.)

The two blocks of code are in the wrong order ...

A more limited patch would help. Specifically, just
fixing whatever bug was seen in the original code,
rather than abstracting it into a new subroutine and
changing the logic while doing so.

- Dave

> + /* If B-device wants to do HNP, it will do it after we suspend */
> + if (bus->b_hnp_enable) {
> + dev_err(&udev->dev, "trying HNP, suspending bus\n");
> + err = __usb_port_suspend(udev, udev->bus->otg_port);
> + if (err < 0) {
> + dev_err(&udev->dev, "suspend for HNP failed\n");
> + return -ENODEV;
> + }
> +
> + /* TB_ASE0_BRST, minimum 3.125 ms */
> + mdelay(4);
> +
> + /* Failed resume means B-device took over the bus with HNP */
> + err = usb_port_resume(udev);
> + if (err < 0) {
> + dev_err(&udev->dev, "HNP success\n");
> + return -ENODEV;
> + }
> + }
> +
> + /* HNP failed. Reject B-devices not in our whitelist */
> + if (!is_targeted(udev))
> + return -ENODEV;
> +