On 02/03/2017 02:02 AM, Stuart Yoder wrote: > >> -----Original Message----- >> From: [email protected] [mailto:upstream-release- >> [email protected]] On Behalf Of [email protected] >> Sent: Wednesday, February 01, 2017 5:43 AM >> To: [email protected] >> Cc: [email protected]; [email protected]; Ruxandra Ioana Radulescu >> <[email protected]>; >> Roy Pledge <[email protected]>; [email protected]; >> [email protected]; Catalin Horghidan >> <[email protected]>; Leo Li <[email protected]>; Stuart Yoder >> <[email protected]>; >> Laurentiu Tudor <[email protected]> >> Subject: [upstream-release] [PATCH 3/9] staging: fsl-mc: add device release >> callback >> >> From: Laurentiu Tudor <[email protected]> >> >> When hot unplugging a mc-bus device the kernel displays >> this pertinent message, followed by a stack dump: >> "Device 'foo.N' does not have a release() function, >> it is broken and must be fixed." >> Add the required callback to fix. >> >> Signed-off-by: Laurentiu Tudor <[email protected]> >> --- >> drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> index 7c6a43b..6601bde 100644 >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c >> @@ -419,6 +419,22 @@ bool fsl_mc_is_root_dprc(struct device *dev) >> return dev == root_dprc_dev; >> } >> >> +static void fsl_mc_device_release(struct device *dev) >> +{ >> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev); >> + struct fsl_mc_bus *mc_bus = NULL; >> + >> + kfree(mc_dev->regions); >> + >> + if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) >> + mc_bus = to_fsl_mc_bus(mc_dev); >> + >> + if (mc_bus) >> + devm_kfree(mc_dev->dev.parent, mc_bus); >> + else >> + kmem_cache_free(mc_dev_cache, mc_dev); >> +} >> + >> /** >> * Add a newly discovered fsl-mc device to be visible in Linux >> */ >> @@ -460,6 +476,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, >> device_initialize(&mc_dev->dev); >> mc_dev->dev.parent = parent_dev; >> mc_dev->dev.bus = &fsl_mc_bus_type; >> + mc_dev->dev.release = fsl_mc_device_release; >> dev_set_name(&mc_dev->dev, "%s.%d", obj_desc->type, obj_desc->id); >> >> if (strcmp(obj_desc->type, "dprc") == 0) { >> -- > > With this patch applied, you still have this: > > void fsl_mc_device_remove(struct fsl_mc_device *mc_dev) > { > struct fsl_mc_bus *mc_bus = NULL; > > kfree(mc_dev->regions); > > /* > * The device-specific remove callback will get invoked by > device_del() > */ > device_del(&mc_dev->dev); > put_device(&mc_dev->dev); > > if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) > mc_bus = to_fsl_mc_bus(mc_dev); > > if (mc_bus) > devm_kfree(mc_dev->dev.parent, mc_bus); > else > kmem_cache_free(mc_dev_cache, mc_dev); > } > > ...i.e. you are doing the same thing in 2 places. You > need to remove the kfree/devm_kfree/ kmem_cache_free, > here, no? >
Right, thanks for spotting. I started working on a v2 of the series. --- Best Regards, Laurentiu _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
