greybus: interface: drop the control bundle

Drop the control bundle and ignore control descriptors when parsing
manifests.

Every interface has a control connection with a well defined remote
CPort 0 and there's no longer any need to create a bundle for it.

As the control connection is setup and enabled before parsing the
manifest, ignore any legacy descriptors for control cports and bundles
in a manifest.

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

diff --git a/drivers/staging/greybus/interface.c b/drivers/staging/greybus/interface.c
index 2415187..9ff7464 100644 (file)
--- a/drivers/staging/greybus/interface.c
+++ b/drivers/staging/greybus/interface.c
@@ -70,34 +70,6 @@ struct device_type greybus_interface_type = {
 };
 
 /*
- * Create kernel structures corresponding to a bundle and connection for
- * managing control CPort.
- */
-static int
-gb_interface_create_control_bundle_connection(struct gb_interface *intf)
-{
-       struct gb_bundle *bundle;
-       struct gb_connection *connection;
-
-       bundle = gb_bundle_create(intf, GB_CONTROL_BUNDLE_ID,
-                                               GREYBUS_CLASS_CONTROL);
-       if (!bundle) {
-               dev_err(&intf->dev, "failed to create control bundle\n");
-               return -ENOMEM;
-       }
-
-       connection = gb_connection_create_dynamic(intf, bundle,
-                                               GB_CONTROL_CPORT_ID,
-                                               GREYBUS_PROTOCOL_CONTROL);
-       if (!connection) {
-               dev_err(&intf->dev, "failed to create control connection\n");
-               return -ENOMEM;
-       }
-
-       return 0;
-}
-
-/*
  * A Greybus module represents a user-replaceable component on an Ara
  * phone.  An interface is the physical connection on that module.  A
  * module may have more than one interface.
@@ -170,6 +142,9 @@ void gb_interface_remove(struct gb_interface *intf)
        list_for_each_entry_safe(bundle, next, &intf->bundles, links)
                gb_bundle_destroy(bundle);
 
+       if (intf->control)
+               gb_connection_destroy(intf->control->connection);
+
        device_unregister(&intf->dev);
 }
 
@@ -190,15 +165,20 @@ void gb_interfaces_remove(struct gb_host_device *hd)
  */
 int gb_interface_init(struct gb_interface *intf, u8 device_id)
 {
+       struct gb_connection *connection;
        int ret, size;
        void *manifest;
 
        intf->device_id = device_id;
 
        /* Establish control CPort connection */
-       ret = gb_interface_create_control_bundle_connection(intf);
-       if (ret)
-               return ret;
+       connection = gb_connection_create_dynamic(intf, NULL,
+                                               GB_CONTROL_CPORT_ID,
+                                               GREYBUS_PROTOCOL_CONTROL);
+       if (!connection) {
+               dev_err(&intf->dev, "failed to create control connection\n");
+               return -ENOMEM;
+       }
 
        /* Get manifest size using control protocol on CPort */
        size = gb_control_get_manifest_size_operation(intf);

diff --git a/drivers/staging/greybus/manifest.c b/drivers/staging/greybus/manifest.c
index 4fdb98d..ea5ff86 100644 (file)
--- a/drivers/staging/greybus/manifest.c
+++ b/drivers/staging/greybus/manifest.c
@@ -254,24 +254,10 @@ static u32 gb_manifest_parse_cports(struct gb_bundle *bundle)
                /* Found one.  Set up its function structure */
                protocol_id = desc_cport->protocol_id;
 
-               /* Validate declarations of the control protocol CPort */
-               if (cport_id == GB_CONTROL_CPORT_ID) {
-                       /* This should have protocol set to control protocol*/
-                       if (protocol_id != GREYBUS_PROTOCOL_CONTROL)
-                               goto print_error_exit;
-                       /* Don't recreate connection for control cport */
-                       goto release_descriptor;
-               }
-               /* Nothing else should have its protocol as control protocol */
-               if (protocol_id == GREYBUS_PROTOCOL_CONTROL) {
-                       goto print_error_exit;
-               }
-
                if (!gb_connection_create_dynamic(intf, bundle, cport_id,
                                                                protocol_id))
                        goto exit;
 
-release_descriptor:
                count++;
 
                /* Release the cport descriptor */
@@ -279,12 +265,6 @@ release_descriptor:
        }
 
        return count;
-print_error_exit:
-       /* A control protocol parse error was encountered */
-       dev_err(&bundle->dev,
-               "cport_id, protocol_id 0x%04hx,0x%04hx want 0x%04hx,0x%04hx\n",
-               cport_id, protocol_id, GB_CONTROL_CPORT_ID,
-               GREYBUS_PROTOCOL_CONTROL);
 exit:
 
        /*
@@ -308,6 +288,7 @@ static u32 gb_manifest_parse_bundles(struct gb_interface *intf)
        struct gb_bundle *bundle_next;
        u32 count = 0;
        u8 bundle_id;
+       u8 class;
 
        while ((desc = get_next_bundle_desc(intf))) {
                struct greybus_descriptor_bundle *desc_bundle;
@@ -315,37 +296,32 @@ static u32 gb_manifest_parse_bundles(struct gb_interface *intf)
                /* Found one.  Set up its bundle structure*/
                desc_bundle = desc->data;
                bundle_id = desc_bundle->id;
+               class = desc_bundle->class;
+
+               /* Done with this bundle descriptor */
+               release_manifest_descriptor(desc);
 
-               /* Don't recreate bundle for control cport */
+               /* Ignore any legacy control bundles */
                if (bundle_id == GB_CONTROL_BUNDLE_ID) {
-                       /* This should have class set to control class */
-                       if (desc_bundle->class != GREYBUS_CLASS_CONTROL) {
-                               dev_err(&intf->dev,
-                                       "bad class 0x%02x for control bundle\n",
-                                       desc_bundle->class);
-                               goto cleanup;
-                       }
-
-                       bundle = intf->control->connection->bundle;
-                       goto parse_cports;
+                       dev_dbg(&intf->dev, "%s - ignoring control bundle\n",
+                                       __func__);
+                       release_cport_descriptors(&intf->manifest_descs,
+                                                               bundle_id);
+                       continue;
                }
 
                /* Nothing else should have its class set to control class */
-               if (desc_bundle->class == GREYBUS_CLASS_CONTROL) {
+               if (class == GREYBUS_CLASS_CONTROL) {
                        dev_err(&intf->dev,
                                "bundle 0x%02x cannot use control class\n",
                                bundle_id);
                        goto cleanup;
                }
 
-               bundle = gb_bundle_create(intf, bundle_id, desc_bundle->class);
+               bundle = gb_bundle_create(intf, bundle_id, class);
                if (!bundle)
                        goto cleanup;
 
-parse_cports:
-               /* Done with this bundle descriptor */
-               release_manifest_descriptor(desc);
-
                /*
                 * Now go set up this bundle's functions and cports.
                 *
@@ -362,15 +338,8 @@ parse_cports:
                 * separate entities and don't reject entire interface and its
                 * bundles on failing to initialize a cport. But make sure the
                 * bundle which needs the cport, gets destroyed properly.
-                *
-                * The control bundle and its connections are special. The
-                * entire manifest should be rejected if we failed to initialize
-                * the control bundle/connections.
                 */
                if (!gb_manifest_parse_cports(bundle)) {
-                       if (bundle_id == GB_CONTROL_BUNDLE_ID)
-                               goto cleanup;
-
                        gb_bundle_destroy(bundle);
                        continue;
                }