On Mon, Nov 13, 2006 at 03:22:35PM -0800, Sarah Bailey wrote:
> This patch is an update for Greg K-H's proposed usbfs2:
> http://sourceforge.net/mailarchive/message.php?msg_id=19295229
>
> It creates a dynamic major for USB endpoints and fixes
> the endpoint minor calculation.
>
> Signed-off-by: Sarah Bailey <[EMAIL PROTECTED]>
This address is different from your "From:" line, should they be the
same?
> The patch currently creates a minor for each physical endpoint (rather
> than logical endpoint). I'm not sure which way is easier/better, I just
> picked one.
Nice first cut, but I have a few questions about it:
> The patch uses the newer way of allocating major numbers to allocate as
> many minor numbers as the system would need. The host controller file
> specifies a maximum of 64 USB busses; each one can have 128 devices with
> 32 physical endpoints. That results in 262,144 minors. I thought about
> adding minors as each new bus was added (perhaps in usb_register_bus in
> hcd.c or by registering a new function using usb_register_notify in
> hcd.c). In the end, I went with the simpler patch.
Hm, how about just using the code in idr.h to just get an arbitrary
number. That interface will handle giving the numbers back when needed,
and that way you don't have to rely on the fact that we don't have more
than 64 USB busses in the system at once.
It should make the code a bit simpler too.
> I diffed against Linus' current git tree. Let me know if I should diff
> against another tree.
If that's the easiest for you, that's fine, I can handle patches against
it.
>
>
> drivers/usb/core/endpoint.c | 10 +++++++---
> drivers/usb/core/file.c | 24 ++++++++++++++++++++++++
> drivers/usb/core/hcd.c | 2 --
> drivers/usb/core/hcd.h | 2 ++
> drivers/usb/core/usb.c | 6 ++++++
> drivers/usb/core/usb.h | 4 ++++
> 6 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/endpoint.c b/drivers/usb/core/endpoint.c
> index 3b2d137..eb5649e 100644
> --- a/drivers/usb/core/endpoint.c
> +++ b/drivers/usb/core/endpoint.c
> @@ -226,12 +226,16 @@ int usb_create_ep_files(struct device *p
> goto error_alloc;
> }
>
> - /* fun calculation to determine the minor of this endpoint */
> - minor = (((udev->bus->busnum - 1) * 128) * 16) + (udev->devnum - 1);
> + /* fun calculation to determine the minor of this endpoint
> + * using the endpoint physical address
> + */
> + minor = (((udev->bus->busnum - 1) * 128 + (udev->devnum - 1))*32) +
> + ((endpoint->desc.bEndpointAddress*0xF) << 1) +
> + ((endpoint->desc.bEndpointAddress*0x80) >> 7);
>
> ep_dev->desc = &endpoint->desc;
> ep_dev->udev = udev;
> - ep_dev->dev.devt = MKDEV(442, minor); // FIXME fake number...
> + ep_dev->dev.devt = MKDEV(usb_endpoints_major, minor);
> ep_dev->dev.class = ep_class->class;
> ep_dev->dev.parent = parent;
> ep_dev->dev.release = ep_device_release;
> diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
> index f794f07..b1562e1 100644
> --- a/drivers/usb/core/file.c
> +++ b/drivers/usb/core/file.c
> @@ -21,10 +21,13 @@ #include <linux/errno.h>
> #include <linux/usb.h>
>
> #include "usb.h"
> +#include "hcd.h"
>
> #define MAX_USB_MINORS 256
> +#define MAX_ENDPOINT_MINORS (USB_MAXBUS*128*32)
> static const struct file_operations *usb_minors[MAX_USB_MINORS];
> static DEFINE_SPINLOCK(minor_lock);
> +int usb_endpoints_major;
>
> static int usb_open(struct inode * inode, struct file * file)
> {
> @@ -123,6 +126,27 @@ void usb_major_cleanup(void)
> unregister_chrdev(USB_MAJOR, "usb");
> }
>
> +int endpoints_major_init(void)
usb_endpoints_major_init() perhaps to keep the namespace sane?
> +{
> + dev_t dev;
> + int error;
> +
> + error = alloc_chrdev_region(&dev, 0, MAX_ENDPOINT_MINORS,
> "usb_endpoint");
> + if (error)
> + {
Wrong use of {, please put it after the "if (error)" on that same line.
> + err("unable to get a dynamic major for usb endpoints");
> + return error;
> + }
> + usb_endpoints_major = MAJOR(dev);
> +
> + return error;
> +}
> +
> +void endpoints_major_cleanup(void)
usb_endpoints_major_cleanup()?
> +{
> + unregister_chrdev_region(MKDEV(usb_endpoints_major, 0),
> MAX_ENDPOINT_MINORS);
> +}
> +
Why not put these functions and variable in the endpoint.c file? That
way you don't have to export the variable.
Care to retry with these changes?
thanks,
greg k-h
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
