[U-Boot] [RFC PATCH 1/7] dm: usb: Add a uclass for USB controllers

Vivek Gautam
2015-03-09T06:32:11Z

Hi,

On Mon, Mar 9, 2015 at 11:50 AM, Vivek Gautam <gautamvivek1987@gmail.com> wrote:

Sorry i was in the middle of adding comment and the message just got sent.
I will add other comments here.

> Hi Simon,
>
>
> On Sat, Jan 31, 2015 at 12:34 AM, Simon Glass <sjg@chromium.org> wrote:
>> Add a uclass that can represent a USB controller. For now we do not create
>> devices for things attached to the controller.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
> Please find my comments inline below.
>
>>
>> drivers/usb/host/Makefile | 2 +
>> drivers/usb/host/usb-uclass.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>> include/dm/uclass-id.h | 1 +
>> 3 files changed, 230 insertions(+)
>> create mode 100644 drivers/usb/host/usb-uclass.c
>>
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index c11b551..d0b890a 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -5,6 +5,8 @@
>> # SPDX-License-Identifier: GPL-2.0+
>> #
>>
>> +obj-$(CONFIG_DM_USB) += usb-uclass.o
>> +
>> # ohci
>> obj-$(CONFIG_USB_OHCI_NEW) += ohci-hcd.o
>> obj-$(CONFIG_USB_ATMEL) += ohci-at91.o
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> new file mode 100644
>> index 0000000..86564db
>> --- /dev/null
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * (C) Copyright 2015 Google, Inc
>> + * Written by Simon Glass <sjg@chromium.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <usb.h>
>> +#include <dm/device-internal.h>
>> +
>> +static bool usb_started; /* flag for the started/stopped USB status */
>> +static bool asynch_allowed;
>> +
>> +int usb_disable_asynch(int disable)
>> +{
>> + int old_value = asynch_allowed;
>> +
>> + asynch_allowed = !disable;
>> + return old_value;
>> +}
>> +
>> +int submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer,
>> + int length, int interval)
>> +{
>> + struct udevice *dev = udev->controller_dev;
>> + struct dm_usb_ops *ops = usb_get_ops(dev);
>> +
>> + if (!ops->control)
>> + return -ENOSYS;
>> +
>> + return ops->interrupt(dev, udev, pipe, buffer, length, interval);
>> +}
>> +
>> +int submit_control_msg(struct usb_device *udev, unsigned long pipe,
>> + void *buffer, int length, struct devrequest *setup)
>> +{
>> + struct udevice *dev = udev->controller_dev;
>> + struct dm_usb_ops *ops = usb_get_ops(dev);
>> +
>> + if (!ops->control)
>> + return -ENOSYS;
>> +
>> + return ops->control(dev, udev, pipe, buffer, length, setup);
>> +}
>> +
>> +int submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer,
>> + int length)
>> +{
>> + struct udevice *dev = udev->controller_dev;
>> + struct dm_usb_ops *ops = usb_get_ops(dev);
>> +
>> + if (!ops->control)
>> + return -ENOSYS;
>> +
>> + return ops->bulk(dev, udev, pipe, buffer, length);
>> +}
>> +
>> +int usb_alloc_device(struct usb_device *udev)
>> +{
>> + struct udevice *dev = udev->controller_dev;
>> + struct dm_usb_ops *ops = usb_get_ops(dev);
>> +
>> + if (!ops->alloc_device)
>> + return -ENOSYS;
>> +
>> + return ops->alloc_device(dev, udev);
>> +}
>> +
>> +int usb_stop(void)
>> +{

i think you are already planning to add "remove()" call for host
controller here,
alongwith freeing the device, usb_free_device().

>> + return 0;
>> +}
>> +
>> +int usb_init(void)
>> +{
>> + int controllers_initialized = 0;
>> + struct usb_device *udev;
>> + struct udevice *dev;
>> + struct uclass *uc;
>> + int count = 0;
>> + int ret;
>> +
>> + asynch_allowed = 1;
>
> you may still want to do a usb_hub_reset() like usb_init()
> in common/usb.c does ?
>
> Also make all devices unknown initially
i think this may not be required.

>
>> + ret = uclass_get(UCLASS_USB, &uc);
>> + if (ret)
>> + return ret;
>
> nit: just add an extra line here.
>
>> + uclass_foreach_dev(dev, uc) {
>> + struct dm_usb_info *usb;
>> +
>> + /* init low_level USB */
>> + count++;
>> + printf("USB");
>> + ret = device_probe(dev);
>> + printf("%d: ", dev->seq);
>> + if (ret == -ENODEV) { /* No such device. */
>> + puts("Port not available.\n");
>> + controllers_initialized++;
>> + continue;
>> + }
>> +
>> + if (ret) { /* Other error. */
>> + puts("probe failed\n");
>> + continue;
>> + }
>> + /*
>> + * lowlevel init is OK, now scan the bus for devices
>> + * i.e. search HUBs and configure them
>> + */
>> + controllers_initialized++;
>> + printf("scanning bus %d for devices... ", dev->seq);
>> + ret = usb_alloc_new_device(dev, &udev);
>> + /*
>> + * device 0 is always present
>> + * (root hub, so let it analyze)
>> + */
>> + if (!ret)
>> + usb_new_device(udev);
>> +
>> + usb = dev_get_uclass_priv(dev);
>> + if (!usb->dev_index)
>> + printf("No USB Device found\n");
>> + else
>> + printf("%d USB Device(s) found\n", usb->dev_index);
>> +
>> + usb_started = true;
>> + }
>> +
>> + debug("scan end\n");
>> + /* if we were not able to find at least one working bus, bail out */
>> + if (!count)
>> + printf("No controllers found\n");
>> + else if (controllers_initialized == 0)
>> + printf("USB error: all controllers failed lowlevel init\n");
>> +
>> + return usb_started ? 0 : -1;
>> +}
>> +
>> +#ifdef CONFIG_MUSB_HOST
>> +int usb_reset_root_port(void)
>> +{
>> + return -ENOSYS;
>> +}
>> +#endif
>> +
>> +int usb_alloc_new_device(struct udevice *controller,
>> + struct usb_device **devp)
>> +{
>> + struct dm_usb_info *usb = dev_get_uclass_priv(controller);
>> + struct usb_device *udev;
>> +
>> + int i;
>> +
>> + debug("New Device %d\n", usb->dev_index);
>> + if (usb->dev_index == USB_MAX_DEVICE) {
>> + printf("ERROR, too many USB Devices, max=%d\n", USB_MAX_DEVICE);
>> + return -ENOSPC;
>> + }
>> + /* default Address is 0, real addresses start with 1 */
>> + udev = &usb->usb_dev[usb->dev_index++];
>> + udev->devnum = usb->dev_index;
>> + udev->maxchild = 0;
>> + for (i = 0; i < USB_MAXCHILDREN; i++)
>> + udev->children[i] = NULL;
>> + udev->parent = NULL;
>> + udev->controller_dev = controller;
>> + udev->controller = usb->controller;
>> + debug("%s: udev=%p\n", __func__, udev);
>> +
>> + *devp = udev;
>> + return 0;
>> +}
>> +
>> +struct usb_device *usb_get_dev_index(struct udevice *controller, int index)
>> +{
>> + struct dm_usb_info *usb = dev_get_uclass_priv(controller);
>> + struct usb_device *udev;
>> +
>> + if (index < 0 || index >= USB_MAX_DEVICE)
>> + return NULL;
>> + udev = &usb->usb_dev[index];
>> +
>> + return udev->controller ? udev : NULL;
>> +}
>> +
>> +/*
>> +static int usb_child_pre_probe(struct udevice *dev)
>> +{
>> + struct usb_device *udev = dev_get_parentdata(dev);
>> +
>> + udev->controller = dev;
>> +
>> + return 0;
>> +}
>> +*/
>> +/*
>> + * Free the newly created device node.
>> + * Called in error cases where configuring a newly attached
>> + * device fails for some reason.
>> + */
>> +void usb_free_device(struct udevice *controller)
>> +{
>> + struct dm_usb_info *usb = dev_get_uclass_priv(controller);
>> +
>> + usb->dev_index--;
>> + debug("Freeing device node: %d\n", usb->dev_index);
>> + memset(&usb->usb_dev[usb->dev_index], '\0', sizeof(struct usb_device));
>> + usb->usb_dev[usb->dev_index].devnum = -1;
>> +}
>> +
>> +UCLASS_DRIVER(usb) = {
>> + .id = UCLASS_USB,
>> + .name = "usb",
>> + .flags = DM_UC_FLAG_SEQ_ALIAS,
>> +/*
>> + .child_pre_probe = usb_child_pre_probe,
>> + .post_probe = i2c_post_probe,
>> +*/
>> + .per_device_auto_alloc_size = sizeof(struct dm_usb_info),
>> +/*
>> + .per_child_auto_alloc_size = sizeof(struct usb_device),
>> + .per_child_platdata_auto_alloc_size = sizeof(struct dm_i2c_chip),
>> + .child_post_bind = i2c_child_post_bind,
>> +*/
>> +};
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 91bb90d..baab810 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -34,6 +34,7 @@ enum uclass_id {
>> UCLASS_I2C_GENERIC, /* Generic I2C device */
>> UCLASS_I2C_EEPROM, /* I2C EEPROM device */
>> UCLASS_MOD_EXP, /* RSA Mod Exp device */
>> + UCLASS_USB, /* USB bus */
>>
>> UCLASS_COUNT,
>> UCLASS_INVALID = -1,
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

Rest looks fine to me.

--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India