[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Vivek Gautam]

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

-- 
1.7.10.4

[U-Boot] [PATCH 1/2] usb: Rename usb_submit_int_msg() API to usb_int_msg()

[Vivek Gautam]

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);
-- 
1.7.10.4

[U-Boot] [PATCH 2/2] RFC: usb: host: Introduce host translational layer

[Vivek Gautam]

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
-- 
1.7.10.4

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Marek Vasut]

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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Vivek Gautam]

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.


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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Marek Vasut]

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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Simon Glass]

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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Marek Vasut]

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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Simon Glass]

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] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Vivek Gautam]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Vivek Gautam]

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?




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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Marek Vasut]

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

[U-Boot] [PATCH RFC 0/2] usb: host: Add a wrapper layer for mutiple host support

[Simon Glass]

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