* RE: [PATCH 3/9] staging: fsl-mc: add device release callback
@ 2017-02-03 0:02 Stuart Yoder
2017-02-03 15:53 ` Laurentiu Tudor
0 siblings, 1 reply; 3+ messages in thread
From: Stuart Yoder @ 2017-02-03 0:02 UTC (permalink / raw)
To: Laurentiu Tudor, gregkh
Cc: devel, arnd, Ruxandra Ioana Radulescu, Roy Pledge, linux-kernel,
agraf, Catalin Horghidan, Leo Li, Laurentiu Tudor
> -----Original Message-----
> From: upstream-release-bounces@linux.freescale.net [mailto:upstream-release-
> bounces@linux.freescale.net] On Behalf Of laurentiu.tudor@nxp.com
> Sent: Wednesday, February 01, 2017 5:43 AM
> To: gregkh@linuxfoundation.org
> Cc: devel@driverdev.osuosl.org; arnd@arndb.de; Ruxandra Ioana Radulescu <ruxandra.radulescu@nxp.com>;
> Roy Pledge <roy.pledge@nxp.com>; linux-kernel@vger.kernel.org; agraf@suse.de; Catalin Horghidan
> <catalin.horghidan@nxp.com>; Leo Li <leoyang.li@nxp.com>; Stuart Yoder <stuart.yoder@nxp.com>;
> Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Subject: [upstream-release] [PATCH 3/9] staging: fsl-mc: add device release callback
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> 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 <laurentiu.tudor@nxp.com>
> ---
> 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?
Stuart
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/9] staging: fsl-mc: add device release callback
2017-02-03 0:02 [PATCH 3/9] staging: fsl-mc: add device release callback Stuart Yoder
@ 2017-02-03 15:53 ` Laurentiu Tudor
0 siblings, 0 replies; 3+ messages in thread
From: Laurentiu Tudor @ 2017-02-03 15:53 UTC (permalink / raw)
To: Stuart Yoder, gregkh
Cc: devel, arnd, Ruxandra Ioana Radulescu, Roy Pledge, linux-kernel,
agraf, Catalin Horghidan, Leo Li
On 02/03/2017 02:02 AM, Stuart Yoder wrote:
>
>> -----Original Message-----
>> From: upstream-release-bounces@linux.freescale.net [mailto:upstream-release-
>> bounces@linux.freescale.net] On Behalf Of laurentiu.tudor@nxp.com
>> Sent: Wednesday, February 01, 2017 5:43 AM
>> To: gregkh@linuxfoundation.org
>> Cc: devel@driverdev.osuosl.org; arnd@arndb.de; Ruxandra Ioana Radulescu <ruxandra.radulescu@nxp.com>;
>> Roy Pledge <roy.pledge@nxp.com>; linux-kernel@vger.kernel.org; agraf@suse.de; Catalin Horghidan
>> <catalin.horghidan@nxp.com>; Leo Li <leoyang.li@nxp.com>; Stuart Yoder <stuart.yoder@nxp.com>;
>> Laurentiu Tudor <laurentiu.tudor@nxp.com>
>> Subject: [upstream-release] [PATCH 3/9] staging: fsl-mc: add device release callback
>>
>> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>
>> 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 <laurentiu.tudor@nxp.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 3/9] staging: fsl-mc: add device release callback
2017-02-01 11:43 [PATCH 0/9] staging: fsl-mc: fixes and cleanups laurentiu.tudor
@ 2017-02-01 11:43 ` laurentiu.tudor
0 siblings, 0 replies; 3+ messages in thread
From: laurentiu.tudor @ 2017-02-01 11:43 UTC (permalink / raw)
To: gregkh
Cc: devel, linux-kernel, agraf, arnd, ioana.ciornei,
ruxandra.radulescu, bharat.bhushan, stuart.yoder,
catalin.horghidan, leoyang.li, roy.pledge, Laurentiu Tudor
From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
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 <laurentiu.tudor@nxp.com>
---
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) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-03 15:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-03 0:02 [PATCH 3/9] staging: fsl-mc: add device release callback Stuart Yoder
2017-02-03 15:53 ` Laurentiu Tudor
-- strict thread matches above, loose matches on Subject: below --
2017-02-01 11:43 [PATCH 0/9] staging: fsl-mc: fixes and cleanups laurentiu.tudor
2017-02-01 11:43 ` [PATCH 3/9] staging: fsl-mc: add device release callback laurentiu.tudor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).