Re: [PATCH 1/2] [media] rc-main: Fix device de-registration logic

[Mauro Carvalho Chehab <mchehab@redhat.com>]

Em 29-07-2011 14:30, Jarod Wilson escreveu:
> On Jul 29, 2011, at 1:53 AM, Mauro Carvalho Chehab wrote:
> 
>> rc unregister logic were deadly broken, preventing some drivers to
>> be removed. Among the broken things, rc_dev_uevent() is being called
>> during device_del(), causing a data filling on an area that it is
>> not ready anymore.
>>
>> Also, some drivers have a stop callback defined, that needs to be called
>> before data removal, as it stops data polling.
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
>>
>> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>> index 51a23f4..666d4bb 100644
>> --- a/drivers/media/rc/rc-main.c
>> +++ b/drivers/media/rc/rc-main.c
>> @@ -928,10 +928,6 @@ out:
>>
>> static void rc_dev_release(struct device *device)
>> {
>> -	struct rc_dev *dev = to_rc_dev(device);
>> -
>> -	kfree(dev);
>> -	module_put(THIS_MODULE);
>> }
> 
> Since this function become a no-op, does it make sense to just remove it
> and not set a .release function for static struct device_type rc_dev_type?

As you tested, this function needs to exist... well, other drivers sometimes
do the same, by defining it as a no-op function.

> Other than that, after reading through the patch several times, along with
> the resulting rc-main.c and some input code, everything seems to make
> sense to me. Will do some quick sanity-testing with a few of my various
> devices before I give an ack though, just to be sure. :)

Thanks! Yeah, a test with other devices is welcome, as we don't want fix for one
and break for the others ;)

The logic there looks simple, but it is, in fact, tricky, especially since
drivers may have polling tasks running, and they need to be cancelled before 
freeing the resources.

Cheers,
Mauro

>