Fix regression introduced which broke the Hauppauge USBLive 2

Commit Message

Mauro Carvalho Chehab July 24, 2011, 12:57 p.m. UTC

Hi Devin,

Em 23-07-2011 22:17, Devin Heitmueller escreveu:
> The following patch addresses the regression introduced in the cx231xx
> driver which stopped the Hauppauge USBLive2 from working.
> 
> Confirmed working by both myself and the user who reported the issue
> on the KernelLabs blog (Robert DeLuca).
> 
> cx231xx: Fix regression introduced which broke the Hauppauge USBLive 2
> 
> From: Devin Heitmueller <[e-mail]>
> 
> At some point during refactoring of the cx231xx driver, the USBLive 2 device
> became broken.  This patch results in the device working again.
> 
> Thanks to Robert DeLuca for sponsoring this work.
> 
> Signed-off-by: Devin Heitmueller <[e-mail]>
> Cc: Robert DeLuca <[e-mail]>
> 
> diff --git a/drivers/media/video/cx231xx/cx231xx-cards.c
> b/drivers/media/video/cx231xx/cx231xx-cards.c
> index 4b22afe..d02c63a 100644
> --- a/drivers/media/video/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/video/cx231xx/cx231xx-cards.c
> @@ -387,6 +387,7 @@ struct cx231xx_board cx231xx_boards[] = {
>  		.norm = V4L2_STD_NTSC,
>  		.no_alt_vanc = 1,
>  		.external_av = 1,
> +		.dont_use_port_3 = 1,
>  		.input = {{
>  			.type = CX231XX_VMUX_COMPOSITE1,
>  			.vmux = CX231XX_VIN_2_1,

I proposed the same fix sometime ago, when Gerd reported this issue
for me. His feedback was that this partially fixed the issue, but
he reported that he also needed to increase the set_power_mode delay
from 5 to 50 ms in order to fully initialize the cx231xx hardware,
as on the enclosed patch. It seems he tested with vanilla Fedora kernel.

So, I suspect that HZ may be affecting this driver as well. As you know,
if HZ is configured with 100, the minimum msleep() delay will be 10.
so, instead of waiting for 5ms, it will wait for 10ms for the device
to powerup.

It would be great to configure HZ with 1000 and see the differences with 
and without Gerd's patch.

Cheers,
Mauro.

-

From a83a7574e07b48b1c6a222d833a7fa0a67133b5c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <[e-mail]>
Date: Thu, 16 Dec 2010 17:39:17 +0100
Subject: [PATCH] cx231xx: raise delay after powerup.

Wait a bit longer after power up so the chips have enougth
time to come up before we try talking to them via i2c.

Signed-off-by: Gerd Hoffmann <[e-mail]>
---
 drivers/media/video/cx231xx/cx231xx-avcore.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Devin Heitmueller July 24, 2011, 1:02 p.m. UTC | #1

On Sun, Jul 24, 2011 at 8:57 AM, Mauro Carvalho Chehab
<[e-mail]> wrote:
> I proposed the same fix sometime ago, when Gerd reported this issue
> for me. His feedback was that this partially fixed the issue, but
> he reported that he also needed to increase the set_power_mode delay
> from 5 to 50 ms in order to fully initialize the cx231xx hardware,
> as on the enclosed patch. It seems he tested with vanilla Fedora kernel.
>
> So, I suspect that HZ may be affecting this driver as well. As you know,
> if HZ is configured with 100, the minimum msleep() delay will be 10.
> so, instead of waiting for 5ms, it will wait for 10ms for the device
> to powerup.
>
> It would be great to configure HZ with 1000 and see the differences with
> and without Gerd's patch.
>
> Cheers,
> Mauro.

I don't dispute the possibility that there is some *other* bug that
effects users who have some other value for HZ, but neither I nor the
other use saw it.  Without this patch though, the device is broken for
*everybody*.

I would suggest checking in this patch, and separately the HZ issue
can be investigated.

I'll see if I can find some cycles today to reconfigure my kernel with
a different HZ.  Will also check the datasheets and see if Conexant
documented any sort of time for power ramping.  It's not uncommon for
such documentation to include a diagram showing timing for power up.

Devin

Devin Heitmueller July 24, 2011, 1:50 p.m. UTC | #2

On Sun, Jul 24, 2011 at 9:02 AM, Devin Heitmueller
<[e-mail]> wrote:
> I don't dispute the possibility that there is some *other* bug that
> effects users who have some other value for HZ, but neither I nor the
> other use saw it.  Without this patch though, the device is broken for
> *everybody*.
>
> I would suggest checking in this patch, and separately the HZ issue
> can be investigated.
>
> I'll see if I can find some cycles today to reconfigure my kernel with
> a different HZ.  Will also check the datasheets and see if Conexant
> documented any sort of time for power ramping.  It's not uncommon for
> such documentation to include a diagram showing timing for power up.

Reconfigured CONFIG_HZ to 1000 and indeed I am seeing the problem.
Will try to track down the correct fix this afternoon.

Patch

diff --git a/drivers/media/video/cx231xx/cx231xx-avcore.c b/drivers/media/video/cx231xx/cx231xx-avcore.c
index cf50faf..cf412cd 100644
--- a/drivers/media/video/cx231xx/cx231xx-avcore.c
+++ b/drivers/media/video/cx231xx/cx231xx-avcore.c
@@ -2412,7 +2412,7 @@  int cx231xx_set_power_mode(struct cx231xx *dev, enum AV_MODE mode)
 		break;
 	}
 
-	msleep(PWR_SLEEP_INTERVAL);
+	msleep(PWR_SLEEP_INTERVAL * 10);
 
 	/* For power saving, only enable Pwr_resetout_n
 	   when digital TV is selected. */