[PATCH 08/11] usb: Add USB subsystem notifications [ver #8]

2019-09-04 Thread David Howells
Add a USB subsystem notification mechanism whereby notifications about
hardware events such as device connection, disconnection, reset and I/O
errors, can be reported to a monitoring process asynchronously.

Firstly, an event queue needs to be created:

fd = open("/dev/event_queue", O_RDWR);
ioctl(fd, IOC_WATCH_QUEUE_SET_SIZE, page_size << n);

then a notification can be set up to report USB notifications via that
queue:

struct watch_notification_filter filter = {
.nr_filters = 1,
.filters = {
[0] = {
.type = WATCH_TYPE_USB_NOTIFY,
.subtype_filter[0] = UINT_MAX;
},
},
};
ioctl(fd, IOC_WATCH_QUEUE_SET_FILTER, &filter);
notify_devices(fd, 12);

After that, records will be placed into the queue when events occur on a
USB device or bus.  Records are of the following format:

struct usb_notification {
struct watch_notification watch;
__u32   error;
__u32   reserved;
__u8name_len;
__u8name[0];
} *n;

Where:

n->watch.type will be WATCH_TYPE_USB_NOTIFY

n->watch.subtype will be the type of notification, such as
NOTIFY_USB_DEVICE_ADD.

n->watch.info & WATCH_INFO_LENGTH will indicate the length of the
record.

n->watch.info & WATCH_INFO_ID will be the second argument to
device_notify(), shifted.

n->error and n->reserved are intended to convey information such as
error codes, but are currently not used

n->name_len and n->name convey the USB device name as an
unterminated string.  This may be truncated - it is currently
limited to a maximum 63 chars.

Note that it is permissible for event records to be of variable length -
or, at least, the length may be dependent on the subtype.

Signed-off-by: David Howells 
Reviewed-by: Greg Kroah-Hartman 
cc: linux-usb@vger.kernel.org
---

 Documentation/watch_queue.rst|9 +++
 drivers/usb/core/Kconfig |9 +++
 drivers/usb/core/devio.c |   49 ++
 drivers/usb/core/hub.c   |4 +++
 include/linux/usb.h  |   18 ++
 include/uapi/linux/watch_queue.h |   28 +-
 6 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/Documentation/watch_queue.rst b/Documentation/watch_queue.rst
index 5cc9c6924727..4087a8e670a8 100644
--- a/Documentation/watch_queue.rst
+++ b/Documentation/watch_queue.rst
@@ -11,6 +11,8 @@ receive notifications from the kernel.  This can be used in 
conjunction with::
 
 * Block layer event notifications
 
+* USB subsystem event notifications
+
 
 The notifications buffers can be enabled by:
 
@@ -315,6 +317,13 @@ Any particular buffer can be fed from multiple sources.  
Sources include:
 or temporary link loss.  Watches of this type are set on the global device
 watch list.
 
+  * WATCH_TYPE_USB_NOTIFY
+
+Notifications of this type indicate USB subsystem events, such as
+attachment, removal, reset and I/O errors.  Separate events are generated
+for buses and devices.  Watchpoints of this type are set on the global
+device watch list.
+
 
 Event Filtering
 ===
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index ecaacc8ed311..57e7b649e48b 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -102,3 +102,12 @@ config USB_AUTOSUSPEND_DELAY
  The default value Linux has always had is 2 seconds.  Change
  this value if you want a different delay and cannot modify
  the command line or module parameter.
+
+config USB_NOTIFICATIONS
+   bool "Provide USB hardware event notifications"
+   depends on USB && DEVICE_NOTIFICATIONS
+   help
+ This option provides support for getting hardware event notifications
+ on USB devices and interfaces.  This makes use of the
+ /dev/watch_queue misc device to handle the notification buffer.
+ device_notify(2) is used to set/remove watches.
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 9063ede411ae..21c07d76f7d7 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "usb.h"
 
@@ -2660,13 +2661,61 @@ static void usbdev_remove(struct usb_device *udev)
}
 }
 
