[U-Boot] [PATCH V5 03/18] usb: echi-hcd: add usb_lowlevel_init_device

Marek Vasut
2013-09-27T17:54:15Z

Dear Troy Kisky,

> Use this function so that we can verify the OTG_ID
> pin is high and device mode should be activated.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>
> ---
> v5: new patch
> ---
> drivers/usb/gadget/mv_udc.c | 2 +-
> drivers/usb/host/ehci-hcd.c | 13 +++++++++++++
> include/usb.h | 1 +
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/mv_udc.c b/drivers/usb/gadget/mv_udc.c
> index b87119c..ecd11d7 100644
> --- a/drivers/usb/gadget/mv_udc.c
> +++ b/drivers/usb/gadget/mv_udc.c
> @@ -684,7 +684,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver
> *driver) if (driver->speed != USB_SPEED_FULL && driver->speed !=
> USB_SPEED_HIGH) return -EINVAL;
>
> - ret = usb_lowlevel_init(0, (void **)&controller.ctrl);
> + ret = usb_lowlevel_init_device(0, (void **)&controller.ctrl);

You're crafting a new undocumented API here :-(

> if (ret)
> return ret;
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index fdad739..dbea756 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1004,6 +1004,19 @@ int usb_lowlevel_init(int index, void **controller)
> return 0;
> }
>
> +int usb_lowlevel_init_device(int index, void **controller)
> +{
> + int rc = ehci_hcd_init(index, &ehcic[index].hccr, &ehcic[index].hcor);
> +
> + /* rc == 0 means host mode, failure for us */

This looks pretty error-prone. A much better idea would be to use
usb_lowlevel_init() and add a flag to init the controller in either device or
host mode. That way you'd not even have to craft this strange new API.

> + if (!rc)
> + return -EINVAL;
> + if (rc != -ENODEV)
> + return rc;
> + *controller = &ehcic[index];
> + return 0;
> +}
> +
> int
> submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
> int length)
> diff --git a/include/usb.h b/include/usb.h
> index 60db897..4bc50cc 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -141,6 +141,7 @@ struct usb_device {
> defined(CONFIG_USB_MUSB_OMAP2PLUS)
>
> int usb_lowlevel_init(int index, void **controller);
> +int usb_lowlevel_init_device(int index, void **controller);
> int usb_lowlevel_stop(int index);
>
> int submit_bulk_msg(struct usb_device *dev, unsigned long pipe,

Best regards,
Marek Vasut