Re: [PATCH 2/4] usbdevfs: Add a USBDEVFS_GET_CAPABILITIES ioctl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Thanks for the review!

On 06/28/2012 06:10 PM, Alan Stern wrote:
On Thu, 28 Jun 2012, Hans de Goede wrote:

There are a few (new) usbdevfs capabilities which an application cannot
discover in any other way then checking the kernel version. There are 3
problems with this:
1) It is just not very pretty
2) Given the tendency of enterprise distros to backport stuff it is not
    reliable
3) Some of these features turn out to not work with certain host controllers,
    making depending on them based on the kernel version not a good idea

This patch besides adding the new ioctl also adds flags the following existing
capabilities:

USBDEVFS_CAP_ZERO_PACKET,        available since 2.6.31
USBDEVFS_CAP_BULK_CONTINUATION,  available since 2.6.32, except for XHCI
USBDEVFS_CAP_NO_PACKET_SIZE_LIM, available since 3.3

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>

+static int proc_get_capabilities(struct dev_state *ps, void __user *arg)
+{
+	__u64 caps = 0;

Does these need to be 64 bits?  Yes, you want to provide room for
future expansion... but we've added capabilities at a rate of about one
per year.  I doubt usbfs will survive the next 30 years.

True, I'll change this to 32 bits in the next revision (which I plan to
post together with a revised version of the scatter-gather patch, as
I assume there will be comments there too).

+
+	caps |= USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM;
+	caps |= USBDEVFS_CAP_BULK_CONTINUATION;

Formatting nit: How about

	caps = USBDEVFS_CAP_ZERO_PACKET |
		USBDEVFS_CAP_NO_PACKET_SIZE_LIM |
		USBDEVFS_CAP_BULK_CONTINUATION |
		0;

It was done this way to make the next patch in the series easier to-do, as
I already knew I wanted to make that change when I wrote this one :)

+/* Device capability flags */
+#define USBDEVFS_CAP_ZERO_PACKET	0x01
+#define USBDEVFS_CAP_BULK_CONTINUATION	0x02 /* Not available on all devs! */
+#define USBDEVFS_CAP_NO_PACKET_SIZE_LIM	0x04

If there really are going to be 64 bits worth of these things,
shouldn't they be defined like
					0x0000000000000001

I think it would need an ll postfix independent of the number
of leading 0-s, but lets just go with 32 bits.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux