Re: [2.6.16] saa7134 disable_ir oops

[Hartmut Hackmann <hartmut.hackmann@t-online.de>]

Hi,


Sergey Vlasov wrote:
> On Fri, Apr 07, 2006 at 10:16:10AM -0300, Mauro Carvalho Chehab wrote:
> 
>>Em Qui, 2006-04-06 ?s 20:20 +0400, Sergey Vlasov escreveu:
>>
>>>On Fri, 24 Mar 2006 14:00:46 -0800 David Liontooth wrote:
>>
>>>Does the following patch fix things?
>>>
>>
>>Applied at v4l-dvb tree. Thanks.
> 
> 
> IMHO this patch should also be added to 2.6.16-stable - it fixes oops in
> configurations which worked fine with older kernels.
> 
> -----------------------------------------------------------------------
> 
> saa7134: Fix oops with disable_ir=1
> 
> When disable_ir=1 parameter is used, or when saa7134_input_init1()
> fails for any other reason, dev->remote will remain NULL, and the
> driver will oops in saa7134_hwinit2().  Therefore dev->remote must be
> checked before dereferencing.
> 
> Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
> 
> --- linux-2.6.16.orig/drivers/media/video/saa7134/saa7134-core.c	2006-03-20 08:53:29 +0300
> +++ linux-2.6.16/drivers/media/video/saa7134/saa7134-core.c	2006-04-06 20:00:56 +0400
> @@ -543,6 +543,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO16) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if  (dev->remote->mask_keydown & 0x10000) {
>  						saa7134_input_irq(dev);
>  					}
> @@ -559,6 +561,8 @@ static irqreturn_t saa7134_irq(int irq, 
>  		if (report & SAA7134_IRQ_REPORT_GPIO18) {
>  			switch (dev->has_remote) {
>  				case SAA7134_REMOTE_GPIO:
> +					if (!dev->remote)
> +						break;
>  					if ((dev->remote->mask_keydown & 0x40000) ||
>  					    (dev->remote->mask_keyup & 0x40000)) {
>  						saa7134_input_irq(dev);
> @@ -671,7 +675,7 @@ static int saa7134_hwinit2(struct saa713
>  		SAA7134_IRQ2_INTE_PE      |
>  		SAA7134_IRQ2_INTE_AR;
>  
> -	if (dev->has_remote == SAA7134_REMOTE_GPIO) {
> +	if (dev->has_remote == SAA7134_REMOTE_GPIO && dev->remote) {
>  		if (dev->remote->mask_keydown & 0x10000)
>  			irq2_mask |= SAA7134_IRQ2_INTE_GPIO16;
>  		else if (dev->remote->mask_keydown & 0x40000)
> 

Let's think this over, please.
1) The problem originally was that a card type with remote control was
    forced for a card that doesn't have one.
2) On the other hand you are right, this situation should not cause an oops.
The code should be that GPIO irqs should be completely ignored unless a handler
has been installed, so the
  if (dev->has_remote)
is completely wrong and should be repaced by something explictly initialized,
maybe the dev->remote pointer does the trick better.
I never worked seriously on this fraction of the driver, so there might be a
better solution.

Comments?
    Hartmut