Am 06.12.2015 um 14:31 schrieb Paul Bolle: > On wo, 2015-12-02 at 18:48 -0500, Peter Hurley wrote: >> On 11/30/2015 01:01 PM, Paul Bolle wrote:
>>> --- a/drivers/isdn/gigaset/ser-gigaset.c
>>> +++ b/drivers/isdn/gigaset/ser-gigaset.c
>>> @@ -42,8 +42,9 @@ MODULE_PARM_DESC(cidmode, "stay in CID mode when
>>> idle");
>>>
>>> static struct gigaset_driver *driver;
>>>
>>> +static struct platform_device pdev;
>>> +
>>> struct ser_cardstate {
>>> - struct platform_device dev;
>>> struct tty_struct *tty;
>>> atomic_t refcnt;
>>> struct completion dead_cmp;
>>> @@ -370,8 +371,8 @@ static void gigaset_freecshw(struct cardstate
>>> *cs)
>>> tasklet_kill(&cs->write_tasklet);
>>> if (!cs->hw.ser)
>>> return;
>>> - dev_set_drvdata(&cs->hw.ser->dev.dev, NULL);
>>> - platform_device_unregister(&cs->hw.ser->dev);
>>> + dev_set_drvdata(&pdev.dev, NULL);
>>> + platform_device_unregister(&pdev);
>>> kfree(cs->hw.ser);
>>
>> Tilman,
>>
>> Is there a 1:1 correspondence and lifetime for the embedded platform
>> device and it's containing memory?
>
> (Haven't heard from Tilman, so I'll give this a try.)
Sorry for that. Been busy.
> That containing memory is a struct ser_cardstate. And currently
> instances of struct _ser_cardstate are malloced and freed in routines
> that also call platform_device_register() and
> platform_device_unregister(). So yes, I think there's a 1:1
> correspondence.
Correct.
>> I ask because the typical approach for device teardown is to put the
>> kfree() in the release method;
>
> (Side note: the (struct device) release method of this driver
> -gigaset_device_release() - is actually a nop. It only frees device
> ->platform_data and platform_device->resource, but neither are actually
> used: they remain NULL through their entire life.)
Yeah, that was just copied unthinkingly from driver/base/platform.c.
So the solution might be as simple as moving the kfree() call from
gigaset_freecshw() to gigaset_device_release(). Something like this:
--- a/drivers/isdn/gigaset/ser-gigaset.c
+++ b/drivers/isdn/gigaset/ser-gigaset.c
@@ -370,19 +370,18 @@ static void gigaset_freecshw(struct cardstate *cs)
tasklet_kill(&cs->write_tasklet);
if (!cs->hw.ser)
return;
- dev_set_drvdata(&cs->hw.ser->dev.dev, NULL);
platform_device_unregister(&cs->hw.ser->dev);
- kfree(cs->hw.ser);
- cs->hw.ser = NULL;
}
static void gigaset_device_release(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
+ struct cardstate *cs = dev_get_drvdata(dev);
- /* adapted from platform_device_release() in
drivers/base/platform.c */
- kfree(dev->platform_data);
- kfree(pdev->resource);
+ if (!cs)
+ return;
+ dev_set_drvdata(dev, NULL);
+ kfree(cs->hw.ser);
+ cs->hw.ser = NULL;
}
/*
(Off the top of my hat, completely untested, don't even know if that
will compile.)
--
Tilman Schmidt E-Mail: til...@imap.cc
Bonn, Germany
Nous, on a des fleurs et des bougies pour nous protéger.
signature.asc
Description: OpenPGP digital signature
