Hi,
On Mon, Mar 9, 2015 at 11:50 AM, Vivek Gautam <[email protected]> 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 <[email protected]> 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 <[email protected]> >> --- > > 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 <[email protected]> >> + * >> + * 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 >> [email protected] >> http://lists.denx.de/mailman/listinfo/u-boot Rest looks fine to me. -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
