[PATCH] media: em28xx-dvb unregister i2c tuner and demod after fe detach

[PATCH] media: em28xx-dvb unregister i2c tuner and demod after fe detach

[Shuah Khan]

i2c tuner and demod are unregisetred in .fini before fe detach.
dvb_unregister_frontend() and dvb_frontend_detach() invoke tuner
sleep() and release() interfaces. Change to unregister i2c tuner
and demod from em28xx_unregister_dvb() after unregistering dvb
and detaching fe.

Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-dvb.c |   32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 8314f51..8d5cb62 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1030,6 +1030,8 @@ fail_adapter:
 
 static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
 {
+	struct i2c_client *client;
+
 	dvb_net_release(&dvb->net);
 	dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_mem);
 	dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_hw);
@@ -1041,6 +1043,21 @@ static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
 	if (dvb->fe[1] && !dvb->dont_attach_fe1)
 		dvb_frontend_detach(dvb->fe[1]);
 	dvb_frontend_detach(dvb->fe[0]);
+
+	/* remove I2C tuner */
+	client = dvb->i2c_client_tuner;
+	if (client) {
+		module_put(client->dev.driver->owner);
+		i2c_unregister_device(client);
+	}
+
+	/* remove I2C demod */
+	client = dvb->i2c_client_demod;
+	if (client) {
+		module_put(client->dev.driver->owner);
+		i2c_unregister_device(client);
+	}
+
 	dvb_unregister_adapter(&dvb->adapter);
 }
 
