[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support
Hi Marek,
It's been very long since we had discussion for introducing the wrapper layer to enable using multiple usb controller types. Its been unfortunate that i had been busy with other tasks, and couldn't look into this. Now that i got sometime, i have prepared a simple RFC patch which right now supports APIs translation for submit_control_msg(), submit_bulk_msg(), submit_int_msg(), and usb_lowlevel_init() as well as usb_lowlevel_stop(). This was the simplest approach that could differentiate between controller types.
I had thought of another approach too, wherein there's a 'list' passed by the usb core layer, which would be filled with 'host_controller_drv' structure, that would contain information about the driver. And then each host controller driver will register certain callbacks that can be called from the upper layers. If you say i will send an RFC for this approach.
Let me know you comments on what you think of current approach.
Thanks Vivek
Vivek Gautam (2): usb: Rename usb_submit_int_msg() API to usb_int_msg() RFC: usb: host: Introduce host translational layer
common/usb.c | 12 +++-- common/usb_kbd.c | 6 +-- common/usb_storage.c | 2 +- drivers/usb/host/Makefile | 2 + drivers/usb/host/ehci-hcd.c | 15 +++--- drivers/usb/host/host_trans.c | 102 +++++++++++++++++++++++++++++++++++++++++ drivers/usb/host/host_trans.h | 42 +++++++++++++++++ drivers/usb/host/xhci.c | 11 +++-- include/usb.h | 17 ++++--- 9 files changed, 181 insertions(+), 28 deletions(-) create mode 100644 drivers/usb/host/host_trans.c create mode 100644 drivers/usb/host/host_trans.h
Until yet usb_**_msg() APIs don't contain the string 'submit'. Rename it to make things uniform. This is also helping while adding a host translational layer wherein we are using usb_submit_**_msg string to name APIs.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/usb.c | 4 ++-- common/usb_kbd.c | 6 +++--- common/usb_storage.c | 2 +- include/usb.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 60daa10..5dd09aa 100644 --- a/common/usb.c +++ b/common/usb.c @@ -155,10 +155,10 @@ int usb_disable_asynch(int disable) /* * submits an Interrupt Message */ -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval) { - return submit_int_msg(dev, pipe, buffer, transfer_len, interval); + return usb_submit_int_msg(dev, pipe, buffer, transfer_len, interval); }
/* diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 0b77c16..d9802b0 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -136,7 +136,7 @@ void usb_kbd_generic_poll(void)
/* Submit a interrupt transfer request */ maxp = usb_maxpacket(usb_kbd_dev, pipe); - usb_submit_int_msg(usb_kbd_dev, pipe, data->new, + usb_int_msg(usb_kbd_dev, pipe, data->new, min(maxp, USB_KBD_BOOT_REPORT_SIZE), ep->bInterval); } @@ -343,7 +343,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev)
/* Submit a interrupt transfer request */ maxp = usb_maxpacket(dev, pipe); - usb_submit_int_msg(dev, pipe, &data->new[0], + usb_int_msg(dev, pipe, &data->new[0], min(maxp, USB_KBD_BOOT_REPORT_SIZE), ep->bInterval);
@@ -472,7 +472,7 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE, 0);
debug("USB KBD: enable interrupt pipe...\n"); - if (usb_submit_int_msg(dev, pipe, data->new, + if (usb_int_msg(dev, pipe, data->new, min(maxp, USB_KBD_BOOT_REPORT_SIZE), ep->bInterval) < 0) { printf("Failed to get keyboard state from device %04x:%04x\n", diff --git a/common/usb_storage.c b/common/usb_storage.c index 6ac358d..91a8a86 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -598,7 +598,7 @@ static int usb_stor_CBI_get_status(ccb *srb, struct us_data *us) int timeout;
us->ip_wanted = 1; - submit_int_msg(us->pusb_dev, us->irqpipe, + usb_submit_int_msg(us->pusb_dev, us->irqpipe, (void *) &us->ip_data, us->irqmaxp, us->irqinterval); timeout = 1000; while (timeout--) { diff --git a/include/usb.h b/include/usb.h index d9fedee..30fc890 100644 --- a/include/usb.h +++ b/include/usb.h @@ -234,7 +234,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, void *data, unsigned short size, int timeout); int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout); -int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, +int usb_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len, int interval); int usb_disable_asynch(int disable); int usb_maxpacket(struct usb_device *dev, unsigned long pipe);
Introduce a wrapper layer to translate submit_**_msg() APIs as well as usb_lowlevel_**() APIs so as to enable using mutiple controller types viz. OHCI/EHCI/XHCI.
This is still WIP, with support for APIs translation for EHCI and XHCI only.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com --- common/usb.c | 8 ++-- drivers/usb/host/Makefile | 2 + drivers/usb/host/ehci-hcd.c | 15 +++--- drivers/usb/host/host_trans.c | 102 +++++++++++++++++++++++++++++++++++++++++ drivers/usb/host/host_trans.h | 42 +++++++++++++++++ drivers/usb/host/xhci.c | 11 +++-- include/usb.h | 15 +++--- 7 files changed, 174 insertions(+), 21 deletions(-) create mode 100644 drivers/usb/host/host_trans.c create mode 100644 drivers/usb/host/host_trans.h
diff --git a/common/usb.c b/common/usb.c index 5dd09aa..ae483d0 100644 --- a/common/usb.c +++ b/common/usb.c @@ -58,6 +58,7 @@ char usb_started; /* flag for the started/stopped USB status */ int usb_init(void) { void *ctrl; + int ctrl_type; struct usb_device *dev; int i, start_index = 0;
@@ -75,7 +76,7 @@ int usb_init(void) for (i = 0; i < CONFIG_USB_MAX_CONTROLLER_COUNT; i++) { /* init low_level USB */ printf("USB%d: ", i); - if (usb_lowlevel_init(i, USB_INIT_HOST, &ctrl)) { + if (usb_lowlevel_init(i, USB_INIT_HOST, &ctrl, &ctrl_type)) { puts("lowlevel init failed\n"); continue; } @@ -86,6 +87,7 @@ int usb_init(void) start_index = dev_index; printf("scanning bus %d for devices... ", i); dev = usb_alloc_new_device(ctrl); + dev->ctrl_type = ctrl_type; /* * device 0 is always present * (root hub, so let it analyze) @@ -193,7 +195,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, request, requesttype, value, index, size); dev->status = USB_ST_NOT_PROC; /*not yet processed */
- if (submit_control_msg(dev, pipe, data, size, setup_packet) < 0) + if (usb_submit_control_msg(dev, pipe, data, size, setup_packet) < 0) return -1; if (timeout == 0) return (int)size; @@ -226,7 +228,7 @@ int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, if (len < 0) return -1; dev->status = USB_ST_NOT_PROC; /*not yet processed */ - if (submit_bulk_msg(dev, pipe, data, len) < 0) + if (usb_submit_bulk_msg(dev, pipe, data, len) < 0) return -1; while (timeout--) { if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC)) diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 7211c6a..9c6eb84 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -43,3 +43,5 @@ obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o obj-$(CONFIG_USB_XHCI) += xhci.o xhci-mem.o xhci-ring.o obj-$(CONFIG_USB_XHCI_EXYNOS) += xhci-exynos5.o obj-$(CONFIG_USB_XHCI_OMAP) += xhci-omap.o + +obj-$(CONFIG_USB_HOST_TRANS) += host_trans.o diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index eaf5913..460c509 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -31,6 +31,7 @@ #include <linux/compiler.h>
#include "ehci.h" +#include "host_trans.h"
#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 @@ -917,13 +918,13 @@ unknown: return -1; }
-int usb_lowlevel_stop(int index) +int ehci_lowlevel_stop(int index) { ehci_shutdown(&ehcic[index]); return ehci_hcd_stop(index); }
-int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) +int ehci_lowlevel_init(int index, enum usb_init_type init, void **controller) { uint32_t reg; uint32_t cmd; @@ -1052,8 +1053,8 @@ done: }
int -submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int length) +ehci_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length) {
if (usb_pipetype(pipe) != PIPE_BULK) { @@ -1064,8 +1065,8 @@ submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, }
int -submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int length, struct devrequest *setup) +ehci_submit_control_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length, struct devrequest *setup) { struct ehci_ctrl *ctrl = dev->controller;
@@ -1348,7 +1349,7 @@ out: }
int -submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, +ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int length, int interval) { void *backbuffer; diff --git a/drivers/usb/host/host_trans.c b/drivers/usb/host/host_trans.c new file mode 100644 index 0000000..701faa2 --- /dev/null +++ b/drivers/usb/host/host_trans.c @@ -0,0 +1,102 @@ +/* + * USB HOST Controller translation layer + * + * Provides a smooth translation from usb_*() APIs from + * core usb drivers to controller drivers, enabling simultaneous + * use of different types of controller at once. + * + * Copyright (C) 2014 Samsung Electronics Co.Ltd + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include "host_trans.h" + +static int ehci_count; +static int xhci_count; + +int usb_lowlevel_init(int index, enum usb_init_type init, void **controller, + int *ctrl_type) +{ + if (index >= CONFIG_USB_MAX_CONTROLLER_COUNT) + return 0; + + /* first enumerate available EHCI controllers */ + if (ehci_count < CONFIG_USB_MAX_EHCI_COUNT) { + if (ehci_lowlevel_init) { + ehci_count++; + *ctrl_type = USB_HOST_EHCI; + /* init max number of ehci controllers */ + return ehci_lowlevel_init(ehci_count, init, controller); + } + } + + if (xhci_count < CONFIG_USB_MAX_XHCI_COUNT) { + if (xhci_lowlevel_init) { + xhci_count++; + *ctrl_type = USB_HOST_XHCI; + /* init max number of xhci controllers */ + return xhci_lowlevel_init(index, init, controller); + } + } + + return -1; +} + +int usb_lowlevel_stop(int index) +{ + if (index >= CONFIG_USB_MAX_CONTROLLER_COUNT) + return 0; + + ehci_count = xhci_count = 0; + + if (ehci_count < CONFIG_USB_MAX_EHCI_COUNT) { + if (ehci_lowlevel_init) { + ehci_count++; + return ehci_lowlevel_stop(index); + } + } + + if (xhci_count < CONFIG_USB_MAX_XHCI_COUNT) { + if (xhci_lowlevel_init) { + xhci_count++; + return xhci_lowlevel_stop(index); + } + } + + return -1; +} + +int usb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int transfer_len) +{ + if (dev->ctrl_type == USB_HOST_EHCI) + return ehci_submit_bulk_msg(dev, pipe, buffer, transfer_len); + + if (dev->ctrl_type == USB_HOST_XHCI) + return xhci_submit_bulk_msg(dev, pipe, buffer, transfer_len); +} + +int usb_submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, + int transfer_len, struct devrequest *setup) +{ + if (dev->ctrl_type == USB_HOST_EHCI) + return ehci_submit_control_msg(dev, pipe, buffer, + transfer_len, setup); + + if (dev->ctrl_type == USB_HOST_EHCI) + return xhci_submit_control_msg(dev, pipe, buffer, + transfer_len, setup); +} + +int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, + int transfer_len, int interval) +{ + if (dev->ctrl_type == USB_HOST_EHCI) + return ehci_submit_int_msg(dev, pipe, buffer, + transfer_len, interval); + + if (dev->ctrl_type == USB_HOST_XHCI) + return xhci_submit_int_msg(dev, pipe, buffer, + transfer_len, interval); +} diff --git a/drivers/usb/host/host_trans.h b/drivers/usb/host/host_trans.h new file mode 100644 index 0000000..90bcc92 --- /dev/null +++ b/drivers/usb/host/host_trans.h @@ -0,0 +1,42 @@ + +#ifndef _HOST_TRANS_H_ +#define _HOST_TRANS_H_ + +#include <common.h> +#include <usb.h> +#include <linux/compat.h> + +#ifndef CONFIG_USB_MAX_EHCI_COUNT +#define CONFIG_USB_MAX_EHCI_COUNT 1 +#endif + +#ifndef CONFIG_USB_MAX_XHCI_COUNT +#define CONFIG_USB_MAX_XHCI_COUNT 1 +#endif + +enum usb_ctrl_type { + USB_HOST_OHCI, + USB_HOST_UHCI, + USB_HOST_EHCI, + USB_HOST_XHCI, +}; + +int ehci_submit_int_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length, int interval); +int ehci_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length); +int ehci_submit_control_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int length, struct devrequest *setup); +int ehci_lowlevel_init(int index, enum usb_init_type init, void **controller); +int ehci_lowlevel_stop(int index); + +int xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe, + void *buffer, int length, int interval); +int xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe, + void *buffer, int length); +int xhci_submit_control_msg(struct usb_device *udev, unsigned long pipe, + void *buffer, int length, struct devrequest *setup); +int xhci_lowlevel_init(int index, enum usb_init_type init, void **controller); +int xhci_lowlevel_stop(int index); + +#endif /* _HOST_TRANS_H_ */ diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d1c2e5c..2e7e3f0 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -29,6 +29,7 @@ #include <asm/unaligned.h> #include <asm-generic/errno.h> #include "xhci.h" +#include "host_trans.h"
#ifndef CONFIG_USB_MAX_CONTROLLER_COUNT #define CONFIG_USB_MAX_CONTROLLER_COUNT 1 @@ -859,7 +860,7 @@ unknown: * @return 0 */ int -submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, +xhci_submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, int length, int interval) { /* @@ -879,7 +880,7 @@ submit_int_msg(struct usb_device *udev, unsigned long pipe, void *buffer, * @return returns 0 if successful else -1 on failure */ int -submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, +xhci_submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, int length) { if (usb_pipetype(pipe) != PIPE_BULK) { @@ -901,7 +902,7 @@ submit_bulk_msg(struct usb_device *udev, unsigned long pipe, void *buffer, * @return returns 0 if successful else -1 on failure */ int -submit_control_msg(struct usb_device *udev, unsigned long pipe, void *buffer, +xhci_submit_control_msg(struct usb_device *udev, unsigned long pipe, void *buffer, int length, struct devrequest *setup) { struct xhci_ctrl *ctrl = udev->controller; @@ -936,7 +937,7 @@ submit_control_msg(struct usb_device *udev, unsigned long pipe, void *buffer, * @param index index to the host controller data structure * @return pointer to the intialised controller */ -int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) +int xhci_lowlevel_init(int index, enum usb_init_type init, void **controller) { uint32_t val; uint32_t val2; @@ -1009,7 +1010,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) * @param index index to the host controller data structure * @return none */ -int usb_lowlevel_stop(int index) +int xhci_lowlevel_stop(int index) { struct xhci_ctrl *ctrl = (xhcic + index); u32 temp; diff --git a/include/usb.h b/include/usb.h index 30fc890..212a34d 100644 --- a/include/usb.h +++ b/include/usb.h @@ -125,6 +125,7 @@ struct usb_device { struct usb_device *children[USB_MAXCHILDREN];
void *controller; /* hardware controller private data */ + int ctrl_type; /* controller type - OHCI/EHCI/XHCI */ /* slot_id - for xHCI enabled devices */ unsigned int slot_id; }; @@ -152,15 +153,17 @@ enum usb_init_type { defined(CONFIG_USB_MUSB_DSPS) || defined(CONFIG_USB_MUSB_AM35X) || \ defined(CONFIG_USB_MUSB_OMAP2PLUS) || defined(CONFIG_USB_XHCI)
-int usb_lowlevel_init(int index, enum usb_init_type init, void **controller); +int usb_lowlevel_init(int index, enum usb_init_type init, void **controller, + int *ctrl_type); int usb_lowlevel_stop(int index);
-int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, +int usb_submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int transfer_len); -int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int transfer_len, struct devrequest *setup); -int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int transfer_len, int interval); +int usb_submit_control_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int transfer_len, + struct devrequest *setup); +int usb_submit_int_msg(struct usb_device *dev, unsigned long pipe, + void *buffer, int transfer_len, int interval);
/* Defines */ #define USB_UHCI_VEND_ID 0x8086
On Tuesday, June 24, 2014 at 04:10:20 PM, Vivek Gautam wrote:
Hi Marek,
It's been very long since we had discussion for introducing the wrapper layer to enable using multiple usb controller types. Its been unfortunate that i had been busy with other tasks, and couldn't look into this. Now that i got sometime, i have prepared a simple RFC patch which right now supports APIs translation for submit_control_msg(), submit_bulk_msg(), submit_int_msg(), and usb_lowlevel_init() as well as usb_lowlevel_stop(). This was the simplest approach that could differentiate between controller types.
I had thought of another approach too, wherein there's a 'list' passed by the usb core layer, which would be filled with 'host_controller_drv' structure, that would contain information about the driver. And then each host controller driver will register certain callbacks that can be called from the upper layers. If you say i will send an RFC for this approach.
I think this approach in this patchset will not play well with the driver model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
Best regards, Marek Vasut
Hi Marek,
On Tue, Jun 24, 2014 at 7:56 PM, Marek Vasut marex@denx.de wrote:
On Tuesday, June 24, 2014 at 04:10:20 PM, Vivek Gautam wrote:
Hi Marek,
It's been very long since we had discussion for introducing the wrapper layer to enable using multiple usb controller types. Its been unfortunate that i had been busy with other tasks, and couldn't look into this. Now that i got sometime, i have prepared a simple RFC patch which right now supports APIs translation for submit_control_msg(), submit_bulk_msg(), submit_int_msg(), and usb_lowlevel_init() as well as usb_lowlevel_stop(). This was the simplest approach that could differentiate between controller types.
I had thought of another approach too, wherein there's a 'list' passed by the usb core layer, which would be filled with 'host_controller_drv' structure, that would contain information about the driver. And then each host controller driver will register certain callbacks that can be called from the upper layers. If you say i will send an RFC for this approach.
I think this approach in this patchset will not play well with the driver model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
On Wednesday, June 25, 2014 at 07:11:42 AM, Vivek Gautam wrote:
Hi Marek,
On Tue, Jun 24, 2014 at 7:56 PM, Marek Vasut marex@denx.de wrote:
On Tuesday, June 24, 2014 at 04:10:20 PM, Vivek Gautam wrote:
Hi Marek,
It's been very long since we had discussion for introducing the wrapper layer to enable using multiple usb controller types. Its been unfortunate that i had been busy with other tasks, and couldn't look into this. Now that i got sometime, i have prepared a simple RFC patch which right now supports APIs translation for submit_control_msg(), submit_bulk_msg(), submit_int_msg(), and usb_lowlevel_init() as well as usb_lowlevel_stop(). This was the simplest approach that could differentiate between controller types.
I had thought of another approach too, wherein there's a 'list' passed by the usb core layer, which would be filled with 'host_controller_drv' structure, that would contain information about the driver. And then each host controller driver will register certain callbacks that can be called from the upper layers. If you say i will send an RFC for this approach.
I think this approach in this patchset will not play well with the driver model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Best regards, Marek Vasut
Hi,
On Jun 25, 2014 12:08 AM, "Marek Vasut" marex@denx.de wrote:
On Wednesday, June 25, 2014 at 07:11:42 AM, Vivek Gautam wrote:
Hi Marek,
On Tue, Jun 24, 2014 at 7:56 PM, Marek Vasut marex@denx.de wrote:
On Tuesday, June 24, 2014 at 04:10:20 PM, Vivek Gautam wrote:
Hi Marek,
It's been very long since we had discussion for introducing the
wrapper
layer to enable using multiple usb controller types. Its been unfortunate that i had been busy with other tasks, and
couldn't
look into this. Now that i got sometime, i have prepared a simple RFC patch which
right
now supports APIs translation for submit_control_msg(), submit_bulk_msg(), submit_int_msg(), and usb_lowlevel_init() as well
as
usb_lowlevel_stop(). This was the simplest approach that could differentiate between controller types.
I had thought of another approach too, wherein there's a 'list'
passed
by the usb core layer, which would be filled with
'host_controller_drv'
structure, that would contain information about the driver. And then each host controller driver will register certain callbacks that can
be
called from the upper layers. If you say i will send an RFC for this approach.
I think this approach in this patchset will not play well with the
driver
model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
Regards, Simon
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
Best regards, Marek Vasut
Hi Marek,
On 25 June 2014 02:33, Marek Vasut marex@denx.de wrote:
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
If you add CONFIG_DM_USB as an option, you can then pull in either usb-uclass.c or the old usb code. Since USB is often tied to a board then you can move just that board (or group of boards) to dm.
I am keeping a working tree in u-boot-dm.git which does this for serial, SPI, SPI flash and GPIO. It seems to work fairly well as a technique for keeping both things in the tree in the interim..
As to the size increase, yes it will increase the size, but not that much, and after all, aren't we trying to move the code to dm?
Regards, Simon
On Thu, Jun 26, 2014 at 8:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Marek,
On 25 June 2014 02:33, Marek Vasut marex@denx.de wrote:
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
model. Instead, I'd love to see a mean to instantiate each *HCI controller and have a USB core which would track those instances. The USB core would then be able to call whatever generic ops on those instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
If you add CONFIG_DM_USB as an option, you can then pull in either usb-uclass.c or the old usb code. Since USB is often tied to a board then you can move just that board (or group of boards) to dm.
I am keeping a working tree in u-boot-dm.git which does this for serial, SPI, SPI flash and GPIO. It seems to work fairly well as a technique for keeping both things in the tree in the interim..
As to the size increase, yes it will increase the size, but not that much, and after all, aren't we trying to move the code to dm?
Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Simon, Marek,
On Thu, Jun 26, 2014 at 10:04 AM, Vivek Gautam gautam.vivek@samsung.com wrote:
sorry for spamming, the earlier message got sent by mistake.
On Thu, Jun 26, 2014 at 8:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Marek,
On 25 June 2014 02:33, Marek Vasut marex@denx.de wrote:
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
> model. Instead, I'd love to see a mean to instantiate each *HCI > controller and have a USB core which would track those instances. The > USB core would then be able to call whatever generic ops on those > instances as needed. Does that make sense please ?
True, i understand your point here. I think the second approach i was talking of, goes in this direction. I think i could not put it well in words there.
I will prepare an RFC patch for that, and post it as soon as its ready, so that you can have a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
If you add CONFIG_DM_USB as an option, you can then pull in either usb-uclass.c or the old usb code. Since USB is often tied to a board then you can move just that board (or group of boards) to dm.
I am keeping a working tree in u-boot-dm.git which does this for serial, SPI, SPI flash and GPIO. It seems to work fairly well as a technique for keeping both things in the tree in the interim..
Ok, so i am having a look at the u-boot-dm tree, and also going through the documentation for driver-model. The driver-model looks a promising choice at the moment keeping in mind that later we would need to move to it anyways.
I will try understanding the things and raise a flag in case something is not clear.
As to the size increase, yes it will increase the size, but not that much, and after all, aren't we trying to move the code to dm?
On Thursday, June 26, 2014 at 06:46:11 AM, Vivek Gautam wrote:
Hi Simon, Marek,
On Thu, Jun 26, 2014 at 10:04 AM, Vivek Gautam gautam.vivek@samsung.com wrote:
sorry for spamming, the earlier message got sent by mistake.
On Thu, Jun 26, 2014 at 8:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Marek,
On 25 June 2014 02:33, Marek Vasut marex@denx.de wrote:
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
> > model. Instead, I'd love to see a mean to instantiate each *HCI > > controller and have a USB core which would track those > > instances. The USB core would then be able to call whatever > > generic ops on those instances as needed. Does that make sense > > please ? > > True, i understand your point here. I think the second approach i > was talking of, goes in this direction. > I think i could not put it well in words there. > > I will prepare an RFC patch for that, and post it as soon as its > ready, so that you can have > a look.
Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
If you add CONFIG_DM_USB as an option, you can then pull in either usb-uclass.c or the old usb code. Since USB is often tied to a board then you can move just that board (or group of boards) to dm.
I am keeping a working tree in u-boot-dm.git which does this for serial, SPI, SPI flash and GPIO. It seems to work fairly well as a technique for keeping both things in the tree in the interim..
Ok, so i am having a look at the u-boot-dm tree, and also going through the documentation for driver-model. The driver-model looks a promising choice at the moment keeping in mind that later we would need to move to it anyways.
I will try understanding the things and raise a flag in case something is not clear.
Even better, if I don't have to do this myself :) I'm really glad to see how many people put effort into the USB and how things are coming together nicely. Thank you guys!
Best regards, Marek Vasut
Hi,
On 26 June 2014 03:21, Marek Vasut marex@denx.de wrote:
On Thursday, June 26, 2014 at 06:46:11 AM, Vivek Gautam wrote:
Hi Simon, Marek,
On Thu, Jun 26, 2014 at 10:04 AM, Vivek Gautam gautam.vivek@samsung.com wrote:
sorry for spamming, the earlier message got sent by mistake.
On Thu, Jun 26, 2014 at 8:00 AM, Simon Glass sjg@chromium.org wrote:
Hi Marek,
On 25 June 2014 02:33, Marek Vasut marex@denx.de wrote:
On Wednesday, June 25, 2014 at 08:27:39 AM, Simon Glass wrote: [...]
> > > model. Instead, I'd love to see a mean to instantiate each *HCI > > > controller and have a USB core which would track those > > > instances. The USB core would then be able to call whatever > > > generic ops on those instances as needed. Does that make sense > > > please ? > > > > True, i understand your point here. I think the second approach i > > was talking of, goes in this direction. > > I think i could not put it well in words there. > > > > I will prepare an RFC patch for that, and post it as soon as its > > ready, so that you can have > > a look. > > Ah, this would be so very appreciated! Thank you!
Should we consider just going straight for driver model?
I was thinking about that, but I'm worried it might break USB support on some platforms. Also, the size of U-Boot will grow on many platforms, right?
What do you think ?
If you add CONFIG_DM_USB as an option, you can then pull in either usb-uclass.c or the old usb code. Since USB is often tied to a board then you can move just that board (or group of boards) to dm.
I am keeping a working tree in u-boot-dm.git which does this for serial, SPI, SPI flash and GPIO. It seems to work fairly well as a technique for keeping both things in the tree in the interim..
Ok, so i am having a look at the u-boot-dm tree, and also going through the documentation for driver-model. The driver-model looks a promising choice at the moment keeping in mind that later we would need to move to it anyways.
I will try understanding the things and raise a flag in case something is not clear.
Even better, if I don't have to do this myself :) I'm really glad to see how many people put effort into the USB and how things are coming together nicely. Thank you guys!
Please note I have updated the 'working' branch. A parent device (such as a SPI bus or USB bus) can now have private data for each of its children. This was useful for SPI and may be useful for USB.
Regards, Simon
participants (3)
-
Marek Vasut -
Simon Glass -
Vivek Gautam