Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
On Fri, 14 Nov 2014, Sean O. Stalley wrote: To summarize the spec: MA USB groups a host connected devices into MA service sets (MSS). The architectural limit is 254 MA devices per MSS. If the host needs to connect more devices than that, It can start a new MSS and connect to 254 more MA devices. Is supporting up to 254 devices on one machine sufficient? It's probably more than enough. Would it make sense (and does the usb stack support) having 254 root ports on one host controller? If so, we could make our host controller instance have 254 ports. I'm guessing the hub driver may have a problem with this (especially for superspeed). The USB stack is likely to have problems if there are more than 31 ports on any hub. If that doesn't make sense (or isn't supported), we can have 1 host controller instance per MA device. Would that be preferred? It doesn't make much difference. Whatever you think will be easier to support. You might check and see how usbip does it. Also, I noticed that your patch adds a new bus type for these MA host controllers. It really seems like overkill to have a whole new bus type if there's only going to be one device on it. The bus was added when we were quickly trying to replace the platform device code. It's probably not the right thing to do. I'm still not sure why we can't make our hcd a platform device, especially since dummy_hcd the usbip's hcd are both platform devices. A platform device is the right way to go. Alan Stern ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
On Wed, Nov 12, 2014 at 05:03:18PM -0500, Alan Stern wrote: On Wed, 12 Nov 2014, Sean O. Stalley wrote: Our plan to support multiple MA devices is to have them all connected to the same virtual host controller, so only 1 would be needed. Would you prefer we have 1 host controller instance per MA device? We are definitely open to suggestions on how this should be architected. I haven't read the MA USB spec, so I don't know how it's intended to work. Still, what happens if you create a virtual host controller with, say, 16 ports, and then someone wants to connect a 17th MA device? To summarize the spec: MA USB groups a host connected devices into MA service sets (MSS). The architectural limit is 254 MA devices per MSS. If the host needs to connect more devices than that, It can start a new MSS and connect to 254 more MA devices. Is supporting up to 254 devices on one machine sufficient? Would it make sense (and does the usb stack support) having 254 root ports on one host controller? If so, we could make our host controller instance have 254 ports. I'm guessing the hub driver may have a problem with this (especially for superspeed). If that doesn't make sense (or isn't supported), we can have 1 host controller instance per MA device. Would that be preferred? Also, I noticed that your patch adds a new bus type for these MA host controllers. It really seems like overkill to have a whole new bus type if there's only going to be one device on it. The bus was added when we were quickly trying to replace the platform device code. It's probably not the right thing to do. I'm still not sure why we can't make our hcd a platform device, especially since dummy_hcd the usbip's hcd are both platform devices. If we get rid of these locks, endpoints can't run simultaneously. MA USB IN endpoints have to copy data, which could take a while. I don't know what you mean by run simultaneously. Certainly multiple network packets can be transmitted and received concurrently even if you use a single spinlock, since your locking won't affect the networking subsystem. I meant we couldn't have 2 threads in our driver. With one lock, One thread would always have to wait for the other, even though they could be working on 2 different endpoints doing completely independent tasks. Couldn't this cause a bottleneck? Probably not enough to matter. After all, the other host controller drivers rely on a single spinlock. And if it did matter, you could drop the spinlock while copying the data. Good point. We can cut our driver down to using 1 lock. If we find that only having 1 spinlock does cause a bottleneck, we can deal with it then. Thanks, Sean ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
Thanks for reviewing. My responses are inline.
Greg has asked that we clean up this code internally before we
send out another patchset to the mailing list. I will address
the issues you pointed out, but it may be a while before you see
another patchset.
Thanks Again,
Sean
On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote:
On Mon, 10 Nov 2014, Stephanie Wallick wrote:
+static struct mausb_hcd mhcd;
Only one statically-allocated structure? What if somebody wants to
have more than one of these things in their system?
Our plan to support multiple MA devices is to have them all connected
to the same virtual host controller, so only 1 would be needed.
Would you prefer we have 1 host controller instance per MA device?
We are definitely open to suggestions on how this should be architected.
+/**
+ * @maurb: Media agnostic structure with URB to release.
+ * @status:Status for URB that is getting released.
+ *
+ * Removes an URB from the queue, deletes the media agnostic information in
+ * the urb, and gives the URB back to the HCD. Caller must be holding the
+ * driver's spinlock.
+ */
+void mausb_unlink_giveback_urb(struct mausb_urb *maurb, int status)
+{
+ struct urb *urb;
+ struct usb_hcd *hcd;
+ struct api_context *ctx = NULL;
+ unsigned long irq_flags;
+
+ hcd = mausb_hcd_to_usb_hcd(mhcd);
+
+ spin_lock_irqsave(mhcd.giveback_lock, irq_flags);
Why do you need multiple spinlocks? Isn't one lock sufficient?
We will simplify the locking scheme before resubmitting.
I think it might be worthwhile to have a per-endpoint lock, see below.
+ if (!maurb) {
+ mausb_err(mhcd, %s: no maurb\n, __func__);
+ spin_unlock_irqrestore(mhcd.giveback_lock, irq_flags);
+ return;
+ } else {
+ urb = maurb-urb;
+ ctx = urb-context;
+ }
+
+ if (!urb) {
+ mausb_err(mhcd, %s: no urb\n, __func__);
+ mausb_internal_drop_maurb(maurb, mhcd);
+ spin_unlock_irqrestore(mhcd.giveback_lock, irq_flags);
+ return;
+ }
+
+ mausb_dbg(mhcd, %s: returning urb with status %i\n, __func__,
status);
+
+ usb_hcd_unlink_urb_from_ep(hcd, urb);
+ usb_hcd_giveback_urb(hcd, urb, status);
You must not call this function while holding any spinlocks. What happens
if the URB's completion routine tries to resubmit?
This works with our multi-lock scheme, but I will fix when we move to 1 lock.
+
+ /* remove the mausb-specific data */
+ mausb_internal_drop_maurb(maurb, mhcd);
+
+ spin_unlock_irqrestore(mhcd.giveback_lock, irq_flags);
+}
+
+/**
+ * Adds an URB to the endpoint queue then calls the URB handler. URB is
wrapped
+ * in media agnostic structure before being enqueued.
+ */
+static int mausb_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t memflags)
+{
+ int ret = 0;
+ struct mausb_urb*maurb;
+ struct mausb_host_ep*ep;
+ unsigned long irq_flags;
+
+ if (!hcd || !urb) {
+ pr_err(%s: no %s\n, __func__, (hcd ? urb : USB hcd));
+ }
This can never happen. The USB core guarantees it; you don't need
to check.
I will remove this check (along with any other unnecessary checks for things
guaranteed from usbcore).
+ ep = usb_to_ma_endpoint(urb-ep);
+
+ if (!ep) {
+ mausb_err(mhcd, %s: no endpoint\n, __func__);
+ return -EINVAL;
+ }
+
+ if (urb-status != -EINPROGRESS) {
+ mausb_err(mhcd, %s: urb already unlinked, status is %i\n,
+ __func__, urb-status);
+ return urb-status;
+ }
You also don't need to check this.
Will remove.
+ /* If the endpoint isn't activated, we can't enqueue anything. */
+ if (MAUSB_EP_HANDLE_UNASSIGNED == ep-ep_handle_state) {
+ mausb_err(mhcd, %s: endpoint handle unassigned\n, __func__);
+ return -EPIPE;
+ }
+
+ if (USB_SPEED_FULL != urb-dev-speed) /* suppress checks */
+ ep-max_pkt = usb_endpoint_maxp(urb-ep-desc);
What happens to full-speed devices? Don't they have maxpacket values?
+
+ /* initialize the maurb */
+ maurb = mausb_alloc_maurb(ep, memflags);
+ if (!maurb) {
+ mausb_err(mhcd, could not allocate memory for MA USB urb\n);
+ return -ENOMEM;
+ }
+
+ /* set maurb member values */
+ maurb-urb = urb;
+ urb-hcpriv = maurb;
+
+ /* submit urb to hcd and add to endpoint queue */
+ ret = usb_hcd_link_urb_to_ep(hcd, urb);
Read the kerneldoc for this function. You must hold your private
spinlock when you call it.
Will fix this make sure we hold our lock.
+ if (ret 0) {
+ mausb_err(mhcd, urb enqueue failed: error %d\n, ret);
+ usb_hcd_unlink_urb_from_ep(hcd, urb);
Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
On Wed, 12 Nov 2014, Sean O. Stalley wrote: On Tue, Nov 11, 2014 at 10:54:30AM -0500, Alan Stern wrote: On Mon, 10 Nov 2014, Stephanie Wallick wrote: +static struct mausb_hcd mhcd; Only one statically-allocated structure? What if somebody wants to have more than one of these things in their system? Our plan to support multiple MA devices is to have them all connected to the same virtual host controller, so only 1 would be needed. Would you prefer we have 1 host controller instance per MA device? We are definitely open to suggestions on how this should be architected. I haven't read the MA USB spec, so I don't know how it's intended to work. Still, what happens if you create a virtual host controller with, say, 16 ports, and then someone wants to connect a 17th MA device? Also, I noticed that your patch adds a new bus type for these MA host controllers. It really seems like overkill to have a whole new bus type if there's only going to be one device on it. If we get rid of these locks, endpoints can't run simultaneously. MA USB IN endpoints have to copy data, which could take a while. I don't know what you mean by run simultaneously. Certainly multiple network packets can be transmitted and received concurrently even if you use a single spinlock, since your locking won't affect the networking subsystem. Couldn't this cause a bottleneck? Probably not enough to matter. After all, the other host controller drivers rely on a single spinlock. And if it did matter, you could drop the spinlock while copying the data. Alan Stern ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
This is where we interface with the existing USB stack and implement the
functionality of a USB host controller driver. From the host's perspective,
we appear as just another USB host controller. However, instead of passing
traffic along a wired USB bus, the driver hands USB packets off for transport
per Media Agnostic USB protocol.
Signed-off-by: Sean O. Stalley sean.stal...@intel.com
Signed-off-by: Stephanie Wallick stephanie.s.wall...@intel.com
---
drivers/staging/mausb/drivers/mausb_hcd.c | 981 ++
drivers/staging/mausb/drivers/mausb_hcd.h | 184 ++
2 files changed, 1165 insertions(+)
create mode 100755 drivers/staging/mausb/drivers/mausb_hcd.c
create mode 100644 drivers/staging/mausb/drivers/mausb_hcd.h
diff --git a/drivers/staging/mausb/drivers/mausb_hcd.c
b/drivers/staging/mausb/drivers/mausb_hcd.c
new file mode 100755
index 000..b35a62b
--- /dev/null
+++ b/drivers/staging/mausb/drivers/mausb_hcd.c
@@ -0,0 +1,981 @@
+/* Name: mausb_hcd.c
+ * Description: Creates and initializes a virtual USB host controller driver
+ * for the Media Agnostic USB host driver.
+ *
+ * This file is provided under a dual BSD/GPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2014 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * Contact Information:
+ * Sean Stalley, sean.stal...@intel.com
+ * Stephanie Wallick, stephanie.s.wall...@intel.com
+ * 2111 NE 25th Avenue
+ * Hillsboro, Oregon 97124
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2014 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+* Redistributions of source code must retain the above copyright
+ notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+ notice, this list of conditions and the following disclaimer in
+ the documentation and/or other materials provided with the
+ distribution.
+* Neither the name of Intel Corporation nor the names of its
+ contributors may be used to endorse or promote products derived
+ from this software without specific prior written permission.
+
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * AS IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include linux/init.h
+#include linux/wait.h
+#include linux/module.h
+#include linux/usb.h
+#include linux/usb/hcd.h
+#include linux/usb/ch11.h
+#include linux/pm_runtime.h
+#include linux/usb/gadget.h
+#include linux/kthread.h
+#include linux/random.h
+
+#include mausb_hcd.h
+#include mausb_hub.h
+#include mausb_pkt.h
+#include mausb_mem-host.h
+#include mausb_msapi.h
+#include mausb_mgmt.h
+#include mausb_state.h
+#include mausb_tx.h
+
+static int mausb_bus_match(struct device *dev, struct device_driver *drv)
+{
+ if (strncmp(dev-bus-name, drv-name, strlen(drv-name)))
+ return 0; /* no match */
+ else
+ return 1; /* match */
+}
+
+static int mausb_bus_probe(struct device *dev)
+{
+ return mausb_probe(dev);
+}
+
+static int mausb_bus_remove(struct device *dev)
+{
+ return mausb_remove(dev);
+}
+
+static void mausb_dev_release(struct device *dev)
+{
+ /* TODO: if we dynamically allocate anything, free it here */
+}
+
+static struct class*mausb_class;
+
+static struct bus_type mausb_bus_type = {
+ .name = MAUSB_NAME,
+ .match = mausb_bus_match,
+ .probe = mausb_bus_probe,
+ .remove = mausb_bus_remove,
+};
+
+static struct device_driver mhcd_driver = {
+ .name = MAUSB_NAME,
+ .bus= mausb_bus_type,
+ .owner
Re: [V2 PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 10, 2014 at 06:09:32PM -0800, Stephanie Wallick wrote:
+static int mausb_bus_probe(struct device *dev)
+{
+ return mausb_probe(dev);
+}
+
+static int mausb_bus_remove(struct device *dev)
+{
+ return mausb_remove(dev);
+}
Wrapper functions that just call another function? Why?
+static void mausb_dev_release(struct device *dev)
+{
+ /* TODO: if we dynamically allocate anything, free it here */
+}
As per the documentation in the kernel source tree[1], I am now allowed
to mock you mercilessly for thinking that you know more than the kernel,
and are just providing an empty function just to shut it up from
complaining about no release function at all. Did you stop to think
about _why_ the kernel was warning you about this, and how would an
empty function solve anything?
Sorry, I can never accept code that does this in the kernel, even in
staging, which says a lot...
thanks,
greg k-h
[1] Documentation/kobject.txt, line 270
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 04:13:55PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 04:04:42PM -0800, steph wrote:
On Mon, Nov 03, 2014 at 01:21:39PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
[snip]
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
The driver doesn't require platform resources. It looks like a host
controller driver but communicates over the network instead of to
a physical host controller. There is no MA USB-specific hardware.
Should we use a struct device instead of a struct platform_device?
Yes, please make it a virtual device.
Is it OK for our virtual host controller to use struct platform_device?
The other virtual host controllers (usbip/vhci_hcd.c gadget/udc/dummy_hcd.c)
use the platform_device struct. Unless I am missing something, it doesn't look
like the other virtual host controllers use platform resources.
If it is not ok, is there a good example somewhere of a virtual non-platform
device?
Thank You,
Sean O. Stalley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Wed, Nov 05, 2014 at 12:14:33PM -0800, sostalle wrote:
On Mon, Nov 03, 2014 at 04:13:55PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 04:04:42PM -0800, steph wrote:
On Mon, Nov 03, 2014 at 01:21:39PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
[snip]
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
The driver doesn't require platform resources. It looks like a host
controller driver but communicates over the network instead of to
a physical host controller. There is no MA USB-specific hardware.
Should we use a struct device instead of a struct platform_device?
Yes, please make it a virtual device.
Is it OK for our virtual host controller to use struct platform_device?
The other virtual host controllers (usbip/vhci_hcd.c gadget/udc/dummy_hcd.c)
use the platform_device struct. Unless I am missing something, it doesn't look
like the other virtual host controllers use platform resources.
If it is not ok, is there a good example somewhere of a virtual non-platform
device?
If your device is not really a platform device (i.e. no platform
resources), then just create a virtual device with a call to
device_create() and don't pass in a parent pointer. But you need to
create a class for it, which is a pain, but shouldn't be that hard.
Hope this helps,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/10] added media agnostic (MA) USB HCD driver
This is where we interface with the existing USB stack and implement the
functionality of a USB host controller driver. From the host's perspective,
we appear as just another USB host controller. However, instead of passing
traffic along a wired USB bus, the driver hands USB packets off for transport
per Media Agnostic USB protocol.
Signed-off-by: Sean O. Stalley sean.stal...@intel.com
Signed-off-by: Stephanie Wallick stephanie.s.wall...@intel.com
---
drivers/staging/mausb/drivers/mausb_hcd.c | 970 ++
drivers/staging/mausb/drivers/mausb_hcd.h | 171 ++
2 files changed, 1141 insertions(+)
create mode 100755 drivers/staging/mausb/drivers/mausb_hcd.c
create mode 100644 drivers/staging/mausb/drivers/mausb_hcd.h
diff --git a/drivers/staging/mausb/drivers/mausb_hcd.c
b/drivers/staging/mausb/drivers/mausb_hcd.c
new file mode 100755
index 000..03e8f0f
--- /dev/null
+++ b/drivers/staging/mausb/drivers/mausb_hcd.c
@@ -0,0 +1,970 @@
+/* Name: mausb_hcd.c
+ * Description: Creates and initializes a virtual USB host controller driver
+ * for the Media Agnostic USB host driver.
+ *
+ * This file is provided under a dual BSD/GPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2014 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * Contact Information:
+ * Sean Stalley, sean.stal...@intel.com
+ * Stephanie Wallick, stephanie.s.wall...@intel.com
+ * 2111 NE 25th Avenue
+ * Hillsboro, Oregon 97124
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2014 Intel Corporation.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+* Redistributions of source code must retain the above copyright
+ notice, this list of conditions and the following disclaimer.
+* Redistributions in binary form must reproduce the above copyright
+ notice, this list of conditions and the following disclaimer in
+ the documentation and/or other materials provided with the
+ distribution.
+* Neither the name of Intel Corporation nor the names of its
+ contributors may be used to endorse or promote products derived
+ from this software without specific prior written permission.
+
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * AS IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#define DEBUG
+
+#include linux/init.h
+#include linux/wait.h
+#include linux/module.h
+#include linux/usb.h
+#include linux/usb/hcd.h
+#include linux/platform_device.h
+#include linux/usb/ch11.h
+#include linux/pm_runtime.h
+#include linux/usb/gadget.h
+#include linux/kthread.h
+#include linux/random.h
+
+#include mausb_hcd.h
+#include mausb_hub.h
+#include mausb_pkt.h
+#include mausb_mem-host.h
+#include mausb_msapi.h
+#include mausb_mgmt.h
+#include mausb_state.h
+#include mausb_tx.h
+
+static struct platform_device mausb_pdev;
+
+struct api_context {
+ struct completion done;
+ int status;
+};
+
+/*
+ * pointer conversion functions
+ */
+inline struct mausb_hcd *usb_hcd_to_mausb_hcd(struct usb_hcd *hcd)
+{
+ if (usb_hcd_is_primary_hcd(hcd))
+ return *((struct mausb_hcd **) (hcd-hcd_priv));
+ else
+ return *((struct mausb_hcd **) (hcd-primary_hcd-hcd_priv));
+}
+
+inline struct usb_hcd *mausb_hcd_to_usb_hcd(struct mausb_hcd *mhcd)
+{
+ if (mhcd-shared_hcd !usb_hcd_is_primary_hcd(mhcd-shared_hcd))
+ return mhcd-shared_hcd;
+ else
+ return mhcd-usb_hcd;
+}
+
+inline struct device *mausb_hcd_to_dev(struct mausb_hcd *mhcd)
+{
+ return mausb_hcd_to_usb_hcd(mhcd)-self.controller;
+}
+
+inline struct mausb_urb *usb_urb_to_mausb_urb(struct urb *urb)
+{
+
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote: +#define DEBUG I doubt you want this in the driver enabled by default :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
+EXPORT_SYMBOL(mausb_register_ms_driver);
EXPORT_SYMBOL_GPL()? I have to ask...
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
+ if (ret 0) {
+ printk(KERN_DEBUG %s: failed to register HC driver:
+ error number %d\n, __func__, ret);
pr_err()?
return here, that way you don't need:
+ } else {
This indentation.
+ /* register HCD device */
+ ret = platform_device_register(mausb_pdev);
But again, why is this a platform device? What platform resources does
it have / require?
+
+ if (ret 0) {
+ printk(KERN_DEBUG %s: failed to register HC device:
+ error number %d\n, __func__, ret);
pr_err()?
+ platform_driver_unregister(mausb_driver);
+ } else {
+ /* direct the release function (for exiting) */
+ mausb_pdev.dev.release = mausb_dev_release;
That seems like a serious hack, why do you need to do this in this
manner?
+
+ if (ret 0) {
+ printk(KERN_DEBUG failed to register HC
+ chardev: error number %d\n, ret);
pr_err()?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 01:18:16PM -0800, Greg KH wrote: On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote: +#define DEBUG I doubt you want this in the driver enabled by default :( Thank you for catching, will remove in the next patch version. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 01:21:39PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
+EXPORT_SYMBOL(mausb_register_ms_driver);
EXPORT_SYMBOL_GPL()? I have to ask...
The source is dual-licenced under BSD and GPL. It was our understanding
that dual-licensed should use EXPORT_SYMBOL() instead. Is that wrong?
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
The driver doesn't require platform resources. It looks like a host
controller driver but communicates over the network instead of to
a physical host controller. There is no MA USB-specific hardware.
Should we use a struct device instead of a struct platform_device?
+ if (ret 0) {
+ printk(KERN_DEBUG %s: failed to register HC driver:
+error number %d\n, __func__, ret);
pr_err()?
Will change all printk() to pr_err() in next patch.
return here, that way you don't need:
+ } else {
This indentation.
Will fix in next patch.
+ /* register HCD device */
+ ret = platform_device_register(mausb_pdev);
But again, why is this a platform device? What platform resources does
it have / require?
See above.
+
+ if (ret 0) {
+ printk(KERN_DEBUG %s: failed to register HC device:
+ error number %d\n, __func__, ret);
pr_err()?
See above.
+ platform_driver_unregister(mausb_driver);
+ } else {
+ /* direct the release function (for exiting) */
+ mausb_pdev.dev.release = mausb_dev_release;
That seems like a serious hack, why do you need to do this in this
manner?
This will go away when we get rid of the platform device.
+
+ if (ret 0) {
+ printk(KERN_DEBUG failed to register HC
+chardev: error number %d\n, ret);
pr_err()?
See above.
thanks,
greg k-h
Thanks,
Stephanie
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 04:04:42PM -0800, steph wrote:
On Mon, Nov 03, 2014 at 01:21:39PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
+EXPORT_SYMBOL(mausb_register_ms_driver);
EXPORT_SYMBOL_GPL()? I have to ask...
The source is dual-licenced under BSD and GPL. It was our understanding
that dual-licensed should use EXPORT_SYMBOL() instead. Is that wrong?
Talk to your company lawyers please to get confirmation of what you want
to do here, I can't answer this question, I just have to ask...
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
The driver doesn't require platform resources. It looks like a host
controller driver but communicates over the network instead of to
a physical host controller. There is no MA USB-specific hardware.
Should we use a struct device instead of a struct platform_device?
Yes, please make it a virtual device.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] added media agnostic (MA) USB HCD driver
On Mon, Nov 03, 2014 at 04:13:55PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 04:04:42PM -0800, steph wrote:
On Mon, Nov 03, 2014 at 01:21:39PM -0800, Greg KH wrote:
On Mon, Nov 03, 2014 at 12:42:48PM -0800, Stephanie Wallick wrote:
+EXPORT_SYMBOL(mausb_register_ms_driver);
EXPORT_SYMBOL_GPL()? I have to ask...
The source is dual-licenced under BSD and GPL. It was our understanding
that dual-licensed should use EXPORT_SYMBOL() instead. Is that wrong?
Talk to your company lawyers please to get confirmation of what you want
to do here, I can't answer this question, I just have to ask...
We have permission to go forward with the dual BSD/GPL license. I will leave
as is unless there is a future issue.
+static int mausb_hcd_init(void)
+{
+ int ret;
+
+ /* register HCD driver */
+ ret = platform_driver_register(mausb_driver);
Why is this a platform driver? How does this relate to platform
hardware?
The driver doesn't require platform resources. It looks like a host
controller driver but communicates over the network instead of to
a physical host controller. There is no MA USB-specific hardware.
Should we use a struct device instead of a struct platform_device?
Yes, please make it a virtual device.
Will do.
Thanks,
Stephanie
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