@@ -1628,7 +1645,6 @@ static inline void prevent_sleep(struct dvb_frontend_ops *ops)
 static int em28xx_dvb_fini(struct em28xx *dev)
 {
 	struct em28xx_dvb *dvb;
-	struct i2c_client *client;
 
 	if (dev->is_audio_only) {
 		/* Shouldn't initialize IR for this interface */
@@ -1646,7 +1662,6 @@ static int em28xx_dvb_fini(struct em28xx *dev)
 	em28xx_info("Closing DVB extension");
 
 	dvb = dev->dvb;
-	client = dvb->i2c_client_tuner;
 
 	em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
 
@@ -1659,19 +1674,6 @@ static int em28xx_dvb_fini(struct em28xx *dev)
 			prevent_sleep(&dvb->fe[1]->ops);
 	}
 
-	/* remove I2C tuner */
-	if (client) {
-		module_put(client->dev.driver->owner);
-		i2c_unregister_device(client);
-	}
-
-	/* remove I2C demod */
-	client = dvb->i2c_client_demod;
-	if (client) {
-		module_put(client->dev.driver->owner);
-		i2c_unregister_device(client);
-	}
-
 	em28xx_unregister_dvb(dvb);
 	kfree(dvb);
 	dev->dvb = NULL;
-- 
1.7.10.4

Re: [PATCH] media: em28xx-dvb unregister i2c tuner and demod after fe detach

[Antti Palosaari]

Moikka Shuah!
I suspect that patch makes no sense. On DVB there is runtime PM 
controlled by DVB frontend. It wakes up all FE sub-devices when frontend 
device is opened and sleeps when closed.

FE release() is not relevant at all for those sub-devices which are 
implemented as a proper I2C client. I2C client has own remove() for that.

em28xx_dvb_init and em28xx_dvb_fini are counterparts. Those I2C drivers 
are load on em28xx_dvb_init so logical place for unload is em28xx_dvb_fini.

Is there some real use case you need that change?

regards
Antti


On 07/11/2014 06:45 PM, Shuah Khan wrote:
> i2c tuner and demod are unregisetred in .fini before fe detach.
> dvb_unregister_frontend() and dvb_frontend_detach() invoke tuner
> sleep() and release() interfaces. Change to unregister i2c tuner
> and demod from em28xx_unregister_dvb() after unregistering dvb
> and detaching fe.
>
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
>   drivers/media/usb/em28xx/em28xx-dvb.c |   32 +++++++++++++++++---------------
>   1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
> index 8314f51..8d5cb62 100644
> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -1030,6 +1030,8 @@ fail_adapter:
>
>   static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
>   {
> +	struct i2c_client *client;
> +
>   	dvb_net_release(&dvb->net);
>   	dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_mem);
>   	dvb->demux.dmx.remove_frontend(&dvb->demux.dmx, &dvb->fe_hw);
> @@ -1041,6 +1043,21 @@ static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
>   	if (dvb->fe[1] && !dvb->dont_attach_fe1)
>   		dvb_frontend_detach(dvb->fe[1]);
>   	dvb_frontend_detach(dvb->fe[0]);
> +
> +	/* remove I2C tuner */
> +	client = dvb->i2c_client_tuner;
> +	if (client) {
> +		module_put(client->dev.driver->owner);
> +		i2c_unregister_device(client);
> +	}
> +
> +	/* remove I2C demod */
> +	client = dvb->i2c_client_demod;
> +	if (client) {
> +		module_put(client->dev.driver->owner);
> +		i2c_unregister_device(client);
> +	}
> +
>   	dvb_unregister_adapter(&dvb->adapter);
>   }
>
> @@ -1628,7 +1645,6 @@ static inline void prevent_sleep(struct dvb_frontend_ops *ops)
>   static int em28xx_dvb_fini(struct em28xx *dev)
>   {
>   	struct em28xx_dvb *dvb;
> -	struct i2c_client *client;
>
>   	if (dev->is_audio_only) {
>   		/* Shouldn't initialize IR for this interface */
> @@ -1646,7 +1662,6 @@ static int em28xx_dvb_fini(struct em28xx *dev)
>   	em28xx_info("Closing DVB extension");
>
>   	dvb = dev->dvb;
> -	client = dvb->i2c_client_tuner;
>
>   	em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
>
> @@ -1659,19 +1674,6 @@ static int em28xx_dvb_fini(struct em28xx *dev)
>   			prevent_sleep(&dvb->fe[1]->ops);
>   	}
>
> -	/* remove I2C tuner */
> -	if (client) {
> -		module_put(client->dev.driver->owner);
> -		i2c_unregister_device(client);
> -	}
> -
> -	/* remove I2C demod */
> -	client = dvb->i2c_client_demod;
> -	if (client) {
> -		module_put(client->dev.driver->owner);
> -		i2c_unregister_device(client);
> -	}
> -
>   	em28xx_unregister_dvb(dvb);
>   	kfree(dvb);
>   	dev->dvb = NULL;
>

-- 
http://palosaari.fi/

Re: [PATCH] media: em28xx-dvb unregister i2c tuner and demod after fe detach

[Shuah Khan]

On 07/12/2014 02:14 PM, Antti Palosaari wrote:
> Moikka Shuah!
> I suspect that patch makes no sense. On DVB there is runtime PM
> controlled by DVB frontend. It wakes up all FE sub-devices when frontend
> device is opened and sleeps when closed.
>
> FE release() is not relevant at all for those sub-devices which are
> implemented as a proper I2C client. I2C client has own remove() for that.
>
> em28xx_dvb_init and em28xx_dvb_fini are counterparts. Those I2C drivers
> are load on em28xx_dvb_init so logical place for unload is em28xx_dvb_fini.
>
> Is there some real use case you need that change?
>
> regards
> Antti
>

Hi Antti,

The reason I made this change is because dvb_frontend_detach()
calls release interfaces for fe as well as tuner. So it made
sense to move the remove after that is all done. Are you saying
fe and tuner release calls aren't relevant when sub-devices
implement a proper i2c client? If that is the case then, and
there is no chance for these release calls to be invoked when a
proper i2c is present, then my patch isn't needed.

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

Re: [PATCH] media: em28xx-dvb unregister i2c tuner and demod after fe detach

[Antti Palosaari]

Moikka!

On 07/15/2014 08:29 PM, Shuah Khan wrote:
> On 07/12/2014 02:14 PM, Antti Palosaari wrote:
>> Moikka Shuah!
>> I suspect that patch makes no sense. On DVB there is runtime PM
>> controlled by DVB frontend. It wakes up all FE sub-devices when frontend
>> device is opened and sleeps when closed.
>>
>> FE release() is not relevant at all for those sub-devices which are
>> implemented as a proper I2C client. I2C client has own remove() for that.
>>
>> em28xx_dvb_init and em28xx_dvb_fini are counterparts. Those I2C drivers
>> are load on em28xx_dvb_init so logical place for unload is
>> em28xx_dvb_fini.
>>
>> Is there some real use case you need that change?
>>
>> regards
>> Antti
>>
>
> Hi Antti,
>
> The reason I made this change is because dvb_frontend_detach()
> calls release interfaces for fe as well as tuner. So it made
> sense to move the remove after that is all done. Are you saying
> fe and tuner release calls aren't relevant when sub-devices
> implement a proper i2c client? If that is the case then, and
> there is no chance for these release calls to be invoked when a
> proper i2c is present, then my patch isn't needed.

Yes, that is just case. Proprietary DVB binding model uses attach / 
release, but I2C binding model has probe / remove. I see no reason use 
DVB proprietary model, instead drivers should be converted to kernel I2C 
model.

regards
Antti

-- 
http://palosaari.fi/