[linux-dvb] [PATCH] Siemens DVB-C PCI: SAA7113 Analog Module Extension: Fix missing Video (CVBS+Y/C) Inputs in AV7111X V4L driver

[Johannes Stezenbach]

thomas schorpp wrote:
> this patch enables the before not implemented video inputs of the
> SAA7113 Analog Cable Tuner Extension Module of that "classic" dvb-c card
> listed:
> 
> http://www.vdr-wiki.de/wiki/index.php/DVB-C_full-featured-Karten#Fujitsu-Siemens_DVB-C
> 
> - tested O.K. with original Siemens PCI Card + CI + Analog Module
> - tested O.K. with xawtv (latest 3.xx release at this time)
> - tested O.K. with gnomemeeting (v4l1 only)
> - tested O.K. with tvtime 0.9x (NOT OK if tuner is accessed! be careful)
> 
> - not tested with vdr-analog patches, yet. supported?
> - not tested the Y/C input configuration, is guessed from datasheet.
> 
> signed-off-by: t.schorpp at gmx.de

Thanks. I haven't had time to test this yet. Just a few comments:


> @@ -212,11 +228,17 @@
>  	}
> 
>  	if (0 != av7110->current_input) {
> +		

This adds trailing whitespace. (also in a couple of other places)

> +		dprintk(1, "switching to analog TV: \n");

printk with trailing space before the \n is also not cool

>  		adswitch = 1;
>  		source = SAA7146_HPS_SOURCE_PORT_B;
>  		sync = SAA7146_HPS_SYNC_PORT_B;
>  		memcpy(standard, analog_standard, sizeof(struct saa7146_standard) * 2);
> -		dprintk(1, "switching to analog TV\n");
> +		
> +		switch (av7110->current_input) {
> +		case 1:
> +		{
> +		dprintk(1, "switching SAA7113 to Analog Tuner Input.\n");
>  		msp_writereg(av7110, MSP_WR_DSP, 0x0008, 0x0000); // loudspeaker source
>  		msp_writereg(av7110, MSP_WR_DSP, 0x0009, 0x0000); // headphone source
>  		msp_writereg(av7110, MSP_WR_DSP, 0x000a, 0x0000); // SCART 1 source

indentation is broken; the style rules for switch/case are relatively
flexible, but other code in the same file indents like this:

		switch (av7110->current_input) {
		case 1:
		{
			dprintk(1, "switching SAA7113 to Analog Tuner Input.\n");
			...
			break;
		}
		case foo:


> @@ -231,6 +253,37 @@
>  			saa7146_setgpio(dev, 1, SAA7146_GPIO_OUTHI); // TDA9198 pin9(STD)
>  			saa7146_setgpio(dev, 3, SAA7146_GPIO_OUTHI); // TDA9198 pin30(VIF)
>  		}
> +		
> +		if (i2c_writereg(av7110, 0x48, 0x02, 0xd0) != 1) {
> +			dprintk(1, "saa7113 write failed @ card %d", av7110->dvb_adapter.num);
> +		}

no braces for one-line if()s (repeats a few times)

> +		case 2:
> +		{
> +		if (i2c_writereg(av7110, 0x48, 0x02, 0xd2) != 1) {
> +			dprintk(1, "saa7113 write failed @ card %d", av7110->dvb_adapter.num);
> +		}

no this gets confusing: indentation + no {} for the if


Johannes