+#ifdef CONFIG_USB_NOTIFICATIONS
+static noinline void post_usb_notification(const char *devname,
+  enum usb_notification_type subtype,
+  u32 error)
+{
+   unsigned int gran = WATCH_LENGTH_GRANULARITY;
+   un

[PATCH 09/11] Add sample notification program [ver #8]

2019-09-04 Thread David Howells
This needs to be linked with -lkeyutils.

It is run like:

./watch_test

and watches "/" for mount changes and the current session keyring for key
changes:

# keyctl add user a a @s
1035096409
# keyctl unlink 1035096409 @s

producing:

# ./watch_test
ptrs h=4 t=2 m=20003
NOTIFY[0004-0002] ty=0003 sy=0002 i=0110
KEY 2ffc2e5d change=2[linked] aux=1035096409
ptrs h=6 t=4 m=20003
NOTIFY[0006-0004] ty=0003 sy=0003 i=0110
KEY 2ffc2e5d change=3[unlinked] aux=1035096409

Other events may be produced, such as with a failing disk:

ptrs h=5 t=2 m=604
NOTIFY[0005-0002] ty=0004 sy=0006 i=0418
BLOCK 00800050 e=6[critical medium] s=5be8

This corresponds to:

print_req_error: critical medium error, dev sdf, sector 23528 flags 0

in dmesg.

Signed-off-by: David Howells 
---

 samples/Kconfig  |6 +
 samples/Makefile |1 
 samples/watch_queue/Makefile |8 +
 samples/watch_queue/watch_test.c |  231 ++
 4 files changed, 246 insertions(+)
 create mode 100644 samples/watch_queue/Makefile
 create mode 100644 samples/watch_queue/watch_test.c

diff --git a/samples/Kconfig b/samples/Kconfig
index c8dacb4dda80..2c3e07addd38 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -169,4 +169,10 @@ config SAMPLE_VFS
  as mount API and statx().  Note that this is restricted to the x86
  arch whilst it accesses system calls that aren't yet in all arches.
 
+config SAMPLE_WATCH_QUEUE
+   bool "Build example /dev/watch_queue notification consumer"
+   help
+ Build example userspace program to use the new mount_notify(),
+ sb_notify() syscalls and the KEYCTL_WATCH_KEY keyctl() function.
+
 endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index 7d6e4ca28d69..a61a39047d02 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_SAMPLE_TRACE_PRINTK) += trace_printk/
 obj-$(CONFIG_VIDEO_PCI_SKELETON)   += v4l/
 obj-y  += vfio-mdev/
 subdir-$(CONFIG_SAMPLE_VFS)+= vfs
+subdir-$(CONFIG_SAMPLE_WATCH_QUEUE)+= watch_queue
diff --git a/samples/watch_queue/Makefile b/samples/watch_queue/Makefile
new file mode 100644
index ..6ee61e3ca8d2
--- /dev/null
+++ b/samples/watch_queue/Makefile
@@ -0,0 +1,8 @@
+# List of programs to build
+hostprogs-y := watch_test
+
+# Tell kbuild to always build the programs
+always := $(hostprogs-y)
+
+HOSTCFLAGS_watch_test.o += -I$(objtree)/usr/include
+HOSTLDLIBS_watch_test += -lkeyutils
diff --git a/samples/watch_queue/watch_test.c b/samples/watch_queue/watch_test.c
new file mode 100644
index ..ecc191174b7a
--- /dev/null
+++ b/samples/watch_queue/watch_test.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Use /dev/watch_queue to watch for notifications.
+ *
+ * Copyright (C) 2019 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowe...@redhat.com)
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#ifndef KEYCTL_WATCH_KEY
+#define KEYCTL_WATCH_KEY -1
+#endif
+#ifndef __NR_watch_devices
+#define __NR_watch_devices -1
+#endif
+
+#define BUF_SIZE 4
+
+static long keyctl_watch_key(int key, int watch_fd, int watch_id)
+{
+   return syscall(__NR_keyctl, KEYCTL_WATCH_KEY, key, watch_fd, watch_id);
+}
+
+static const char *key_subtypes[256] = {
+   [NOTIFY_KEY_INSTANTIATED]   = "instantiated",
+   [NOTIFY_KEY_UPDATED]= "updated",
+   [NOTIFY_KEY_LINKED] = "linked",
+   [NOTIFY_KEY_UNLINKED]   = "unlinked",
+   [NOTIFY_KEY_CLEARED]= "cleared",
+   [NOTIFY_KEY_REVOKED]= "revoked",
+   [NOTIFY_KEY_INVALIDATED]= "invalidated",
+   [NOTIFY_KEY_SETATTR]= "setattr",
+};
+
+static void saw_key_change(struct watch_notification *n)
+{
+   struct key_notification *k = (struct key_notification *)n;
+   unsigned int len = (n->info & WATCH_INFO_LENGTH) >> 
WATCH_INFO_LENGTH__SHIFT;
+
+   if (len != sizeof(struct key_notification) / WATCH_LENGTH_GRANULARITY)
+   return;
+
+   printf("KEY %08x change=%u[%s] aux=%u\n",
+  k->key_id, n->subtype, key_subtypes[n->subtype], k->aux);
+}
+
+static const char *block_subtypes[256] = {
+   [NOTIFY_BLOCK_ERROR_TIMEOUT]= "timeout",
+   [NOTIFY_BLOCK_ERROR_NO_SPACE]   = "critical space 
allocation",
+   [NOTIFY_BLOCK_ERROR_RECOVERABLE_TRANSPORT]  = "recoverabl

Re: [PATCH 00/16] remove eight obsolete architectures

2018-03-15 Thread David Howells
Do we have anything left that still implements NOMMU?

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


Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-14 Thread David Howells
Andy Lutomirski  wrote:

> David, are these encrypted keys ever exported anywhere?  If not, could
> the code use a mode that doesn't need padding?

ecryptfs uses them, I think.

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


Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-14 Thread David Howells
Andy Lutomirski  wrote:

> > -   sg_set_buf(&sg_out[1], pad, sizeof pad);
> > +   sg_set_buf(&sg_out[1], empty_zero_page, 16);
> 
> My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
> exactly is the code trying to do?  The old code makes no sense.  It's
> setting the *output* buffer to zeroed padding.

Padding goes into the encrypt function and is going to come out of the decrypt
function.  Possibly derived_key_decrypt() should be checking that the padding
that comes out is actually a bunch of zeros.  Maybe we don't actually need to
get the padding out, but I'm not sure whether the crypto layer will
malfunction if we don't give it a buffer for the padding.

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


Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread David Howells
Andy Lutomirski  wrote:

> I don't know whether you're right, but that sounds a bit silly to me.
> This is a *tiny* amount of memory.

Assuming a 1MiB kernel image in 4K pages, that gets you back a couple of pages
I think - useful if you've only got a few MiB of RAM.

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


Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-13 Thread David Howells
Andy Lutomirski  wrote:

> After all, rodata is ordinary memory, is backed by struct page, etc.

Is that actually true?  I thought some arches excluded the kernel image from
the page struct array to make the array consume less memory.

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


Re: [PATCH] keys/encrypted: Fix two crypto-on-the-stack bugs

2016-12-12 Thread David Howells
Andy Lutomirski  wrote:

> +static const char zero_pad[16] = {0};

Isn't there a global page of zeros or something that we can share?  Also, you
shouldn't explicitly initialise it so that it stays in .bss.

> - sg_set_buf(&sg_out[1], pad, sizeof pad);
> + sg_set_buf(&sg_out[1], zero_pad, sizeof zero_pad);

Can you put brackets on the sizeof?

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


Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types

2015-09-21 Thread David Howells
Andrzej Hajda  wrote:

> Semantic patch finds comparisons of types:
> unsigned < 0
> unsigned >= 0
> The former is always false, the latter is always true.
> Such comparisons are useless, so theoretically they could be
> safely removed, but their presence quite often indicates bugs.

Or someone has left them in because they don't matter and there's the
possibility that the type being tested might be or become signed under some
circumstances.  If the comparison is useless, I'd expect the compiler to just
discard it - for such cases your patch is pointless.

If I have, for example:

unsigned x;

if (x == 0 || x > 27)
give_a_range_error();

I will write this as:

unsigned x;

if (x <= 0 || x > 27)
give_a_range_error();

because it that gives a way to handle x being changed to signed at some point
in the future for no cost.  In which case, your changing the <= to an ==
"because the < part of the case is useless" is arguably wrong.

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


[PATCH 10/26] fsl_udc: Don't use create_proc_read_entry() [RFC]

2013-04-11 Thread David Howells
Don't use create_proc_read_entry() as that is deprecated, but rather use
proc_create_data() and seq_file instead.

Signed-off-by: David Howells 
cc: Li Yang 
cc: Felipe Balbi 
cc: Greg Kroah-Hartman 
cc: linux-usb@vger.kernel.org
cc: linuxppc-...@lists.ozlabs.org
---

 drivers/usb/gadget/fsl_udc_core.c |  124 +
 1 file changed, 43 insertions(+), 81 deletions(-)

diff --git a/drivers/usb/gadget/fsl_udc_core.c 
b/drivers/usb/gadget/fsl_udc_core.c
index 04d5fef..ede70ff 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2038,47 +2038,37 @@ static int fsl_udc_stop(struct usb_gadget *g,
 
 static const char proc_filename[] = "driver/fsl_usb2_udc";
 
-static int fsl_proc_read(char *page, char **start, off_t off, int count,
-   int *eof, void *_dev)
+static int fsl_proc_read(struct seq_file *m, void *v)
 {
-   char *buf = page;
-   char *next = buf;
-   unsigned size = count;
unsigned long flags;
-   int t, i;
+   int i;
u32 tmp_reg;
struct fsl_ep *ep = NULL;
struct fsl_req *req;
 
struct fsl_udc *udc = udc_controller;
-   if (off != 0)
-   return 0;
 
spin_lock_irqsave(&udc->lock, flags);
 
/* --basic driver information  */
-   t = scnprintf(next, size,
+   seq_printf(m,
DRIVER_DESC "\n"
"%s version: %s\n"
"Gadget driver: %s\n\n",
driver_name, DRIVER_VERSION,
udc->driver ? udc->driver->driver.name : "(none)");
-   size -= t;
-   next += t;
 
/* -- DR Registers - */
tmp_reg = fsl_readl(&dr_regs->usbcmd);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USBCMD reg:\n"
"SetupTW: %d\n"
"Run/Stop: %s\n\n",
(tmp_reg & USB_CMD_SUTW) ? 1 : 0,
(tmp_reg & USB_CMD_RUN_STOP) ? "Run" : "Stop");
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->usbsts);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Status Reg:\n"
"Dr Suspend: %d Reset Received: %d System Error: %s "
"USB Error Interrupt: %s\n\n",
@@ -2086,11 +2076,9 @@ static int fsl_proc_read(char *page, char **start, off_t 
off, int count,
(tmp_reg & USB_STS_RESET) ? 1 : 0,
(tmp_reg & USB_STS_SYS_ERR) ? "Err" : "Normal",
(tmp_reg & USB_STS_ERR) ? "Err detected" : "No err");
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->usbintr);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Interrupt Enable Reg:\n"
"Sleep Enable: %d SOF Received Enable: %d "
"Reset Enable: %d\n"
@@ -2104,33 +2092,25 @@ static int fsl_proc_read(char *page, char **start, 
off_t off, int count,
(tmp_reg & USB_INTR_PTC_DETECT_EN) ? 1 : 0,
(tmp_reg & USB_INTR_ERR_INT_EN) ? 1 : 0,
(tmp_reg & USB_INTR_INT_EN) ? 1 : 0);
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->frindex);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Frame Index Reg: Frame Number is 0x%x\n\n",
(tmp_reg & USB_FRINDEX_MASKS));
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->deviceaddr);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Device Address Reg: Device Addr is 0x%x\n\n",
(tmp_reg & USB_DEVICE_ADDRESS_MASK));
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->endpointlistaddr);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Endpoint List Address Reg: "
"Device Addr is 0x%x\n\n",
(tmp_reg & USB_EP_LIST_ADDRESS_MASK));
-   size -= t;
-   next += t;
 
