greybus: fix host-device buffer constraints

Host devices impose buffer-size constraints on Greybus core which are
taken into account when allocating messages.

Make sure to verify these constraints when the host device is allocated,
rather than when the first message is allocated.

Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c
index 45fa4c3..e32d6c4 100644 (file)
--- a/drivers/staging/greybus/core.c
+++ b/drivers/staging/greybus/core.c
@@ -173,7 +173,8 @@ static void free_hd(struct kref *kref)
 }
 
 struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver,
-                                             struct device *parent)
+                                             struct device *parent,
+                                             size_t buffer_size_max)
 {
        struct greybus_host_device *hd;
 
@@ -187,6 +188,16 @@ struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver
                return NULL;
        }
 
+       /*
+        * Make sure to never allocate messages larger than what the Greybus
+        * protocol supports.
+        */
+       if (buffer_size_max > GB_OPERATION_MESSAGE_SIZE_MAX) {
+               dev_warn(parent, "limiting buffer size to %u\n",
+                        GB_OPERATION_MESSAGE_SIZE_MAX);
+               buffer_size_max = GB_OPERATION_MESSAGE_SIZE_MAX;
+       }
+
        hd = kzalloc(sizeof(*hd) + driver->hd_priv_size, GFP_KERNEL);
        if (!hd)
                return NULL;
@@ -197,6 +208,7 @@ struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *driver
        INIT_LIST_HEAD(&hd->interfaces);
        INIT_LIST_HEAD(&hd->connections);
        ida_init(&hd->cport_id_map);
