fix PCTV 461e tuner I2C binding

fix PCTV 461e tuner I2C binding

[Shuah Khan]

Mauro/Antti,

I have been looking at the following commit to

drivers/media/usb/em28xx/em28xx-dvb.c

a83b93a7480441a47856dc9104bea970e84cda87

+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1630,6 +1630,7 @@ static int em28xx_dvb_resume(struct em28xx *dev)
         em28xx_info("Resuming DVB extension");
         if (dev->dvb) {
                 struct em28xx_dvb *dvb = dev->dvb;
+               struct i2c_client *client = dvb->i2c_client_tuner;

                 if (dvb->fe[0]) {
                         ret = dvb_frontend_resume(dvb->fe[0]);
@@ -1640,6 +1641,15 @@ static int em28xx_dvb_resume(struct em28xx *dev)
                         ret = dvb_frontend_resume(dvb->fe[1]);
                         em28xx_info("fe1 resume %d", ret);
                 }
+               /* remove I2C tuner */
+               if (client) {
+                       module_put(client->dev.driver->owner);
+                       i2c_unregister_device(client);
+               }
+
+               em28xx_unregister_dvb(dvb);
+               kfree(dvb);
+               dev->dvb = NULL;
         }

Why are we unregistering i2c devices and dvb in this resume path?
Looks incorrect to me.

-- Shuah

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

Re: fix PCTV 461e tuner I2C binding

[Antti Palosaari]

Moikka Shuah

On 07/08/2014 02:34 AM, Shuah Khan wrote:
> Mauro/Antti,
>
> I have been looking at the following commit to
>
> drivers/media/usb/em28xx/em28xx-dvb.c
>
> a83b93a7480441a47856dc9104bea970e84cda87
>
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -1630,6 +1630,7 @@ static int em28xx_dvb_resume(struct em28xx *dev)
>          em28xx_info("Resuming DVB extension");
>          if (dev->dvb) {
>                  struct em28xx_dvb *dvb = dev->dvb;
> +               struct i2c_client *client = dvb->i2c_client_tuner;
>
>                  if (dvb->fe[0]) {
>                          ret = dvb_frontend_resume(dvb->fe[0]);
> @@ -1640,6 +1641,15 @@ static int em28xx_dvb_resume(struct em28xx *dev)
>                          ret = dvb_frontend_resume(dvb->fe[1]);
>                          em28xx_info("fe1 resume %d", ret);
>                  }
> +               /* remove I2C tuner */
> +               if (client) {
> +                       module_put(client->dev.driver->owner);
> +                       i2c_unregister_device(client);
> +               }
> +
> +               em28xx_unregister_dvb(dvb);
> +               kfree(dvb);
> +               dev->dvb = NULL;
>          }
>
> Why are we unregistering i2c devices and dvb in this resume path?
> Looks incorrect to me.

I don't know. Original patch I send was a bit different and tuner was 
removed only during em28xx_dvb_fini()

https://patchwork.linuxtv.org/patch/22275/

regards
Antti

-- 
http://palosaari.fi/

Re: fix PCTV 461e tuner I2C binding

[Shuah Khan]

Moikka Antti,

On 07/07/2014 09:45 PM, Antti Palosaari wrote:
> Moikka Shuah
>

>> Why are we unregistering i2c devices and dvb in this resume path?
>> Looks incorrect to me.
>
> I don't know. Original patch I send was a bit different and tuner was
> removed only during em28xx_dvb_fini()
>
> https://patchwork.linuxtv.org/patch/22275/
>

Yes. That's what I suspected. My patch and yours got munged somehow.
I will send a fix in.

btw: thanks for teaching me how to say hello in Finnish

-- Shuah



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

Re: fix PCTV 461e tuner I2C binding

[Antti Palosaari]

On 07/08/2014 04:55 PM, Shuah Khan wrote:
> Moikka Antti,
>
> On 07/07/2014 09:45 PM, Antti Palosaari wrote:
>> Moikka Shuah
>>
>
>>> Why are we unregistering i2c devices and dvb in this resume path?
>>> Looks incorrect to me.
>>
>> I don't know. Original patch I send was a bit different and tuner was
>> removed only during em28xx_dvb_fini()
>>
>> https://patchwork.linuxtv.org/patch/22275/
>>
>
> Yes. That's what I suspected. My patch and yours got munged somehow.
> I will send a fix in.

There has been merge conflict and that is end result. None has reported 
that bug so far. Likely it is very rare users suspend/resume these 
devices as DVB suspend/resume has been largely broken always...

> btw: thanks for teaching me how to say hello in Finnish

:=)

regards
Antti

-- 
http://palosaari.fi/

Re: fix PCTV 461e tuner I2C binding

[Shuah Khan]

On 07/08/2014 12:51 PM, Antti Palosaari wrote:
> On 07/08/2014 04:55 PM, Shuah Khan wrote:
>> Moikka Antti,
>>
>> On 07/07/2014 09:45 PM, Antti Palosaari wrote:
>>> Moikka Shuah
>>>
>>
>>>> Why are we unregistering i2c devices and dvb in this resume path?
>>>> Looks incorrect to me.
>>>
>>> I don't know. Original patch I send was a bit different and tuner was
>>> removed only during em28xx_dvb_fini()
>>>
>>> https://patchwork.linuxtv.org/patch/22275/
>>>
>>
>> Yes. That's what I suspected. My patch and yours got munged somehow.
>> I will send a fix in.
>
> There has been merge conflict and that is end result. None has reported
> that bug so far. Likely it is very rare users suspend/resume these
> devices as DVB suspend/resume has been largely broken always...
>

Somebody reported last week on v4l2 irc that suspend/resume not
working starting 3.15. That's what got me started on looking at
the code closely.

It should be fixed anyways. :)

-- Shuah


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