From 937d0da83ffde769f24b5124d66f793045c5da10 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 3 Oct 2014 14:14:25 -0500 Subject: [PATCH] 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 Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/core.c | 13 ++++++-- drivers/staging/greybus/manifest.c | 63 +++++++++++++++++--------------------- drivers/staging/greybus/manifest.h | 3 +- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c index 40c8996..41fb7ca 100644 --- 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 ad7922a..03282fd 100644 --- 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 29cbf92..a1fe2c12 100644 --- 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 */ -- 2.7.4