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

Vivek Gautam gautamvivek1987 at gmail.com
Mon Mar 9 07:32:11 CET 2015


Hi,


On Mon, Mar 9, 2015 at 11:50 AM, Vivek Gautam <gautamvivek1987 at 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 at 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 at 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 at 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


More information about the U-Boot mailing list