greybus: fix module setup

The code that was setting up a module was not properly initializing
the module data structure.  Fixing this required a little rework.

Now gb_add_module() (which the host device pointer and module id)
allocates and initializes the structure, and passes it to
gb_manifest_parse() for populating it further.

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c
index 40c899697a339d84c3ab7ac6a75e41b8dbc18a2d..41fb7caf0d8ba9e47c2addddb88a52dff1d24f48 100644 (file)
--- a/drivers/staging/greybus/core.c
+++ b/drivers/staging/greybus/core.c
@@ -203,14 +203,19 @@ void gb_add_module(struct greybus_host_device *hd, u8 module_id,
        struct gb_module *gmod;
        int retval;
 
+       gmod = gb_module_create(hd, module_id);
+       if (!gmod) {
+               dev_err(hd->parent, "failed to create module\n");
+               return;
+       }
+
        /*
         * Parse the manifest and build up our data structures
         * representing what's in it.
         */
-       gmod = gb_manifest_parse(data, size);
-       if (!gmod) {
+       if (!gb_manifest_parse(gmod, data, size)) {
                dev_err(hd->parent, "manifest error\n");
-               return;
+               goto error;
        }
 
        /*
@@ -246,6 +251,8 @@ error_subdevs:
        device_del(&gmod->dev);
 
 error:
+       gb_module_destroy(gmod);
+
        put_device(&gmod->dev);
        greybus_module_release(&gmod->dev);
 }

diff --git a/drivers/staging/greybus/manifest.c b/drivers/staging/greybus/manifest.c
index ad7922aba32da2ad2f8d930dde90b932116bc65e..03282fd643b36142e46a99f3f4fab0797d3cc5c5 100644 (file)
--- a/drivers/staging/greybus/manifest.c
+++ b/drivers/staging/greybus/manifest.c
@@ -259,27 +259,20 @@ static u32 gb_manifest_parse_interfaces(struct gb_module *gmod)
        return count;
 }
 
-static struct gb_module *gb_manifest_parse_module(struct manifest_desc *module_desc)
+static bool gb_manifest_parse_module(struct gb_module *gmod,
+                                       struct manifest_desc *module_desc)
 {
        struct greybus_descriptor *desc = module_desc->data;
        struct greybus_descriptor_module *desc_module = &desc->module;
-       struct gb_module *gmod;
-
-       gmod = kzalloc(sizeof(*gmod), GFP_KERNEL);
-       if (!gmod)
-               return NULL;
 
        /* Handle the strings first--they can fail */
        gmod->vendor_string = gb_string_get(desc_module->vendor_stringid);
-       if (IS_ERR(gmod->vendor_string)) {
-               kfree(gmod);
-               return NULL;
-       }
+       if (IS_ERR(gmod->vendor_string))
+               return false;
+
        gmod->product_string = gb_string_get(desc_module->product_stringid);
        if (IS_ERR(gmod->product_string)) {
-               kfree(gmod->vendor_string);
-               kfree(gmod);
-               return NULL;
+               goto out_err;
        }
 
        gmod->vendor = le16_to_cpu(desc_module->vendor);
@@ -293,11 +286,17 @@ static struct gb_module *gb_manifest_parse_module(struct manifest_desc *module_d
        /* A module must have at least one interface descriptor */
        if (!gb_manifest_parse_interfaces(gmod)) {
                pr_err("manifest interface descriptors not valid\n");
-               gb_module_destroy(gmod);
-               return NULL;
+               goto out_err;
        }
 
-       return gmod;
+       return true;
+out_err:
+       kfree(gmod->product_string);
+       gmod->product_string = NULL;
+       kfree(gmod->vendor_string);
+       gmod->vendor_string = NULL;
+
+       return false;
 }
 
 /*
@@ -321,25 +320,23 @@ static struct gb_module *gb_manifest_parse_module(struct manifest_desc *module_d
  * After that we look for the module's interfaces--there must be at
  * least one of those.
  *
- * Return a pointer to an initialized gb_module structure
- * representing the content of the module manifest, or a null
- * pointer if an error occurs.
+ * Returns true if parsing was successful, false otherwise.
  */
-struct gb_module *gb_manifest_parse(void *data, size_t size)
+bool gb_manifest_parse(struct gb_module *gmod, void *data, size_t size)
 {
        struct greybus_manifest *manifest;
        struct greybus_manifest_header *header;
        struct greybus_descriptor *desc;
        struct manifest_desc *descriptor;
-       struct manifest_desc *module_desc = NULL;
-       struct gb_module *gmod;
+       struct manifest_desc *module_desc = false;
        u16 manifest_size;
        u32 found = 0;
+       bool result = false;
 
        /* we have to have at _least_ the manifest header */
        if (size <= sizeof(manifest->header)) {
                pr_err("short manifest (%zu)\n", size);
-               return NULL;
+               return false;
        }
 
        /* Make sure the size is right */
@@ -349,7 +346,7 @@ struct gb_module *gb_manifest_parse(void *data, size_t size)
        if (manifest_size != size) {
                pr_err("manifest size mismatch %zu != %hu\n",
                        size, manifest_size);
-               return NULL;
+               return false;
        }
 
        /* Validate major/minor number */
@@ -357,7 +354,7 @@ struct gb_module *gb_manifest_parse(void *data, size_t size)
                pr_err("manifest version too new (%hhu.%hhu > %hhu.%hhu)\n",
                        header->version_major, header->version_minor,
                        GREYBUS_VERSION_MAJOR, GREYBUS_VERSION_MINOR);
-               return NULL;
+               return false;
        }
 
        /* OK, find all the descriptors */
@@ -370,7 +367,7 @@ struct gb_module *gb_manifest_parse(void *data, size_t size)
                if (desc_size <= 0) {
                        if (!desc_size)
                                pr_err("zero-sized manifest descriptor\n");
-                       goto out_err;
+                       goto out;
                }
                desc = (struct greybus_descriptor *)((char *)desc + desc_size);
                size -= desc_size;
@@ -385,24 +382,20 @@ struct gb_module *gb_manifest_parse(void *data, size_t size)
        if (found != 1) {
                pr_err("manifest must have 1 module descriptor (%u found)\n",
                        found);
-               goto out_err;
+               goto out;
        }
 
        /* Parse the module manifest, starting with the module descriptor */
-       gmod = gb_manifest_parse_module(module_desc);
+       result = gb_manifest_parse_module(gmod, module_desc);
 
        /*
         * We really should have no remaining descriptors, but we
         * don't know what newer format manifests might leave.
         */
-       if (!list_empty(&manifest_descs)) {
+       if (!list_empty(&manifest_descs))
                pr_info("excess descriptors in module manifest\n");
-               release_manifest_descriptors();
-       }
-
-       return gmod;
-out_err:
+out:
        release_manifest_descriptors();
 
-       return NULL;
+       return false;
 }

diff --git a/drivers/staging/greybus/manifest.h b/drivers/staging/greybus/manifest.h
index 29cbf92bc2cf626b57af1aba072f027b97dc3c28..a1fe2c1281ad9fc1a9f5f56729bacbf72cc8d2d0 100644 (file)
--- a/drivers/staging/greybus/manifest.h
+++ b/drivers/staging/greybus/manifest.h
@@ -9,6 +9,7 @@
 #ifndef __MANIFEST_H
 #define __MANIFEST_H
 
-struct gb_module *gb_manifest_parse(void *data, size_t size);
+struct gb_module;
+bool gb_manifest_parse(struct gb_module *gmod, void *data, size_t size);
 
 #endif /* __MANIFEST_H */