+       hd->buffer_size_max = buffer_size_max;
 
        hd->endo = gb_endo_create(hd);
        if (!hd->endo) {

diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c
index 4bba5d5..e0fae26 100644 (file)
--- a/drivers/staging/greybus/es1.c
+++ b/drivers/staging/greybus/es1.c
@@ -98,22 +98,6 @@ static void cport_out_callback(struct urb *urb);
 static void usb_log_enable(struct es1_ap_dev *es1);
 static void usb_log_disable(struct es1_ap_dev *es1);
 
-/*
- * Buffer constraints for the host driver.
- *
- * A "buffer" is used to hold data to be transferred for Greybus by
- * the host driver.  A buffer is represented by a "buffer pointer",
- * which defines a region of memory used by the host driver for
- * transferring the data.  When Greybus allocates a buffer, it must
- * do so subject to the constraints associated with the host driver.
- *
- *  size_max:  The maximum size of a buffer
- */
-static void hd_buffer_constraints(struct greybus_host_device *hd)
-{
-       hd->buffer_size_max = ES1_GBUF_MSG_SIZE_MAX;
-}
-
 #define ES1_TIMEOUT    500     /* 500 ms for the SVC to do something */
 static int submit_svc(struct svc_msg *svc_msg, struct greybus_host_device *hd)
 {
@@ -571,15 +555,12 @@ static int ap_probe(struct usb_interface *interface,
 
        udev = usb_get_dev(interface_to_usbdev(interface));
 
-       hd = greybus_create_hd(&es1_driver, &udev->dev);
+       hd = greybus_create_hd(&es1_driver, &udev->dev, ES1_GBUF_MSG_SIZE_MAX);
        if (!hd) {
                usb_put_dev(udev);
                return -ENOMEM;
        }
 
-       /* Fill in the buffer allocation constraints */
-       hd_buffer_constraints(hd);
-
        es1 = hd_to_es1(hd);
        es1->hd = hd;
        es1->usb_intf = interface;

diff --git a/drivers/staging/greybus/es2.c b/drivers/staging/greybus/es2.c
index cc73fbd..05aac3d 100644 (file)
--- a/drivers/staging/greybus/es2.c
+++ b/drivers/staging/greybus/es2.c
@@ -98,22 +98,6 @@ static void cport_out_callback(struct urb *urb);
 static void usb_log_enable(struct es1_ap_dev *es1);
 static void usb_log_disable(struct es1_ap_dev *es1);
 
-/*
- * Buffer constraints for the host driver.
- *
- * A "buffer" is used to hold data to be transferred for Greybus by
- * the host driver.  A buffer is represented by a "buffer pointer",
- * which defines a region of memory used by the host driver for
- * transferring the data.  When Greybus allocates a buffer, it must
- * do so subject to the constraints associated with the host driver.
- *
- *  size_max:  The maximum size of a buffer
- */
-static void hd_buffer_constraints(struct greybus_host_device *hd)
-{
-       hd->buffer_size_max = ES1_GBUF_MSG_SIZE_MAX;
-}
-
 #define ES1_TIMEOUT    500     /* 500 ms for the SVC to do something */
 static int submit_svc(struct svc_msg *svc_msg, struct greybus_host_device *hd)
 {
@@ -571,15 +555,12 @@ static int ap_probe(struct usb_interface *interface,
 
        udev = usb_get_dev(interface_to_usbdev(interface));
 
-       hd = greybus_create_hd(&es1_driver, &udev->dev);
+       hd = greybus_create_hd(&es1_driver, &udev->dev, ES1_GBUF_MSG_SIZE_MAX);
        if (!hd) {
                usb_put_dev(udev);
                return -ENOMEM;
        }
 
-       /* Fill in the buffer allocation constraints */
-       hd_buffer_constraints(hd);
-
        es1 = hd_to_es1(hd);
        es1->hd = hd;
        es1->usb_intf = interface;

diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h
index 0a25d0c..dbb4e78 100644 (file)
--- a/drivers/staging/greybus/greybus.h
+++ b/drivers/staging/greybus/greybus.h
@@ -105,7 +105,8 @@ struct greybus_host_device {
 };
 
 struct greybus_host_device *greybus_create_hd(struct greybus_host_driver *hd,
-                                             struct device *parent);
+                                             struct device *parent,
+                                             size_t buffer_size_max);
 void greybus_remove_hd(struct greybus_host_device *hd);
 
 struct greybus_driver {

diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c
index 1ec930c..c78ccc0 100644 (file)
--- a/drivers/staging/greybus/operation.c
+++ b/drivers/staging/greybus/operation.c
@@ -17,12 +17,6 @@
 /* The default amount of time a request is given to complete */
 #define OPERATION_TIMEOUT_DEFAULT      1000    /* milliseconds */
 
-/*
- * XXX This needs to be coordinated with host driver parameters
- * XXX May need to reduce to allow for message header within a page
- */
-#define GB_OPERATION_MESSAGE_SIZE_MAX  4096
-
 static struct kmem_cache *gb_operation_cache;
 static struct kmem_cache *gb_message_cache;
 
@@ -282,12 +276,6 @@ gb_operation_message_alloc(struct greybus_host_device *hd, u8 type,
        struct gb_operation_msg_hdr *header;
        size_t message_size = payload_size + sizeof(*header);
 
-       if (hd->buffer_size_max > GB_OPERATION_MESSAGE_SIZE_MAX) {
-               pr_warn("limiting buffer size to %u\n",
-                       GB_OPERATION_MESSAGE_SIZE_MAX);
-               hd->buffer_size_max = GB_OPERATION_MESSAGE_SIZE_MAX;
-       }
-
        if (message_size > hd->buffer_size_max) {
                pr_warn("requested message size too big (%zu > %zu)\n",
                                message_size, hd->buffer_size_max);
@@ -936,9 +924,6 @@ EXPORT_SYMBOL_GPL(gb_operation_sync);
 
 int gb_operation_init(void)
 {
-       BUILD_BUG_ON(GB_OPERATION_MESSAGE_SIZE_MAX >
-                       U16_MAX - sizeof(struct gb_operation_msg_hdr));
-
        gb_message_cache = kmem_cache_create("gb_message_cache",
                                sizeof(struct gb_message), 0, 0, NULL);
        if (!gb_message_cache)

diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h
index 82b8fe5..3b02db5 100644 (file)
--- a/drivers/staging/greybus/operation.h
+++ b/drivers/staging/greybus/operation.h
@@ -69,6 +69,8 @@ struct gb_operation_msg_hdr {
        __u8    pad[2];         /* must be zero (ignore when read) */
 } __aligned(sizeof(u64));
 
+#define GB_OPERATION_MESSAGE_SIZE_MAX  4096
+
 /*
  * Protocol code should only examine the payload and payload_size
  * fields.  All other fields are intended to be private to the