tmp_reg = fsl_readl(&dr_regs->portsc1);
-   t = scnprintf(next, size,
+   seq_printf(m,
"USB Port Status&Control Reg:\n"
"Port Transceiver Type : %s Port Speed: %s\n"
"PHY Low Power Suspend: %s Port Reset: %s "
@@ -2139,7 +2119,7 @@ static int fsl_proc_read(char *page, char **start, off_t 
off, int count,
 

[PATCH 09/26] goku_udc: Don't use create_proc_read_entry() [RFC]

2013-04-11 Thread David Howells
Don't use create_proc_read_entry() as that is deprecated, but rather use
proc_create_data() and seq_file instead.

Signed-off-by: David Howells 
cc: Felipe Balbi 
cc: Greg Kroah-Hartman 
cc: linux-usb@vger.kernel.org
---

 drivers/usb/gadget/goku_udc.c |   89 ++---
 1 file changed, 39 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index 85742d4..57a5470 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1008,7 +1009,7 @@ static const struct usb_gadget_ops goku_ops = {
 
 /*-*/
 
-static inline char *dmastr(void)
+static inline const char *dmastr(void)
 {
if (use_dma == 0)
return "(dma disabled)";
@@ -1025,13 +1026,10 @@ static const char proc_node_name [] = "driver/udc";
 #define FOURBITS "%s%s%s%s"
 #define EIGHTBITS FOURBITS FOURBITS
 
-static void
-dump_intmask(const char *label, u32 mask, char **next, unsigned *size)
+static void dump_intmask(struct seq_file *m, const char *label, u32 mask)
 {
-   int t;
-
/* int_status is the same format ... */
-   t = scnprintf(*next, *size,
+   seq_printf(m,
"%s %05X =" FOURBITS EIGHTBITS EIGHTBITS "\n",
label, mask,
(mask & INT_PWRDETECT) ? " power" : "",
@@ -1058,33 +1056,23 @@ dump_intmask(const char *label, u32 mask, char **next, 
unsigned *size)
(mask & INT_ENDPOINT0) ? " ep0" : "",
(mask & INT_USBRESET) ? " reset" : "",
(mask & INT_SUSPEND) ? " suspend" : "");
-   *size -= t;
-   *next += t;
 }
 
 
-static int
-udc_proc_read(char *buffer, char **start, off_t off, int count,
-   int *eof, void *_dev)
+static int udc_proc_read(struct seq_file *m, void *v)
 {
-   char*buf = buffer;
-   struct goku_udc *dev = _dev;
+   struct goku_udc *dev = m->private;
struct goku_udc_regs __iomem*regs = dev->regs;
-   char*next = buf;
-   unsignedsize = count;
unsigned long   flags;
-   int i, t, is_usb_connected;
+   int i, is_usb_connected;
u32 tmp;
 
-   if (off != 0)
-   return 0;
-
local_irq_save(flags);
 
/* basic device status */
tmp = readl(®s->power_detect);
is_usb_connected = tmp & PW_DETECT;
-   t = scnprintf(next, size,
+   seq_printf(m,
"%s - %s\n"
"%s version: %s %s\n"
"Gadget driver: %s\n"
@@ -1096,7 +1084,7 @@ udc_proc_read(char *buffer, char **start, off_t off, int 
count,
is_usb_connected
? ((tmp & PW_PULLUP) ? "full speed" : "powered")
: "disconnected",
-   ({char *state;
+   ({const char *state;
switch(dev->ep0state){
case EP0_DISCONNECT:state = "ep0_disconnect"; break;
case EP0_IDLE:  state = "ep0_idle"; break;
@@ -1108,27 +1096,24 @@ udc_proc_read(char *buffer, char **start, off_t off, 
int count,
default:state = "ep0_?"; break;
} state; })
);
-   size -= t;
-   next += t;
 
-   dump_intmask("int_status", readl(®s->int_status), &next, &size);
-   dump_intmask("int_enable", readl(®s->int_enable), &next, &size);
+   dump_intmask(m, "int_status", readl(®s->int_status));
+   dump_intmask(m, "int_enable", readl(®s->int_enable));
 
if (!is_usb_connected || !dev->driver || (tmp & PW_PULLUP) == 0)
goto done;
 
/* registers for (active) device and ep0 */
-   t = scnprintf(next, size, "\nirqs %lu\ndataset %02x "
+   if (seq_printf(m, "\nirqs %lu\ndataset %02x "
"single.bcs %02x.%02x state %x addr %u\n",
dev->irqs, readl(®s->DataSet),
readl(®s->EPxSingle), readl(®s->EPxBCS),
readl(®s->UsbState),
-   readl(®s->address));
-   size -= t;
-   next += t;
+   readl(®s->address)) < 0)
+   goto done;
 
tmp = readl(®s->dma_master);
-   

Re: [PATCH 2/2] xhci: Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h

2013-04-02 Thread David Howells
Sarah Sharp  wrote:

> I guess my question is a deeper one: do we need to rename all the xHCI
> macros to have the XHCI_ prefix, in order to avoid future collision?
> For example, one of the macros is MAX_HC_PORTS, which could possibly be
> used by other host drivers in the future.

Hmmm...

I suspect the question is whether your symbols are likely to collide with
core symbols rather than symbols of unrelated drivers - after all, you're
unlikely to be #including the headers of those drivers.

I personally prefer to prefix the names of symbols in drivers with something
consistent for that driver to reduce namespace collisions - but I know not
everyone cares about that.  Linux doesn't have much of a policy in this area
though.  I also like it because it makes tags easier to use (fewer definitions
of the same symbol).

Whether you should go back and rename existing xHCI functions, I don't know.
I'd be tempted to leave it for now unless there's some collision.  However,
things like MAX_HC_PORTS does seem a little generic.  Further #define
collisions go unnoticed under some circumstances.  Two obvious cases are (a)
redefinition of a symbol because it happens to be the same value and (b) where
the second one is accidentally suppressed because it is wrapped in a
conditional.

Perhaps we should move to C++ and use namespaces;-)

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


Re: [PATCH 2/2] xhci: Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h

2013-03-28 Thread David Howells
Sarah Sharp  wrote:

> I'm a little bit confused about your description for the second one.
> Did you need to change the #defines names because they could conflict
> with other drivers when the xHCI driver is built in?  Or is there some
> other point I'm missing?

Sorry, I should say.  I'm trying to clean up the UAPI headers and I noticed
that the xHCI SEGMENT_SIZE macro is named the same as one defined by a.out.h
that cannot be changed as it is seen by userspace.  Although it's unlikely
that within the kernel they are unlikely to collide, one cannot be entirely
sure that will stay true as new arches get added (hopefully no one will add
new arches that use a.out format).  It seems best that the xHCI one be renamed
if possible.

> Are these feature patches for 3.10, or bug fixes for 3.9?  If they're
> for 3.9, should they go into stable?  My inclination is the first patch
> shouldn't but I'm not sure about this one.

Both are aimed at 3.10.  Neither are fixes for extant bugs that I know of.

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


[PATCH 1/2] xhci: Use ilog2() rather than __ffs() for calculating SEGMENT_SHIFT

2013-03-28 Thread David Howells
Use ilog2() rather than __ffs() for calculating SEGMENT_SHIFT as ilog2() can
be worked out at compile time, whereas __ffs() must be calculated at runtime.

Signed-off-by: David Howells 
cc: Sarah Sharp 
cc: Greg Kroah-Hartman 
cc: linux-usb@vger.kernel.org
---

 drivers/usb/host/xhci.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2c510e4..558ba50 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1235,7 +1235,7 @@ union xhci_trb {
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS  (TRBS_PER_SEGMENT - 3)
 #define SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
-#define SEGMENT_SHIFT  (__ffs(SEGMENT_SIZE))
+#define SEGMENT_SHIFT  (ilog2(SEGMENT_SIZE))
 /* TRB buffer pointers can't cross 64KB boundaries */
 #define TRB_MAX_BUFF_SHIFT 16
 #define TRB_MAX_BUFF_SIZE  (1 << TRB_MAX_BUFF_SHIFT)

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


[PATCH 2/2] xhci: Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h

2013-03-28 Thread David Howells
Rename SEGMENT_SIZE and SEGMENT_SHIFT as the former is used in a.out.h.

Signed-off-by: David Howells 
cc: Sarah Sharp 
cc: Greg Kroah-Hartman 
cc: linux-usb@vger.kernel.org
---

 drivers/usb/host/xhci-mem.c |   16 
 drivers/usb/host/xhci.h |4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 35616ff..e11e2af 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -51,7 +51,7 @@ static struct xhci_segment *xhci_segment_alloc(struct 
xhci_hcd *xhci,
return NULL;
}
 
-   memset(seg->trbs, 0, SEGMENT_SIZE);
+   memset(seg->trbs, 0, TRB_SEGMENT_SIZE);
/* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */
if (cycle_state == 0) {
for (i = 0; i < TRBS_PER_SEGMENT; i++)
@@ -467,7 +467,7 @@ struct xhci_ring *xhci_dma_to_transfer_ring(
 {
if (ep->ep_state & EP_HAS_STREAMS)
return radix_tree_lookup(&ep->stream_info->trb_address_map,
-   address >> SEGMENT_SHIFT);
+   address >> TRB_SEGMENT_SHIFT);
return ep->ring;
 }
 
@@ -478,7 +478,7 @@ static struct xhci_ring *dma_to_stream_ring(
u64 address)
 {
return radix_tree_lookup(&stream_info->trb_address_map,
-   address >> SEGMENT_SHIFT);
+   address >> TRB_SEGMENT_SHIFT);
 }
 #endif /* CONFIG_USB_XHCI_HCD_DEBUGGING */
 
@@ -514,7 +514,7 @@ static int xhci_test_radix_tree(struct xhci_hcd *xhci,
 
cur_ring = stream_info->stream_rings[cur_stream];
for (addr = cur_ring->first_seg->dma;
-   addr < cur_ring->first_seg->dma + SEGMENT_SIZE;
+   addr < cur_ring->first_seg->dma + 
TRB_SEGMENT_SIZE;
addr += trb_size) {
mapped_ring = dma_to_stream_ring(stream_info, addr);
if (cur_ring != mapped_ring) {
@@ -662,7 +662,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
cur_stream, (unsigned long long) addr);
 
key = (unsigned long)
-   (cur_ring->first_seg->dma >> SEGMENT_SHIFT);
+   (cur_ring->first_seg->dma >> TRB_SEGMENT_SHIFT);
ret = radix_tree_insert(&stream_info->trb_address_map,
key, cur_ring);
if (ret) {
@@ -693,7 +693,7 @@ cleanup_rings:
if (cur_ring) {
addr = cur_ring->first_seg->dma;
radix_tree_delete(&stream_info->trb_address_map,
-   addr >> SEGMENT_SHIFT);
+   addr >> TRB_SEGMENT_SHIFT);
xhci_ring_free(xhci, cur_ring);
stream_info->stream_rings[cur_stream] = NULL;
}
@@ -764,7 +764,7 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
if (cur_ring) {
addr = cur_ring->first_seg->dma;
radix_tree_delete(&stream_info->trb_address_map,
-   addr >> SEGMENT_SHIFT);
+   addr >> TRB_SEGMENT_SHIFT);
xhci_ring_free(xhci, cur_ring);
stream_info->stream_rings[cur_stream] = NULL;
}
@@ -2325,7 +2325,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 * so we pick the greater alignment need.
 */
xhci->segment_pool = dma_pool_create("xHCI ring segments", dev,
-   SEGMENT_SIZE, 64, xhci->page_size);
+   TRB_SEGMENT_SIZE, 64, xhci->page_size);
 
/* See Table 46 and Note on Figure 55 */
xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 558ba50..6bf2257 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1234,8 +1234,8 @@ union xhci_trb {
 #define TRBS_PER_SEGMENT   64
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS  (TRBS_PER_SEGMENT - 3)
-#define SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
-#define SEGMENT_SHIFT  (ilog2(SEGMENT_SIZE))
+#define TRB_SEGMENT_SIZE   (TRBS_PER_SEGMENT*16)
+#define TRB_SEGMENT_SHIFT  (ilog2(TRB_SEGMENT_SIZE))
 /* TRB buffer pointers can't cross 64KB boundaries */
 #define TRB_MAX_BUFF_SHIFT 16
 #define TRB_MAX_BUFF_SIZE  (1 << TRB_MAX_BUFF_SHIFT)

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


Re: [GIT PULL] Disintegrate UAPI for usb

2012-10-09 Thread David Howells
Greg KH  wrote:

> > Can you merge the following branch into the usb tree please.
> 
> Is this (and the other pull requests for other subsystems) for 3.7 or
> for 3.8?

I don't really mind.  There are no dependencies on it.  Getting it in 3.7
means there's more chance it'll just apply when Linus does pull it.

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


[GIT PULL] Disintegrate UAPI for usb

2012-10-09 Thread David Howells
Can you merge the following branch into the usb tree please.

This is to complete part of the Userspace API (UAPI) disintegration for which
the preparatory patches were pulled recently.  After these patches, userspace
headers will be segregated into:

include/uapi/linux/.../foo.h

for the userspace interface stuff, and:

include/linux/.../foo.h

for the strictly kernel internal stuff.

---
The following changes since commit 9e2d8656f5e8aa214e66b462680cf86b210b74a8:

  Merge branch 'akpm' (Andrew's patch-bomb) (2012-10-09 16:23:15 +0900)

are available in the git repository at:


  git://git.infradead.org/users/dhowells/linux-headers.git 
tags/disintegrate-usb-20121009

for you to fetch changes up to 5e1ddb481776a487b15b40579a000b279ce527c9:

  UAPI: (Scripted) Disintegrate include/linux/usb (2012-10-09 09:49:07 +0100)


UAPI Disintegration 2012-10-09

--------
David Howells (1):
  UAPI: (Scripted) Disintegrate include/linux/usb

 include/linux/usb/Kbuild |  10 -
 include/linux/usb/audio.h| 524 +---
 include/linux/usb/ch9.h  | 960 +-
 include/linux/usb/functionfs.h   | 167 +-
 include/uapi/linux/usb/Kbuild|  10 +
 include/uapi/linux/usb/audio.h   | 545 +
 include/{ => uapi}/linux/usb/cdc.h   |   0
 include/{ => uapi}/linux/usb/ch11.h  |   0
 include/uapi/linux/usb/ch9.h | 993 +++
 include/uapi/linux/usb/functionfs.h  | 169 ++
 include/{ => uapi}/linux/usb/g_printer.h |   0
 include/{ => uapi}/linux/usb/gadgetfs.h  |   0
 include/{ => uapi}/linux/usb/midi.h  |   0
 include/{ => uapi}/linux/usb/tmc.h   |   0
 include/{ => uapi}/linux/usb/video.h |   0
 15 files changed, 1720 insertions(+), 1658 deletions(-)
 create mode 100644 include/uapi/linux/usb/audio.h
 rename include/{ => uapi}/linux/usb/cdc.h (100%)
 rename include/{ => uapi}/linux/usb/ch11.h (100%)
 create mode 100644 include/uapi/linux/usb/ch9.h
 create mode 100644 include/uapi/linux/usb/functionfs.h
 rename include/{ => uapi}/linux/usb/g_printer.h (100%)
 rename include/{ => uapi}/linux/usb/gadgetfs.h (100%)
 rename include/{ => uapi}/linux/usb/midi.h (100%)
 rename include/{ => uapi}/linux/usb/tmc.h (100%)
 rename include/{ => uapi}/linux/usb/video.h (100%)
.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html