linux-kernel - Re: [V4L][SAA7134] fix tda9887 detection on cold and eeprom read corruption on warm Medion 7134

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 09 Nov 2010 16:02:44 +0100
From:	Maciej Szmigiero <mhej@...pl>
To:	Mauro Carvalho Chehab <mchehab@...radead.org>
CC:	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [V4L][SAA7134] fix tda9887 detection on cold and eeprom read
 corruption on warm Medion 7134

W dniu 09.11.2010 11:53, Mauro Carvalho Chehab pisze:
> Em 25-10-2010 15:59, Maciej Szmigiero escreveu:
>> +			printk(KERN_NOTICE "%s DVB-T demod i2c gate was left"
>> +						    " closed\n", dev->name);
>> +			printk(KERN_NOTICE "%s previous informational"
>> +					    " EEPROM read might have been"
>> +					    " corrupted\n", dev->name);
> 
> hmm... I don't think we need those debug messages on normal cases. 

I added this message because when the gate was left closed the eeprom content (printed out unconditionally in saa7134_i2c_eeprom) looks garbled in dmesg,
so it's better to inform user that he (or she) shouldn't be worried about this.
The eeprom dump is called from saa7134_i2c_register, before card-specific code has opportunity to run.

>>  	saa7134_tuner_setup(dev);
>>  
>> +	/* some cards (Medion 7134 for example) needs tuner to be setup */
>> +	/* before tda9887 shows itself on i2c bus */
>> +	if ((TUNER_ABSENT != dev->tuner_type)
>> +			&& (dev->tda9887_conf & TDA9887_PRESENT)) {
>> +		v4l2_i2c_new_subdev(&dev->v4l2_dev,
>> +			&dev->i2c_adap, "tuner", "tuner",
>> +			0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
>> +	}
>> +
>>  	switch (dev->board) {
>>  	case SAA7134_BOARD_BEHOLD_COLUMBUS_TVFM:
>>  	case SAA7134_BOARD_AVERMEDIA_CARDBUS_501:
> 
> The order change for the demod probe will likely break support for other boards.
> If the problem is specific to Medion 7134, what you should do, instead, is to
> change the order just for MD7134 (so, inside the switch(dev->board)).

It was done in that order (tuner first then tda9887) for a long time before it was changed.
For example ( http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/media/video/saa7134/saa7134-core.c;hb=7a766f9ddd74b50d6069f054a3004ece0439f5c1 ) it used to look like that:
 942         /* load i2c helpers */
 943         if (TUNER_ABSENT != dev->tuner_type)
 944                 request_module("tuner");
 945         if (dev->tda9887_conf)
 946                 request_module("tda9887");
 947         if (card_is_empress(dev)) {
 948                 request_module("saa6752hs");
 949                 request_module_depend("saa7134-empress",&need_empress);
 950         }

But then the code for tda9887 was integrated into tuner module and later split out again, this time reversing the detection order (by accident I suppose).

> Cheers,
> Mauro.
> 

Best regards,
Maciej Szmigiero
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/