Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
Em 30-09-2010 18:03, Michael Krufky escreveu: > On Thu, Sep 30, 2010 at 4:26 PM, Mauro Carvalho Chehab > wrote: >> Em 30-09-2010 16:27, Michael Krufky escreveu: >>> Mauro, >>> >>> I think that's a reasonable explanation. Would you be open to >>> reworking the patch such that the register contents only show up if >>> the device is not recognized? (when ret < 0) . In the case where the >>> device is correctly identified (ret == 0), I'd rather preserve the >>> original successful detection message, and not see the ID register >>> contents. >> >> Patch enclosed. >> >> --- >> >> [PATCH] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned >> >> Instead of doing: >> >> [ 82.581639] tda18271 4-0060: creating new instance >> [ 82.588411] Unknown device detected @ 4-0060, device not supported. >> [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 >> [ 82.600530] tda18271 4-0060: destroying instance >> >> Print: >> [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. >> >> for the error message, to help detecting what's going wrong with the >> device. >> >> This helps to detect when the driver is using the wrong I2C bus (or have >> the i2g gate switch pointing to the wrong place), on devices like cx231xx >> that just return 0 on reads to a non-existent i2c device. >> >> Signed-off-by: Mauro Carvalho Chehab >> >> diff --git a/drivers/media/common/tuners/tda18271-fe.c >> b/drivers/media/common/tuners/tda18271-fe.c >> index 7955e49..3db8727 100644 >> --- a/drivers/media/common/tuners/tda18271-fe.c >> +++ b/drivers/media/common/tuners/tda18271-fe.c >> @@ -1156,7 +1156,6 @@ static int tda18271_get_id(struct dvb_frontend *fe) >>struct tda18271_priv *priv = fe->tuner_priv; >>unsigned char *regs = priv->tda18271_regs; >>char *name; >> - int ret = 0; >> >>mutex_lock(&priv->lock); >>tda18271_read_regs(fe); >> @@ -1172,17 +1171,18 @@ static int tda18271_get_id(struct dvb_frontend *fe) >>priv->id = TDA18271HDC2; >>break; >>default: >> - name = "Unknown device"; >> - ret = -EINVAL; >> - break; >> + tda_info("Unknown device (%i) detected @ %d-%04x, device not >> supported.\n", >> +regs[R_ID], >> +i2c_adapter_id(priv->i2c_props.adap), >> +priv->i2c_props.addr); >> + return -EINVAL; >>} >> >> - tda_info("%s detected @ %d-%04x%s\n", name, >> + tda_info("%s detected @ %d-%04x\n", name, >> i2c_adapter_id(priv->i2c_props.adap), >> -priv->i2c_props.addr, >> -(0 == ret) ? "" : ", device not supported."); >> +priv->i2c_props.addr); >> >> - return ret; >> + return 0; >> } >> >> static int tda18271_setup_configuration(struct dvb_frontend *fe, >> >> > > This looks good to me, although you could concatenate these lines together: > > >> + tda_info("Unknown device (%i) detected @ %d-%04x, device not >> supported.\n", >> +regs[R_ID], >> +i2c_adapter_id(priv->i2c_props.adap), >> +priv->i2c_props.addr); >> + return -EINVAL; > > > to be more like this: > > >> + tda_info("Unknown device (%i) detected @ %d-%04x, device not >> supported.\n", >> +regs[R_ID], i2c_adapter_id(priv->i2c_props.adap), >> priv->i2c_props.addr); >> + return -EINVAL; > > > ...that is, if it fits within an 80-char line. All parameters after the format string don't fit on 80 cols. I did a small optimization on that. > > Anyway, > > Reviewed-by: Michael Krufky Thanks, committed. Cheers, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
Em 30-09-2010 19:07, Michael Krufky escreveu: > On Thu, Sep 30, 2010 at 6:00 PM, Antti Palosaari wrote: >> On 09/30/2010 09:52 PM, Michael Krufky wrote: >>> >>> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >>> wrote: By default, tda18271 tries to optimize I2C bus by updating all registers at the same time. Unfortunately, some devices doesn't support it. The current logic has a problem when small_i2c is equal to 8, since there are some transfers using 11 + 1 bytes. Fix the problem by enforcing the max size at the right place, and allows reducing it to max = 3 + 1. Signed-off-by: Mauro Carvalho Chehab >>> >>> This looks to me as if it is working around a problem on the i2c >>> master. I believe that a fix like this really belongs in the i2c >>> master driver, it should be able to break the i2c transactions down >>> into transactions that the i2c master can handle. >>> >>> I wouldn't want to merge this without a better explanation of why it >>> is necessary in the tda18271 driver. It seems to be a band-aid to >>> cover up a problem in the i2c master device driver code. >> >> Yes it is I2C provider limitation, but I think almost all I2C adapters have >> some limit. I suggest to set param for each tuner and demod driver which >> splits reads and writes to len adapter can handle. I did that for tda18218 >> write. >> >> But there is one major point you don't see. It is not simple to add this >> splitting limit to the provider. Provider does not have knowledge which is >> meaning of bytes it transfers to the bus. Without knowledge it breaks >> functionality surely in some point. There is commonly seen 1, 2 and 4 byte >> register address and same for register values. Also some chips like to send >> data as register-value pairs. > > Yes, I understand. We will likely merge Mauro's patch, I just want to > test it on my own hardware first, and I'd like to verify the cx231xx > i2c xfer limit issue that Mauro is reporting. I'll try to get back to > this early next week, or hopefully over this weekend. Feel free to test. Just remember that all Hauppauge devices at Devin's polaris4 tree uses I2C Channel #1. This seems to be the default on all Conexant SDK also. The device I'm working with has the tuner at channel #2, and the ISDB-T demod at channel #1. So, the limits may differ. On my device, everything seem to work fine with max size = 4 (both analog and digital modes). Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bttv: No analogue sound output by TV card
[Please CC me, I'm not on the list.] Hi, after switching from 2.6.34 to 2.6.36-rc5, the sound on my old Hauppauge analogue TV card has stopped working. Audio out from the TV card is connected to line-in of my motherboard (Gigabyte EG45M-DS2H) using the short cable that came with the TV card. Other audio sources (e.g. MP3 player) are audible when connected to line-in. The TV picture is fine. The log messages look really similar for the two kernels. 2.6.34 works: kernel: bttv: driver version 0.9.18 loaded kernel: bttv: using 8 buffers with 2080k (520 pages) each for capture kernel: bttv: Bt8xx card found (0). kernel: bttv0: Bt878 (rev 17) at :03:01.0, irq: 19, latency: 32, mmio: 0xe360 kernel: bttv0: detected: Hauppauge WinTV [card=10], PCI subsystem ID is 0070:13eb kernel: bttv0: using: Hauppauge (bt878) [card=10,autodetected] kernel: IRQ 19/bttv0: IRQF_DISABLED is not guaranteed on shared IRQs kernel: bttv0: gpio: en=, out= in=00db [init] kernel: bttv0: Hauppauge/Voodoo msp34xx: reset line init [5] kernel: tveeprom 1-0050: Hauppauge model 44354, rev D147, serial# 1234567 kernel: tveeprom 1-0050: tuner model is LG TP18PSB01D (idx 47, type 28) kernel: tveeprom 1-0050: TV standards PAL(B/G) (eeprom 0x04) kernel: tveeprom 1-0050: audio processor is MSP3415 (idx 6) kernel: tveeprom 1-0050: has radio kernel: bttv0: Hauppauge eeprom indicates model#44354 kernel: bttv0: tuner type=28 kernel: msp3400 1-0040: MSP3415D-B3 found @ 0x80 (bt878 #0 [sw]) kernel: msp3400 1-0040: msp3400 supports nicam, mode is autodetect kernel: tuner 1-0061: chip found @ 0xc2 (bt878 #0 [sw]) kernel: tuner-simple 1-0061: creating new instance kernel: tuner-simple 1-0061: type set to 28 (LG PAL_BG+FM (TPI8PSB01D)) kernel: bttv0: registered device video0 kernel: bttv0: registered device vbi0 kernel: bttv0: registered device radio0 kernel: bttv0: PLL: 28636363 => 35468950 . kernel: bttv0: PLL: 28636363 => 35468950 . ok kernel: ok 2.6.36-rc5 does not work: kernel: bttv: driver version 0.9.18 loaded kernel: bttv: using 8 buffers with 2080k (520 pages) each for capture kernel: bttv: Bt8xx card found (0). kernel: bttv0: Bt878 (rev 17) at :03:01.0, irq: 19, latency: 32, mmio: 0xe360 kernel: bttv0: detected: Hauppauge WinTV [card=10], PCI subsystem ID is 0070:13eb kernel: bttv0: using: Hauppauge (bt878) [card=10,autodetected] kernel: bttv0: gpio: en=, out= in=00db [init] kernel: bttv0: Hauppauge/Voodoo msp34xx: reset line init [5] kernel: tveeprom 1-0050: Hauppauge model 44354, rev D147, serial# 1234567 kernel: tveeprom 1-0050: tuner model is LG TP18PSB01D (idx 47, type 28) kernel: tveeprom 1-0050: TV standards PAL(B/G) (eeprom 0x04) kernel: tveeprom 1-0050: audio processor is MSP3415 (idx 6) kernel: tveeprom 1-0050: has radio kernel: bttv0: Hauppauge eeprom indicates model#44354 kernel: bttv0: tuner type=28 kernel: msp3400 1-0040: MSP3415D-B3 found @ 0x80 (bt878 #0 [sw]) kernel: msp3400 1-0040: msp3400 supports nicam, mode is autodetect kernel: tuner 1-0061: chip found @ 0xc2 (bt878 #0 [sw]) kernel: tuner-simple 1-0061: creating new instance kernel: tuner-simple 1-0061: type set to 28 (LG PAL_BG+FM (TPI8PSB01D)) kernel: bttv0: registered device video0 kernel: bttv0: registered device vbi0 kernel: bttv0: registered device radio0 kernel: bttv0: PLL: 28636363 => 35468950 . ok The only real change is that the IRQF_DISABLED warning is gone. Cheers, Richard -- __ , | ) /| Richard Atterer | \/ | http://atterer.org -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
On Thu, Sep 30, 2010 at 6:00 PM, Antti Palosaari wrote: > On 09/30/2010 09:52 PM, Michael Krufky wrote: >> >> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >> wrote: >>> >>> By default, tda18271 tries to optimize I2C bus by updating all registers >>> at the same time. Unfortunately, some devices doesn't support it. >>> >>> The current logic has a problem when small_i2c is equal to 8, since there >>> are some transfers using 11 + 1 bytes. >>> >>> Fix the problem by enforcing the max size at the right place, and allows >>> reducing it to max = 3 + 1. >>> >>> Signed-off-by: Mauro Carvalho Chehab >> >> This looks to me as if it is working around a problem on the i2c >> master. I believe that a fix like this really belongs in the i2c >> master driver, it should be able to break the i2c transactions down >> into transactions that the i2c master can handle. >> >> I wouldn't want to merge this without a better explanation of why it >> is necessary in the tda18271 driver. It seems to be a band-aid to >> cover up a problem in the i2c master device driver code. > > Yes it is I2C provider limitation, but I think almost all I2C adapters have > some limit. I suggest to set param for each tuner and demod driver which > splits reads and writes to len adapter can handle. I did that for tda18218 > write. > > But there is one major point you don't see. It is not simple to add this > splitting limit to the provider. Provider does not have knowledge which is > meaning of bytes it transfers to the bus. Without knowledge it breaks > functionality surely in some point. There is commonly seen 1, 2 and 4 byte > register address and same for register values. Also some chips like to send > data as register-value pairs. Yes, I understand. We will likely merge Mauro's patch, I just want to test it on my own hardware first, and I'd like to verify the cx231xx i2c xfer limit issue that Mauro is reporting. I'll try to get back to this early next week, or hopefully over this weekend. Thanks for the input :-) Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
On 09/30/2010 09:52 PM, Michael Krufky wrote: On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab wrote: By default, tda18271 tries to optimize I2C bus by updating all registers at the same time. Unfortunately, some devices doesn't support it. The current logic has a problem when small_i2c is equal to 8, since there are some transfers using 11 + 1 bytes. Fix the problem by enforcing the max size at the right place, and allows reducing it to max = 3 + 1. Signed-off-by: Mauro Carvalho Chehab This looks to me as if it is working around a problem on the i2c master. I believe that a fix like this really belongs in the i2c master driver, it should be able to break the i2c transactions down into transactions that the i2c master can handle. I wouldn't want to merge this without a better explanation of why it is necessary in the tda18271 driver. It seems to be a band-aid to cover up a problem in the i2c master device driver code. Yes it is I2C provider limitation, but I think almost all I2C adapters have some limit. I suggest to set param for each tuner and demod driver which splits reads and writes to len adapter can handle. I did that for tda18218 write. But there is one major point you don't see. It is not simple to add this splitting limit to the provider. Provider does not have knowledge which is meaning of bytes it transfers to the bus. Without knowledge it breaks functionality surely in some point. There is commonly seen 1, 2 and 4 byte register address and same for register values. Also some chips like to send data as register-value pairs. regards, Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
Antti, All, On Thursday 30 September 2010 23:48:57 Antti Palosaari wrote: > On 10/01/2010 12:09 AM, Yann E. MORIN wrote: > > I'm using the latest tree from: > >git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git > No, it is too old. Correct tree is staging/v2.6.37 at: > http://git.linuxtv.org/media_tree.git OK! > > I would make use of the entries left. The af9015_properties is an array > > with currently 3 entries. Each entries currently all have 9 device > > description. Do you prefer that I add the new description: > > - in the first entry, > > - just below the existing A850, (my pick) > > - or in the last entry? > Add it to the first free slot find. It was TerraTec Cinergy T Dual RC I > added lastly. If there is free space put it just behind that, otherwise > to the first free slot in next entry. This entry/dev count really sucks > a little bit, it should be fixed if possible... but as now we left it. OK! > Hmm, now I like it when it is identified as AverMedia AVerTV Red HD+. [--SNIP--] > If you can make patch against latest 2.6.37 pointed I it will be OK. Yes, I will. > Also possible remote could be nice... 2.6.37 af9015 have totally > different remote implementation. I am not using the remote for now. As soon I have it working, I'll either submit a patch, or acknowledge existing stuff works. Thank you! Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
Moi Yann On 10/01/2010 12:09 AM, Yann E. MORIN wrote: Antti, All, On Thursday 30 September 2010 22:42:40 Antti Palosaari wrote: On 09/30/2010 08:56 PM, Yann E. MORIN wrote: OK. The number of supported devices is already 9 in all sections, so I guess I'll have to add a new entry in the af9015_properties array, before I can add a new device, right? Actually you are using too old code as base. You should take latest GIT media tree and 2.6.37 branch. I'm using the latest tree from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git Is that OK? No, it is too old. Correct tree is staging/v2.6.37 at: http://git.linuxtv.org/media_tree.git IIRC max is currently 12 devices per entry. Yes, the array in the struct has 12 entries, but the comments in the af9015 code said: "/* max 9 */". So I stuck to the comment. That`s since count is increased after comment. I have changed it already. I would make use of the entries left. The af9015_properties is an array with currently 3 entries. Each entries currently all have 9 device description. Do you prefer that I add the new description: - in the first entry, - just below the existing A850, (my pick) - or in the last entry? Add it to the first free slot find. It was TerraTec Cinergy T Dual RC I added lastly. If there is free space put it just behind that, otherwise to the first free slot in next entry. This entry/dev count really sucks a little bit, it should be fixed if possible... but as now we left it. And to answer your previous question: Are you sure it does also have such bad eeprom content? Is that really needed? What it happens without this hack? Yes, I just tried without the hack and it breaks. With the hack, it works. I can provide the failing dmesg output if needed (see working one below). OK, then hack is needed. And what is the intrinsic difference between adding a new device section, compared to adding a new PID to an existing device (just curious) ? Not much more than a little bit different device name. Technically you can add all IDs to one device, but I feel better to add new entry per device. If device name is same but only ID is different it typically means different hw revision and in that case I would like to put those same for same entry. In that case device is also a little bit different - at least case colour. OK, got it. I'm afraid the A850T is just a A850 re-branded for the french market. Here is the relevant dmesg output when I plug the stick (with my changes applied on a 2.6.35.6): [12547.002398] usb 3-3.1: new high speed USB device using ehci_hcd and address 9 [12547.090226] usb 3-3.1: New USB device found, idVendor=07ca, idProduct=850b [12547.090228] usb 3-3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [12547.090230] usb 3-3.1: Product: A850 DVBT [12547.090231] usb 3-3.1: Manufacturer: AVerMedia [12547.090232] usb 3-3.1: SerialNumber: 302970601989000 [12547.093558] input: AVerMedia A850 DVBT as /class/input/input14 [12547.093603] generic-usb 0003:07CA:850B.000A: input,hidraw6: USB HID v1.01 Keyboard [AVerMedia A850 DVBT] on usb-:07:02.2-3.1/input1 [12547.488128] dvb-usb: found a 'AverMedia AVerTV Red HD+' in cold state, will try to load a firmware [12547.492200] dvb-usb: downloading firmware from file 'dvb-usb-af9015.fw' [12547.563986] dvb-usb: found a 'AverMedia AVerTV Red HD+' in warm state. [12547.564032] dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer. [12547.564372] DVB: registering new adapter (AverMedia AVerTV Red HD+) [12547.572230] af9013: firmware version:5.1.0 [12547.576731] DVB: registering adapter 0 frontend 0 (Afatech AF9013 DVB-T)... [12547.581615] MXL5005S: Attached at address 0xc6 [12547.581653] input: IR-receiver inside an USB DVB receiver as /class/input/input15 [12547.581656] dvb-usb: schedule remote query interval to 150 msecs. [12547.581658] dvb-usb: AverMedia AVerTV Red HD+ successfully initialized and connected. [12547.678851] usbcore: registered new interface driver dvb_usb_af9015 See the part that reads: input: AVerMedia A850 DVBT as /class/input/input14 ^^^ This is no kernel message, and (I guess) it comes as the ID string from the device. It also appears on a machine where I have no DVB support. Yes, it comes from eeprom, also lsusb should show it (lsusb -vvd usb-id) So I believe the patch is OK in the state, unless you really want a new device description, instead of adding to the existing A850 ( yes, granted, it's not the same color ;-] ). What is your final word? ;-) Hmm, now I like it when it is identified as AverMedia AVerTV Red HD+. Anyway, before you get action and push this patch, Eric helped in the testing so far. Maybe he'll want to add his tested-by? Thank you very much for your comments and guidance! Regards, Yann E. MORIN. If you can make patch against latest 2.6.37 pointed I it will be OK. Also possible remote could be nice... 2.6.37 af9015 have tot
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
Antti, All, On Thursday 30 September 2010 22:42:40 Antti Palosaari wrote: > On 09/30/2010 08:56 PM, Yann E. MORIN wrote: > > OK. The number of supported devices is already 9 in all sections, so I guess > > I'll have to add a new entry in the af9015_properties array, before I can > > add a new device, right? > Actually you are using too old code as base. You should take latest GIT > media tree and 2.6.37 branch. I'm using the latest tree from: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git Is that OK? > IIRC max is currently 12 devices per entry. Yes, the array in the struct has 12 entries, but the comments in the af9015 code said: "/* max 9 */". So I stuck to the comment. I would make use of the entries left. The af9015_properties is an array with currently 3 entries. Each entries currently all have 9 device description. Do you prefer that I add the new description: - in the first entry, - just below the existing A850, (my pick) - or in the last entry? And to answer your previous question: > Are you sure it does also have such bad eeprom content? Is that really > needed? What it happens without this hack? Yes, I just tried without the hack and it breaks. With the hack, it works. I can provide the failing dmesg output if needed (see working one below). > > And what is the intrinsic difference between adding a new device section, > > compared to adding a new PID to an existing device (just curious) ? > Not much more than a little bit different device name. Technically you > can add all IDs to one device, but I feel better to add new entry per > device. If device name is same but only ID is different it typically > means different hw revision and in that case I would like to put those > same for same entry. In that case device is also a little bit different > - at least case colour. OK, got it. I'm afraid the A850T is just a A850 re-branded for the french market. Here is the relevant dmesg output when I plug the stick (with my changes applied on a 2.6.35.6): [12547.002398] usb 3-3.1: new high speed USB device using ehci_hcd and address 9 [12547.090226] usb 3-3.1: New USB device found, idVendor=07ca, idProduct=850b [12547.090228] usb 3-3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [12547.090230] usb 3-3.1: Product: A850 DVBT [12547.090231] usb 3-3.1: Manufacturer: AVerMedia [12547.090232] usb 3-3.1: SerialNumber: 302970601989000 [12547.093558] input: AVerMedia A850 DVBT as /class/input/input14 [12547.093603] generic-usb 0003:07CA:850B.000A: input,hidraw6: USB HID v1.01 Keyboard [AVerMedia A850 DVBT] on usb-:07:02.2-3.1/input1 [12547.488128] dvb-usb: found a 'AverMedia AVerTV Red HD+' in cold state, will try to load a firmware [12547.492200] dvb-usb: downloading firmware from file 'dvb-usb-af9015.fw' [12547.563986] dvb-usb: found a 'AverMedia AVerTV Red HD+' in warm state. [12547.564032] dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer. [12547.564372] DVB: registering new adapter (AverMedia AVerTV Red HD+) [12547.572230] af9013: firmware version:5.1.0 [12547.576731] DVB: registering adapter 0 frontend 0 (Afatech AF9013 DVB-T)... [12547.581615] MXL5005S: Attached at address 0xc6 [12547.581653] input: IR-receiver inside an USB DVB receiver as /class/input/input15 [12547.581656] dvb-usb: schedule remote query interval to 150 msecs. [12547.581658] dvb-usb: AverMedia AVerTV Red HD+ successfully initialized and connected. [12547.678851] usbcore: registered new interface driver dvb_usb_af9015 See the part that reads: input: AVerMedia A850 DVBT as /class/input/input14 ^^^ This is no kernel message, and (I guess) it comes as the ID string from the device. It also appears on a machine where I have no DVB support. So I believe the patch is OK in the state, unless you really want a new device description, instead of adding to the existing A850 ( yes, granted, it's not the same color ;-] ). What is your final word? ;-) Anyway, before you get action and push this patch, Eric helped in the testing so far. Maybe he'll want to add his tested-by? Thank you very much for your comments and guidance! Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
On Thu, Sep 30, 2010 at 4:26 PM, Mauro Carvalho Chehab wrote: > Em 30-09-2010 16:27, Michael Krufky escreveu: >> Mauro, >> >> I think that's a reasonable explanation. Would you be open to >> reworking the patch such that the register contents only show up if >> the device is not recognized? (when ret < 0) . In the case where the >> device is correctly identified (ret == 0), I'd rather preserve the >> original successful detection message, and not see the ID register >> contents. > > Patch enclosed. > > --- > > [PATCH] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned > > Instead of doing: > > [ 82.581639] tda18271 4-0060: creating new instance > [ 82.588411] Unknown device detected @ 4-0060, device not supported. > [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 > [ 82.600530] tda18271 4-0060: destroying instance > > Print: > [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. > > for the error message, to help detecting what's going wrong with the > device. > > This helps to detect when the driver is using the wrong I2C bus (or have > the i2g gate switch pointing to the wrong place), on devices like cx231xx > that just return 0 on reads to a non-existent i2c device. > > Signed-off-by: Mauro Carvalho Chehab > > diff --git a/drivers/media/common/tuners/tda18271-fe.c > b/drivers/media/common/tuners/tda18271-fe.c > index 7955e49..3db8727 100644 > --- a/drivers/media/common/tuners/tda18271-fe.c > +++ b/drivers/media/common/tuners/tda18271-fe.c > @@ -1156,7 +1156,6 @@ static int tda18271_get_id(struct dvb_frontend *fe) > struct tda18271_priv *priv = fe->tuner_priv; > unsigned char *regs = priv->tda18271_regs; > char *name; > - int ret = 0; > > mutex_lock(&priv->lock); > tda18271_read_regs(fe); > @@ -1172,17 +1171,18 @@ static int tda18271_get_id(struct dvb_frontend *fe) > priv->id = TDA18271HDC2; > break; > default: > - name = "Unknown device"; > - ret = -EINVAL; > - break; > + tda_info("Unknown device (%i) detected @ %d-%04x, device not > supported.\n", > + regs[R_ID], > + i2c_adapter_id(priv->i2c_props.adap), > + priv->i2c_props.addr); > + return -EINVAL; > } > > - tda_info("%s detected @ %d-%04x%s\n", name, > + tda_info("%s detected @ %d-%04x\n", name, > i2c_adapter_id(priv->i2c_props.adap), > - priv->i2c_props.addr, > - (0 == ret) ? "" : ", device not supported."); > + priv->i2c_props.addr); > > - return ret; > + return 0; > } > > static int tda18271_setup_configuration(struct dvb_frontend *fe, > > This looks good to me, although you could concatenate these lines together: > + tda_info("Unknown device (%i) detected @ %d-%04x, device not > supported.\n", > + regs[R_ID], > + i2c_adapter_id(priv->i2c_props.adap), > + priv->i2c_props.addr); > + return -EINVAL; to be more like this: > + tda_info("Unknown device (%i) detected @ %d-%04x, device not > supported.\n", > + regs[R_ID], i2c_adapter_id(priv->i2c_props.adap), > priv->i2c_props.addr); > + return -EINVAL; ...that is, if it fits within an 80-char line. Anyway, Reviewed-by: Michael Krufky Thank you. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
On 09/30/2010 08:56 PM, Yann E. MORIN wrote: OK. The number of supported devices is already 9 in all sections, so I guess I'll have to add a new entry in the af9015_properties array, before I can add a new device, right? Actually you are using too old code as base. You should take latest GIT media tree and 2.6.37 branch. IIRC max is currently 12 devices per entry. And what is the intrinsic difference between adding a new device section, compared to adding a new PID to an existing device (just curious) ? Not much more than a little bit different device name. Technically you can add all IDs to one device, but I feel better to add new entry per device. If device name is same but only ID is different it typically means different hw revision and in that case I would like to put those same for same entry. In that case device is also a little bit different - at least case colour. Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
Em 30-09-2010 16:27, Michael Krufky escreveu: > Mauro, > > I think that's a reasonable explanation. Would you be open to > reworking the patch such that the register contents only show up if > the device is not recognized? (when ret < 0) . In the case where the > device is correctly identified (ret == 0), I'd rather preserve the > original successful detection message, and not see the ID register > contents. Patch enclosed. --- [PATCH] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned Instead of doing: [ 82.581639] tda18271 4-0060: creating new instance [ 82.588411] Unknown device detected @ 4-0060, device not supported. [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 [ 82.600530] tda18271 4-0060: destroying instance Print: [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. for the error message, to help detecting what's going wrong with the device. This helps to detect when the driver is using the wrong I2C bus (or have the i2g gate switch pointing to the wrong place), on devices like cx231xx that just return 0 on reads to a non-existent i2c device. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c index 7955e49..3db8727 100644 --- a/drivers/media/common/tuners/tda18271-fe.c +++ b/drivers/media/common/tuners/tda18271-fe.c @@ -1156,7 +1156,6 @@ static int tda18271_get_id(struct dvb_frontend *fe) struct tda18271_priv *priv = fe->tuner_priv; unsigned char *regs = priv->tda18271_regs; char *name; - int ret = 0; mutex_lock(&priv->lock); tda18271_read_regs(fe); @@ -1172,17 +1171,18 @@ static int tda18271_get_id(struct dvb_frontend *fe) priv->id = TDA18271HDC2; break; default: - name = "Unknown device"; - ret = -EINVAL; - break; + tda_info("Unknown device (%i) detected @ %d-%04x, device not supported.\n", +regs[R_ID], +i2c_adapter_id(priv->i2c_props.adap), +priv->i2c_props.addr); + return -EINVAL; } - tda_info("%s detected @ %d-%04x%s\n", name, + tda_info("%s detected @ %d-%04x\n", name, i2c_adapter_id(priv->i2c_props.adap), -priv->i2c_props.addr, -(0 == ret) ? "" : ", device not supported."); +priv->i2c_props.addr); - return ret; + return 0; } static int tda18271_setup_configuration(struct dvb_frontend *fe, -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lirc: Make struct file_operations pointer const
struct file_operations was made const in the drivers, but not in struct lirc_driver: drivers/staging/lirc/lirc_it87.c:365: warning: initialization discards qualifiers from pointer target type drivers/staging/lirc/lirc_parallel.c:571: warning: initialization discards qualifiers from pointer target type drivers/staging/lirc/lirc_serial.c:1073: warning: initialization discards qualifiers from pointer target type drivers/staging/lirc/lirc_sir.c:482: warning: initialization discards qualifiers from pointer target type drivers/staging/lirc/lirc_zilog.c:1284: warning: assignment discards qualifiers from pointer target type Signed-off-by: Geert Uytterhoeven --- include/media/lirc_dev.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h index b1f6066..71a896e 100644 --- a/include/media/lirc_dev.h +++ b/include/media/lirc_dev.h @@ -139,7 +139,7 @@ struct lirc_driver { struct lirc_buffer *rbuf; int (*set_use_inc) (void *data); void (*set_use_dec) (void *data); - struct file_operations *fops; + const struct file_operations *fops; struct device *dev; struct module *owner; }; -- 1.7.0.4 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
Em 30-09-2010 16:27, Michael Krufky escreveu: > On Thu, Sep 30, 2010 at 3:16 PM, Mauro Carvalho Chehab > wrote: >> Em 30-09-2010 15:57, Michael Krufky escreveu: >>> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >>> wrote: Instead of doing: [ 82.581639] tda18271 4-0060: creating new instance [ 82.588411] Unknown device detected @ 4-0060, device not supported. [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 [ 82.600530] tda18271 4-0060: destroying instance Print: [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. for the error message, to help detecting what's going wrong with the device. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/common/tuners/tda18271-fe.c b/drivers/media/common/tuners/tda18271-fe.c index 7955e49..77e3642 100644 --- a/drivers/media/common/tuners/tda18271-fe.c +++ b/drivers/media/common/tuners/tda18271-fe.c @@ -1177,7 +1177,7 @@ static int tda18271_get_id(struct dvb_frontend *fe) break; } - tda_info("%s detected @ %d-%04x%s\n", name, + tda_info("%s (%i) detected @ %d-%04x%s\n", name, regs[R_ID] & 0x7f, i2c_adapter_id(priv->i2c_props.adap), priv->i2c_props.addr, (0 == ret) ? "" : ", device not supported."); >>> >>> A patch like this is fine for testing, but I see no reason for merging >>> this into the kernel. Can you provide an explaination as per why this >>> would be useful? In general, if you see, "Unknown device detected @ >>> X-00YY, device not supported." then it means that this is not a >>> tda182x1. >> >> cx231xx have 4 I2C buses. The device I'm working with have the tuner at the >> wrong chip. >> As it doesn't support 0 byte transactions, if you try to read from the wrong >> i2c, it will >> just return 0 to all read requests. >> >> So, this kind of message can be very useful if someone sends us a report >> about a new device. >> The changes are small and are printed only in the case of errors, where >> people will likely >> try to reach the developers. So, I think it is a good idea to have it >> mainstream. > > Mauro, > > I think that's a reasonable explanation. Would you be open to > reworking the patch such that the register contents only show up if > the device is not recognized? (when ret < 0) . In the case where the > device is correctly identified (ret == 0), I'd rather preserve the > original successful detection message, and not see the ID register > contents. Ok, sure. I'll do it. Thanks, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
Em 30-09-2010 16:18, Michael Krufky escreveu: > On Thu, Sep 30, 2010 at 3:12 PM, Mauro Carvalho Chehab > wrote: >> Hi Michael, >> >> Em 30-09-2010 15:52, Michael Krufky escreveu: >>> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >>> wrote: By default, tda18271 tries to optimize I2C bus by updating all registers at the same time. Unfortunately, some devices doesn't support it. The current logic has a problem when small_i2c is equal to 8, since there are some transfers using 11 + 1 bytes. Fix the problem by enforcing the max size at the right place, and allows reducing it to max = 3 + 1. Signed-off-by: Mauro Carvalho Chehab >>> >>> This looks to me as if it is working around a problem on the i2c >>> master. I believe that a fix like this really belongs in the i2c >>> master driver, it should be able to break the i2c transactions down >>> into transactions that the i2c master can handle. >>> >>> I wouldn't want to merge this without a better explanation of why it >>> is necessary in the tda18271 driver. It seems to be a band-aid to >>> cover up a problem in the i2c master device driver code. >> >> In the specific case of cx231xx the hardware don't support any I2C >> transactions >> with more than 4 bytes. It is common to find this kind of limit on other >> types >> of hardware as well. >> >> In the specific case of tda18271, the workaround for tda18271 is already >> there: >> >> enum tda18271_small_i2c { >>TDA18271_39_BYTE_CHUNK_INIT = 0, >>TDA18271_16_BYTE_CHUNK_INIT = 1, >>TDA18271_08_BYTE_CHUNK_INIT = 2, >> }; >> (from upstream tda18271.h header) >> >> Yet, it is currently broken. In thesis, if you use small_i2c, it should >> limit the >> transactions size to a certain value, but, if you try to limit to 8, you'll >> still >> see some transactions with size=11, since the code that honors small_i2c >> only works >> for the initial dump of the 39 registers, as there are some cases where >> writes to >> EP3 registers are done with: >>tda18271_write_regs(fe, R_EP3, 11); >> >> and the current code for tda18271_write_regs don't check it. >> >> So, if there's a code there that allows limiting small_i2c: >>1) this code should work for all transactions, not only to the >> initial init; >>2) if there is such code, why not allow specifying a max size of 4 >> bytes? >> >> Another aspect is that doing such kind or restriction at the i2c adapter >> would require >> a very ugly logic, as some devices like tda18271 have only 8 bits registers >> per i2c address, >> and a write (or read) with length > 1 byte update/read the next consecutive >> registers, >> while other devices like xc3028 have one single I2C address for multi-byte >> operations like >> updating the firmware. >> >> If this logic would be moved into the bridge drivers, they would need to >> have some ugly >> tests, checking for each specific i2c sub-driver at their core drivers. > > Hmm... If you don't mind, would you allow me to think about this a > bit, and run some tests of my own? > > I have already seen the tda18271 work properly on the cx231xx with 8 > byte transactions, the issue that I saw was actually a 12-byte > transaction limit, so the 11 byte transfer didn't cause a problem. > I'd like to test this again myself using the cx231xx device that I > have on hand. Maybe the cx231xx have different limits per I2C bus. The device I'm currently handling uses I2C channel 2 (instead of channel 1) for the tuner. The only device on this bus is tda18271. The original driver uses just one byte for every transactions. On my tests, _all_ I2C transactions to i2c channel #2 with wLength > 4 bytes fail. So, I'm pretty confident that it won't support more than 4 bytes of payload. > However, this transaction in particular: > > tda18271_write_regs(fe, R_EP3, 11); > > ...is not supposed to be broken down to smaller transaction -- this > particular transaction is supposed to be a one shot change to all 11 > regs at once. Straying from this could result in unpredictable > behavior. Well, if the device doesn't support large transactions, what other options do we have? > I'd just like to review some other options and retest this before we > merge. Is that okay with you? Yeah, sure. Cheers, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
On Thu, Sep 30, 2010 at 3:16 PM, Mauro Carvalho Chehab wrote: > Em 30-09-2010 15:57, Michael Krufky escreveu: >> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >> wrote: >>> Instead of doing: >>> >>> [ 82.581639] tda18271 4-0060: creating new instance >>> [ 82.588411] Unknown device detected @ 4-0060, device not supported. >>> [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 >>> [ 82.600530] tda18271 4-0060: destroying instance >>> >>> Print: >>> [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. >>> >>> for the error message, to help detecting what's going wrong with the >>> device. >>> >>> Signed-off-by: Mauro Carvalho Chehab >>> >>> diff --git a/drivers/media/common/tuners/tda18271-fe.c >>> b/drivers/media/common/tuners/tda18271-fe.c >>> index 7955e49..77e3642 100644 >>> --- a/drivers/media/common/tuners/tda18271-fe.c >>> +++ b/drivers/media/common/tuners/tda18271-fe.c >>> @@ -1177,7 +1177,7 @@ static int tda18271_get_id(struct dvb_frontend *fe) >>> break; >>> } >>> >>> - tda_info("%s detected @ %d-%04x%s\n", name, >>> + tda_info("%s (%i) detected @ %d-%04x%s\n", name, regs[R_ID] & 0x7f, >>> i2c_adapter_id(priv->i2c_props.adap), >>> priv->i2c_props.addr, >>> (0 == ret) ? "" : ", device not supported."); >> >> A patch like this is fine for testing, but I see no reason for merging >> this into the kernel. Can you provide an explaination as per why this >> would be useful? In general, if you see, "Unknown device detected @ >> X-00YY, device not supported." then it means that this is not a >> tda182x1. > > cx231xx have 4 I2C buses. The device I'm working with have the tuner at the > wrong chip. > As it doesn't support 0 byte transactions, if you try to read from the wrong > i2c, it will > just return 0 to all read requests. > > So, this kind of message can be very useful if someone sends us a report > about a new device. > The changes are small and are printed only in the case of errors, where > people will likely > try to reach the developers. So, I think it is a good idea to have it > mainstream. Mauro, I think that's a reasonable explanation. Would you be open to reworking the patch such that the register contents only show up if the device is not recognized? (when ret < 0) . In the case where the device is correctly identified (ret == 0), I'd rather preserve the original successful detection message, and not see the ID register contents. Thanks & regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
On Thu, Sep 30, 2010 at 3:12 PM, Mauro Carvalho Chehab wrote: > Hi Michael, > > Em 30-09-2010 15:52, Michael Krufky escreveu: >> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab >> wrote: >>> By default, tda18271 tries to optimize I2C bus by updating all registers >>> at the same time. Unfortunately, some devices doesn't support it. >>> >>> The current logic has a problem when small_i2c is equal to 8, since there >>> are some transfers using 11 + 1 bytes. >>> >>> Fix the problem by enforcing the max size at the right place, and allows >>> reducing it to max = 3 + 1. >>> >>> Signed-off-by: Mauro Carvalho Chehab >> >> This looks to me as if it is working around a problem on the i2c >> master. I believe that a fix like this really belongs in the i2c >> master driver, it should be able to break the i2c transactions down >> into transactions that the i2c master can handle. >> >> I wouldn't want to merge this without a better explanation of why it >> is necessary in the tda18271 driver. It seems to be a band-aid to >> cover up a problem in the i2c master device driver code. > > In the specific case of cx231xx the hardware don't support any I2C > transactions > with more than 4 bytes. It is common to find this kind of limit on other types > of hardware as well. > > In the specific case of tda18271, the workaround for tda18271 is already > there: > > enum tda18271_small_i2c { > TDA18271_39_BYTE_CHUNK_INIT = 0, > TDA18271_16_BYTE_CHUNK_INIT = 1, > TDA18271_08_BYTE_CHUNK_INIT = 2, > }; > (from upstream tda18271.h header) > > Yet, it is currently broken. In thesis, if you use small_i2c, it should limit > the > transactions size to a certain value, but, if you try to limit to 8, you'll > still > see some transactions with size=11, since the code that honors small_i2c only > works > for the initial dump of the 39 registers, as there are some cases where > writes to > EP3 registers are done with: > tda18271_write_regs(fe, R_EP3, 11); > > and the current code for tda18271_write_regs don't check it. > > So, if there's a code there that allows limiting small_i2c: > 1) this code should work for all transactions, not only to the initial > init; > 2) if there is such code, why not allow specifying a max size of 4 > bytes? > > Another aspect is that doing such kind or restriction at the i2c adapter > would require > a very ugly logic, as some devices like tda18271 have only 8 bits registers > per i2c address, > and a write (or read) with length > 1 byte update/read the next consecutive > registers, > while other devices like xc3028 have one single I2C address for multi-byte > operations like > updating the firmware. > > If this logic would be moved into the bridge drivers, they would need to have > some ugly > tests, checking for each specific i2c sub-driver at their core drivers. Hmm... If you don't mind, would you allow me to think about this a bit, and run some tests of my own? I have already seen the tda18271 work properly on the cx231xx with 8 byte transactions, the issue that I saw was actually a 12-byte transaction limit, so the 11 byte transfer didn't cause a problem. I'd like to test this again myself using the cx231xx device that I have on hand. However, this transaction in particular: tda18271_write_regs(fe, R_EP3, 11); ...is not supposed to be broken down to smaller transaction -- this particular transaction is supposed to be a one shot change to all 11 regs at once. Straying from this could result in unpredictable behavior. I'd just like to review some other options and retest this before we merge. Is that okay with you? Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] V4L/DVB: tda18271: Add debug message with frequency divisor
Em 30-09-2010 16:03, Michael Krufky escreveu: > On Tue, Sep 28, 2010 at 2:47 PM, Mauro Carvalho Chehab > wrote: >> Signed-off-by: Mauro Carvalho Chehab >> >> diff --git a/drivers/media/common/tuners/tda18271-common.c >> b/drivers/media/common/tuners/tda18271-common.c >> index 195b30e..7ba3ba3 100644 >> --- a/drivers/media/common/tuners/tda18271-common.c >> +++ b/drivers/media/common/tuners/tda18271-common.c >> @@ -549,6 +549,13 @@ int tda18271_calc_main_pll(struct dvb_frontend *fe, u32 >> freq) >>regs[R_MD1] = 0x7f & (div >> 16); >>regs[R_MD2] = 0xff & (div >> 8); >>regs[R_MD3] = 0xff & div; >> + >> + if (tda18271_debug & DBG_REG) { >> + tda_reg("MAIN_DIV_BYTE_1= 0x%02x\n", 0xff & regs[R_MD1]); >> + tda_reg("MAIN_DIV_BYTE_2= 0x%02x\n", 0xff & regs[R_MD2]); >> + tda_reg("MAIN_DIV_BYTE_3= 0x%02x\n", 0xff & regs[R_MD3]); >> + } >> + >> fail: >>return ret; >> } > > > I would actually prefer NOT to merge this - it is redundant. When > DBG_REG is enabled, the driver will dump the contents of all > registers, including MD1, MD2 and MD3. With this patch applied, it > would dump this data twice. I do not believe this is useful at all. Ok. > > Regards, > > Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
Em 30-09-2010 15:57, Michael Krufky escreveu: > On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab > wrote: >> Instead of doing: >> >> [ 82.581639] tda18271 4-0060: creating new instance >> [ 82.588411] Unknown device detected @ 4-0060, device not supported. >> [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 >> [ 82.600530] tda18271 4-0060: destroying instance >> >> Print: >> [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. >> >> for the error message, to help detecting what's going wrong with the >> device. >> >> Signed-off-by: Mauro Carvalho Chehab >> >> diff --git a/drivers/media/common/tuners/tda18271-fe.c >> b/drivers/media/common/tuners/tda18271-fe.c >> index 7955e49..77e3642 100644 >> --- a/drivers/media/common/tuners/tda18271-fe.c >> +++ b/drivers/media/common/tuners/tda18271-fe.c >> @@ -1177,7 +1177,7 @@ static int tda18271_get_id(struct dvb_frontend *fe) >>break; >>} >> >> - tda_info("%s detected @ %d-%04x%s\n", name, >> + tda_info("%s (%i) detected @ %d-%04x%s\n", name, regs[R_ID] & 0x7f, >> i2c_adapter_id(priv->i2c_props.adap), >> priv->i2c_props.addr, >> (0 == ret) ? "" : ", device not supported."); > > A patch like this is fine for testing, but I see no reason for merging > this into the kernel. Can you provide an explaination as per why this > would be useful? In general, if you see, "Unknown device detected @ > X-00YY, device not supported." then it means that this is not a > tda182x1. cx231xx have 4 I2C buses. The device I'm working with have the tuner at the wrong chip. As it doesn't support 0 byte transactions, if you try to read from the wrong i2c, it will just return 0 to all read requests. So, this kind of message can be very useful if someone sends us a report about a new device. The changes are small and are printed only in the case of errors, where people will likely try to reach the developers. So, I think it is a good idea to have it mainstream. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
Hi Michael, Em 30-09-2010 15:52, Michael Krufky escreveu: > On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab > wrote: >> By default, tda18271 tries to optimize I2C bus by updating all registers >> at the same time. Unfortunately, some devices doesn't support it. >> >> The current logic has a problem when small_i2c is equal to 8, since there >> are some transfers using 11 + 1 bytes. >> >> Fix the problem by enforcing the max size at the right place, and allows >> reducing it to max = 3 + 1. >> >> Signed-off-by: Mauro Carvalho Chehab > > This looks to me as if it is working around a problem on the i2c > master. I believe that a fix like this really belongs in the i2c > master driver, it should be able to break the i2c transactions down > into transactions that the i2c master can handle. > > I wouldn't want to merge this without a better explanation of why it > is necessary in the tda18271 driver. It seems to be a band-aid to > cover up a problem in the i2c master device driver code. In the specific case of cx231xx the hardware don't support any I2C transactions with more than 4 bytes. It is common to find this kind of limit on other types of hardware as well. In the specific case of tda18271, the workaround for tda18271 is already there: enum tda18271_small_i2c { TDA18271_39_BYTE_CHUNK_INIT = 0, TDA18271_16_BYTE_CHUNK_INIT = 1, TDA18271_08_BYTE_CHUNK_INIT = 2, }; (from upstream tda18271.h header) Yet, it is currently broken. In thesis, if you use small_i2c, it should limit the transactions size to a certain value, but, if you try to limit to 8, you'll still see some transactions with size=11, since the code that honors small_i2c only works for the initial dump of the 39 registers, as there are some cases where writes to EP3 registers are done with: tda18271_write_regs(fe, R_EP3, 11); and the current code for tda18271_write_regs don't check it. So, if there's a code there that allows limiting small_i2c: 1) this code should work for all transactions, not only to the initial init; 2) if there is such code, why not allow specifying a max size of 4 bytes? Another aspect is that doing such kind or restriction at the i2c adapter would require a very ugly logic, as some devices like tda18271 have only 8 bits registers per i2c address, and a write (or read) with length > 1 byte update/read the next consecutive registers, while other devices like xc3028 have one single I2C address for multi-byte operations like updating the firmware. If this logic would be moved into the bridge drivers, they would need to have some ugly tests, checking for each specific i2c sub-driver at their core drivers. Cheers, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[cron job] v4l-dvb daily build 2.6.26 and up: ERRORS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Thu Sep 30 19:00:05 CEST 2010 path:http://www.linuxtv.org/hg/v4l-dvb changeset: 15164:1da5fed5c8b2 git master: 3e6dce76d99b328716b43929b9195adfee1de00c git media-master: e847bbbf9273533c15c6e8aab204ba62c238cf42 gcc version: i686-linux-gcc (GCC) 4.4.3 host hardware:x86_64 host os: 2.6.32.5 linux-2.6.32.6-armv5: WARNINGS linux-2.6.33-armv5: OK linux-2.6.34-armv5: WARNINGS linux-2.6.35.3-armv5: WARNINGS linux-2.6.36-rc2-armv5: ERRORS linux-2.6.32.6-armv5-davinci: WARNINGS linux-2.6.33-armv5-davinci: WARNINGS linux-2.6.34-armv5-davinci: WARNINGS linux-2.6.35.3-armv5-davinci: WARNINGS linux-2.6.36-rc2-armv5-davinci: ERRORS linux-2.6.32.6-armv5-ixp: WARNINGS linux-2.6.33-armv5-ixp: WARNINGS linux-2.6.34-armv5-ixp: WARNINGS linux-2.6.35.3-armv5-ixp: WARNINGS linux-2.6.36-rc2-armv5-ixp: ERRORS linux-2.6.32.6-armv5-omap2: WARNINGS linux-2.6.33-armv5-omap2: WARNINGS linux-2.6.34-armv5-omap2: WARNINGS linux-2.6.35.3-armv5-omap2: WARNINGS linux-2.6.36-rc2-armv5-omap2: ERRORS linux-2.6.26.8-i686: WARNINGS linux-2.6.27.44-i686: WARNINGS linux-2.6.28.10-i686: WARNINGS linux-2.6.29.1-i686: WARNINGS linux-2.6.30.10-i686: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-rc2-i686: ERRORS linux-2.6.32.6-m32r: WARNINGS linux-2.6.33-m32r: OK linux-2.6.34-m32r: WARNINGS linux-2.6.35.3-m32r: WARNINGS linux-2.6.36-rc2-m32r: ERRORS linux-2.6.32.6-mips: WARNINGS linux-2.6.33-mips: WARNINGS linux-2.6.34-mips: WARNINGS linux-2.6.35.3-mips: WARNINGS linux-2.6.36-rc2-mips: ERRORS linux-2.6.32.6-powerpc64: WARNINGS linux-2.6.33-powerpc64: WARNINGS linux-2.6.34-powerpc64: WARNINGS linux-2.6.35.3-powerpc64: WARNINGS linux-2.6.36-rc2-powerpc64: ERRORS linux-2.6.26.8-x86_64: WARNINGS linux-2.6.27.44-x86_64: WARNINGS linux-2.6.28.10-x86_64: WARNINGS linux-2.6.29.1-x86_64: WARNINGS linux-2.6.30.10-x86_64: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-rc2-x86_64: ERRORS linux-git-Module.symvers: ERRORS linux-git-armv5: ERRORS linux-git-armv5-davinci: ERRORS linux-git-armv5-ixp: ERRORS linux-git-armv5-omap2: ERRORS linux-git-i686: ERRORS linux-git-m32r: ERRORS linux-git-mips: ERRORS linux-git-powerpc64: ERRORS linux-git-x86_64: ERRORS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] V4L/DVB: tda18271: Add debug message with frequency divisor
On Tue, Sep 28, 2010 at 2:47 PM, Mauro Carvalho Chehab wrote: > Signed-off-by: Mauro Carvalho Chehab > > diff --git a/drivers/media/common/tuners/tda18271-common.c > b/drivers/media/common/tuners/tda18271-common.c > index 195b30e..7ba3ba3 100644 > --- a/drivers/media/common/tuners/tda18271-common.c > +++ b/drivers/media/common/tuners/tda18271-common.c > @@ -549,6 +549,13 @@ int tda18271_calc_main_pll(struct dvb_frontend *fe, u32 > freq) > regs[R_MD1] = 0x7f & (div >> 16); > regs[R_MD2] = 0xff & (div >> 8); > regs[R_MD3] = 0xff & div; > + > + if (tda18271_debug & DBG_REG) { > + tda_reg("MAIN_DIV_BYTE_1 = 0x%02x\n", 0xff & regs[R_MD1]); > + tda_reg("MAIN_DIV_BYTE_2 = 0x%02x\n", 0xff & regs[R_MD2]); > + tda_reg("MAIN_DIV_BYTE_3 = 0x%02x\n", 0xff & regs[R_MD3]); > + } > + > fail: > return ret; > } I would actually prefer NOT to merge this - it is redundant. When DBG_REG is enabled, the driver will dump the contents of all registers, including MD1, MD2 and MD3. With this patch applied, it would dump this data twice. I do not believe this is useful at all. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] V4L/DVB: tda18271: Add some hint about what tda18217 reg ID returned
On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab wrote: > Instead of doing: > > [ 82.581639] tda18271 4-0060: creating new instance > [ 82.588411] Unknown device detected @ 4-0060, device not supported. > [ 82.594695] tda18271_attach: [4-0060|M] error -22 on line 1272 > [ 82.600530] tda18271 4-0060: destroying instance > > Print: > [ 468.740392] Unknown device (0) detected @ 4-0060, device not supported. > > for the error message, to help detecting what's going wrong with the > device. > > Signed-off-by: Mauro Carvalho Chehab > > diff --git a/drivers/media/common/tuners/tda18271-fe.c > b/drivers/media/common/tuners/tda18271-fe.c > index 7955e49..77e3642 100644 > --- a/drivers/media/common/tuners/tda18271-fe.c > +++ b/drivers/media/common/tuners/tda18271-fe.c > @@ -1177,7 +1177,7 @@ static int tda18271_get_id(struct dvb_frontend *fe) > break; > } > > - tda_info("%s detected @ %d-%04x%s\n", name, > + tda_info("%s (%i) detected @ %d-%04x%s\n", name, regs[R_ID] & 0x7f, > i2c_adapter_id(priv->i2c_props.adap), > priv->i2c_props.addr, > (0 == ret) ? "" : ", device not supported."); A patch like this is fine for testing, but I see no reason for merging this into the kernel. Can you provide an explaination as per why this would be useful? In general, if you see, "Unknown device detected @ X-00YY, device not supported." then it means that this is not a tda182x1. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes
On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab wrote: > By default, tda18271 tries to optimize I2C bus by updating all registers > at the same time. Unfortunately, some devices doesn't support it. > > The current logic has a problem when small_i2c is equal to 8, since there > are some transfers using 11 + 1 bytes. > > Fix the problem by enforcing the max size at the right place, and allows > reducing it to max = 3 + 1. > > Signed-off-by: Mauro Carvalho Chehab This looks to me as if it is working around a problem on the i2c master. I believe that a fix like this really belongs in the i2c master driver, it should be able to break the i2c transactions down into transactions that the i2c master can handle. I wouldn't want to merge this without a better explanation of why it is necessary in the tda18271 driver. It seems to be a band-aid to cover up a problem in the i2c master device driver code. Regards, Mike Krufky -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
Antti, All, On Thursday 30 September 2010 11:40:12 Antti Palosaari wrote: > > /* AverMedia AVerTV Volar Black HD (A850) device have bad EEPROM > > - content :-( Override some wrong values here. */ > > + content :-( Override some wrong values here. Ditto for the > > + AVerTV Red HD+ (A850T) device. */ > > if (le16_to_cpu(udev->descriptor.idVendor) == USB_VID_AVERMEDIA&& > > - le16_to_cpu(udev->descriptor.idProduct) == USB_PID_AVERMEDIA_A850) { > > + ((le16_to_cpu(udev->descriptor.idProduct) == > > USB_PID_AVERMEDIA_A850) || > > +(le16_to_cpu(udev->descriptor.idProduct) == > > USB_PID_AVERMEDIA_A850T))) { > > deb_info("%s: AverMedia A850: overriding config\n", __func__); > > /* disable dual mode */ > > af9015_config.dual_mode = 0; > Are you sure it does also have such bad eeprom content? Is that really > needed? What it happens without this hack? Well, not really sure. The fact is that it works with the kludge, and that the device is very similar to the A850. On the Ubuntu forums, they simply suggested replacing the A850 Product ID 0x850a with the A850T PID 0x850b to make it work. And indeed it does work flawlessly so far. Disabling dual mode when it is working, is better that enabling it when it is not working. I tried to be conservative in this case. I'll be doing some testing without the kludge later tonight, or at worst during the WE. Unfortunately, the current v4l git tree BUGs in the USB sub-system on my machine (even without my changes). > > .name = "AverMedia AVerTV Volar Black HD " \ > > - "(A850)", > > - .cold_ids = {&af9015_usb_table[20], NULL}, > > + "(A850) / AVerTV Volar Red HD+ (A850T)", > > + .cold_ids = {&af9015_usb_table[20], > > + &af9015_usb_table[33], NULL}, > Add new entry for that device (and leave A850 as untouched). OK. The number of supported devices is already 9 in all sections, so I guess I'll have to add a new entry in the af9015_properties array, before I can add a new device, right? And what is the intrinsic difference between adding a new device section, compared to adding a new PID to an existing device (just curious) ? Thanks for the review. :-) Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ASUS My Cinema-P7131 Hybrid (saa7134) and slow IR
On 29/09/2010 23:03, Mauro Carvalho Chehab wrote: > Em 29-09-2010 14:28, Mauro Carvalho Chehab escreveu: >> Em 29-09-2010 14:06, Giorgio escreveu: >>> Hello, >>> >>> I have an Asus P7131 Hybrid card, and it works like a charm with >>> Ubuntu 8.04 and stock kernel 2.6.24. But, after upgrading my system to >>> Ubuntu 10.04 x86-64, I noticed that the remote control was quite slow >>> to respond. Sometimes the keypresses aren't recognized, and you have >>> to keep pressing the same button two or three times until it works. >>> The remote feels slow, not very responsive. >>> So, to investigate the issue, I loaded the ir-common module with >>> debug=1 and looked at the logs. They report lots of "ir-common: >>> spurious timer_end". The funny thing is, I have tried the Ubuntu 10.04 >>> i386 livecd (with the same kernel) and the problem is not present >>> there. >> >>> Sep 27 15:48:59 holden-desktop kernel: [ 256.770031] ir-common: spurious >>> timer_end >>> Sep 27 15:48:59 holden-desktop kernel: [ 256.880030] ir-common: spurious >>> timer_end >> >> It is using the old RC support. This support will be removed soon, so, the >> better is to convert it to use the new IR core, and fix a bug there, if is >> there any. Understood. >> Please apply the attached patch (it is against my -git tree, but it will >> probably >> apply fine if you have a new kernel). Applied, and indeed I was able to collect all the scancodes I needed :) >> You should notice that the RC_MAP_ASUS_PC39 table is not ready for the new IR >> infrastructure. So, you'll need to enable ir-core debug, and check what >> scancodes are >> detected there. Probably, all we need is to add the RC5 address to all codes >> at the table. Done. > Giorgio, > > Based on the pastebin you posted via IRC, this is likely the patch you > need to also change your current keytable to work with the new RC core. Thanks to your help on IRC, I fixed the keytable, new patch below. Everything should work now, here's the log of my test: hol...@holden-desktop:~$ sudo modprobe -v saa7134 insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/tveeprom.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-core.ko debug=1 insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-common.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/videobuf-core.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/videobuf-dma-sg.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/v4l2-compat-ioctl32.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/v4l1-compat.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/videodev.ko insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/v4l2-common.ko install /sbin/modprobe --ignore-install saa7134 && { /sbin/modprobe --quiet --use-blacklist saa7134-alsa ; : ; } insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/saa7134/saa7134.ko ir_debug=1 insmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/video/saa7134/saa7134-alsa.ko index=-2 hol...@holden-desktop:~$ sudo modprobe -vr ir_sony_decoder ir_jvc_decoder ir_rc6_decoder ir_nec_decoder rmmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-sony-decoder.ko rmmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-jvc-decoder.ko rmmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-rc6-decoder.ko rmmod /lib/modules/2.6.35.6-dvb1/kernel/drivers/media/IR/ir-nec-decoder.ko Sep 30 12:27:57 holden-desktop kernel: [ 1119.452985] IR NEC protocol handler initialized Sep 30 12:27:57 holden-desktop kernel: [ 1119.475713] IR RC5(x) protocol handler initialized Sep 30 12:27:57 holden-desktop kernel: [ 1119.479107] IR RC6 protocol handler initialized Sep 30 12:27:57 holden-desktop kernel: [ 1119.479344] Linux video capture interface: v2.00 Sep 30 12:27:57 holden-desktop kernel: [ 1119.489334] IR JVC protocol handler initialized Sep 30 12:27:57 holden-desktop kernel: [ 1119.496464] IR Sony protocol handler initialized Sep 30 12:27:57 holden-desktop kernel: [ 1119.521130] saa7130/34: v4l2 driver version 0.2.16 loaded Sep 30 12:27:57 holden-desktop kernel: [ 1119.521227] saa7133[0]: found at :02:07.0, rev: 209, irq: 20, latency: 64, mmio: 0xfbfff800 Sep 30 12:27:57 holden-desktop kernel: [ 1119.521234] saa7133[0]: subsystem: 1043:4876, board: ASUSTeK P7131 Hybrid [card=112,autodetected] Sep 30 12:27:57 holden-desktop kernel: [ 1119.521275] saa7133[0]: board init: gpio is 4 Sep 30 12:27:57 holden-desktop kernel: [ 1119.550016] Registered IR keymap rc-asus-pc39 Sep 30 12:27:57 holden-desktop kernel: [ 1119.550026] __ir_input_register: Allocated space for 64 keycode entries (512 bytes) Sep 30 12:27:57 holden-desktop kernel: [ 1119.550030] ir_do_setkeycode: #0: New scan 0x082a with key 0x000b Sep 30 12:27:57 holden-desktop kernel: [ 1119.550032] ir_do_setkeycode: #0: New scan 0x0816 with key 0x0002 Sep 30 12:27:57 holden-desktop kernel: [ 1119.550035] ir_do_setkeycod
Re: [GIT PATCHES FOR 2.6.37] fix long-standing tm6000 compile warning
Em 30-09-2010 11:32, Hans Verkuil escreveu: > The following changes since commit e847bbbf9273533c15c6e8aab204ba62c238cf42: > Hans Verkuil (1): > V4L/DVB: v4l2-common: Move v4l2_find_nearest_format from > videodev2.h to v4l2-common.h > > are available in the git repository at: > > ssh://linuxtv.org/git/hverkuil/v4l-dvb.git fixes > > Hans Verkuil (1): > tm6000-core.c: fix compile warning That warning is there for a purpose: it shouldn't be required to re-initialize the frequency every time analog mode is selected, especially since it takes some time to set a frequency on tm6000, as the chipset has a broken i2c implementation and requires some milisseconds after each url sent, otherwise the device becomes unresponsive. What happens with tm5600/tm6000 devices (not sure if this affects tm6010) is that, if the channel has a weak signal, the chip goes to some sleep state, where it stops receiving the stream. Unfortunately, even after signal return, the stream doesn't return. So, we need to call the function that changes the channel frequency just as a way to wake up the device. This is an ugly hack, and eventually there are some other ways of doing that that would be faster than what this routine does, but we never discovered. My hope on keeping this warning is that one day during the driver development, we would discover the root cause and provide a better fix for it (or when moving it from staging to drivers/media). I don't object to remove the warning though, but the better would be to move the frequency declaration to the beginning of the function, avoiding to have the block under { }. > > drivers/staging/tm6000/tm6000-core.c | 13 - > 1 files changed, 8 insertions(+), 5 deletions(-) > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.37] fix long-standing tm6000 compile warning
The following changes since commit e847bbbf9273533c15c6e8aab204ba62c238cf42: Hans Verkuil (1): V4L/DVB: v4l2-common: Move v4l2_find_nearest_format from videodev2.h to v4l2-common.h are available in the git repository at: ssh://linuxtv.org/git/hverkuil/v4l-dvb.git fixes Hans Verkuil (1): tm6000-core.c: fix compile warning drivers/staging/tm6000/tm6000-core.c | 13 - 1 files changed, 8 insertions(+), 5 deletions(-) -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-dvb/v2.6.37] V4L/DVB: V4L2: add a generic function to find the nearest discrete format to the required one
On Thursday 30 September 2010 14:29:10 Hans Verkuil wrote: > On Thursday, September 30, 2010 13:50:00 Mauro Carvalho Chehab wrote: > > This is an automatic generated email to let you know that the following > > patch were queued at the http://git.linuxtv.org/media-tree.git tree: > > > > Subject: V4L/DVB: V4L2: add a generic function to find the nearest > > discrete format to the required one Author: Guennadi Liakhovetski > > > > Date:Fri Aug 27 13:41:44 2010 -0300 > > > > Many video drivers implement a fixed set of frame formats and thus face a > > task of finding the best match for a user-requested format. Implementing > > this in a generic function has also an advantage, that different drivers > > with similar supported format sets will select the same format for the > > user, which improves consistency across drivers. > > > > Signed-off-by: Guennadi Liakhovetski > > Signed-off-by: Mauro Carvalho Chehab > > > > drivers/media/video/v4l2-common.c | 24 > > include/linux/videodev2.h |8 > > 2 files changed, 32 insertions(+), 0 deletions(-) > > > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > index b06479f..957d5b0 100644 > > --- a/include/linux/videodev2.h > > +++ b/include/linux/videodev2.h > > @@ -397,6 +397,14 @@ struct v4l2_frmsize_discrete { > > > > __u32 height; /* Frame height [pixel] */ > > > > }; > > > > +struct v4l2_discrete_probe { > > + const struct v4l2_frmsize_discrete *sizes; > > + int num_sizes; > > +}; > > + > > +struct v4l2_frmsize_discrete *v4l2_find_nearest_format(struct > > v4l2_discrete_probe *probe, + > >s32 width, s32 height); > > + > > > > struct v4l2_frmsize_stepwise { > > > > __u32 min_width; /* Minimum frame width [pixel] > > */ > > __u32 max_width; /* Maximum frame width [pixel] > > */ > > ??? What is this doing in videodev2.h? This belongs in v4l2-common.h! > Both the return pointer and the probe pointer can be const as well. > > I'll make a patch for this since I've forgotten to adjust several > videobuf_queue_*_init functions as well in my bkl patch :-( And when did that get merged ? We haven't reached any agreement on this API change. It's pretty useless in its current form and will just contribute to the V4L2 kernel API bloat. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.37] Fix videobuf_queue*init and move v4l2_find_nearest_format to v4l2-common.h
Mauro, Feel free to kick my ass next time I present a patch with compile errors like that. I clearly did not do a careful grep on the sources, although I'd have sworn up and down that I did :-( Regards, Hans The following changes since commit 5e26d8407d390b48cc7a4cf1af7bbd4a679308ff: Guennadi Liakhovetski (1): V4L/DVB: soc-camera: allow only one video queue per device are available in the git repository at: ssh://linuxtv.org/git/hverkuil/v4l-dvb.git bkl2 Hans Verkuil (2): V4L/DVB: videobuf: add ext_lock argument to the queue init functions (part 2) v4l2-common: Move v4l2_find_nearest_format from videodev2.h to v4l2-common.h drivers/media/video/davinci/vpfe_capture.c |2 +- drivers/media/video/davinci/vpif_capture.c |3 ++- drivers/media/video/davinci/vpif_display.c |3 ++- drivers/media/video/fsl-viu.c |2 +- drivers/media/video/mx1_camera.c |2 +- drivers/media/video/mx2_camera.c |2 +- drivers/media/video/mx3_camera.c |3 ++- drivers/media/video/omap/omap_vout.c |2 +- drivers/media/video/omap24xxcam.c |2 +- drivers/media/video/pxa_camera.c |2 +- drivers/media/video/s5p-fimc/fimc-core.c |2 +- drivers/media/video/sh_mobile_ceu_camera.c |2 +- drivers/media/video/sh_vou.c |3 ++- drivers/media/video/v4l2-common.c |7 --- include/linux/videodev2.h |8 include/media/v4l2-common.h| 10 ++ 16 files changed, 31 insertions(+), 24 deletions(-) -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-dvb/v2.6.37] V4L/DVB: V4L2: add a generic function to find the nearest discrete format to the required one
On Thursday, September 30, 2010 13:50:00 Mauro Carvalho Chehab wrote: > This is an automatic generated email to let you know that the following patch > were queued at the > http://git.linuxtv.org/media-tree.git tree: > > Subject: V4L/DVB: V4L2: add a generic function to find the nearest discrete > format to the required one > Author: Guennadi Liakhovetski > Date:Fri Aug 27 13:41:44 2010 -0300 > > Many video drivers implement a fixed set of frame formats and thus face a task > of finding the best match for a user-requested format. Implementing this in a > generic function has also an advantage, that different drivers with similar > supported format sets will select the same format for the user, which improves > consistency across drivers. > > Signed-off-by: Guennadi Liakhovetski > Signed-off-by: Mauro Carvalho Chehab > > drivers/media/video/v4l2-common.c | 24 > include/linux/videodev2.h |8 > 2 files changed, 32 insertions(+), 0 deletions(-) > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > index b06479f..957d5b0 100644 > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -397,6 +397,14 @@ struct v4l2_frmsize_discrete { > __u32 height; /* Frame height [pixel] */ > }; > > +struct v4l2_discrete_probe { > + const struct v4l2_frmsize_discrete *sizes; > + int num_sizes; > +}; > + > +struct v4l2_frmsize_discrete *v4l2_find_nearest_format(struct > v4l2_discrete_probe *probe, > +s32 width, s32 height); > + > struct v4l2_frmsize_stepwise { > __u32 min_width; /* Minimum frame width [pixel] > */ > __u32 max_width; /* Maximum frame width [pixel] > */ ??? What is this doing in videodev2.h? This belongs in v4l2-common.h! Both the return pointer and the probe pointer can be const as well. I'll make a patch for this since I've forgotten to adjust several videobuf_queue_*_init functions as well in my bkl patch :-( Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCHES FOR 2.6.37] gspca for_2.6.37
Em 25-09-2010 06:18, Jean-Francois Moine escreveu: > The following changes since commit > dace3857de7a16b83ae7d4e13c94de8e4b267d2a: > > V4L/DVB: tvaudio: remove obsolete tda8425 initialization (2010-09-24 > 19:20:20 -0300) > > are available in the git repository at: > git://linuxtv.org/jfrancois/gspca.git for_2.6.37 > > Jean-François Moine (6): > gspca - benq: Display error messages when gspca debug disabled. > gspca - benq: Remove useless module load/unload messages. > gspca - cpia1: Fix compilation warning when gspca debug disabled. > gspca - many subdrivers: Handle INPUT as a module. Please, don't do that: +#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE) #include +#endif It would be a real mess if we start doing it for every include that have depencencies on kernel. Instead, just keep the include there. I applied the other patches from this series, keeping this one waiting for your version 2. > gspca - spca505: Remove the eeprom write commands of NxUltra. > gspca - sonixj: Propagate USB errors to higher level. > > drivers/media/video/gspca/benq.c| 20 ++ > drivers/media/video/gspca/cpia1.c |2 + > drivers/media/video/gspca/konica.c |8 ++- > drivers/media/video/gspca/ov519.c |6 +- > drivers/media/video/gspca/pac207.c |6 +- > drivers/media/video/gspca/pac7302.c |6 +- > drivers/media/video/gspca/pac7311.c |6 +- > drivers/media/video/gspca/sn9c20x.c |6 +- > drivers/media/video/gspca/sonixb.c |6 +- > drivers/media/video/gspca/sonixj.c | 91 > +-- > drivers/media/video/gspca/spca505.c |4 - > drivers/media/video/gspca/spca561.c |8 ++- > drivers/media/video/gspca/stv06xx/stv06xx.c |6 +- > drivers/media/video/gspca/zc3xx.c |6 +- > 14 files changed, 119 insertions(+), 62 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCHES FOR 2.6.37] Move V4L2 locking into the core framework
Em 26-09-2010 09:25, Hans Verkuil escreveu: > Hi Mauro, > > These are the locking patches. It's based on my previous test tree, but with > more testing with em28xx and radio-mr800 and some small tweaks relating to > disconnect handling and video_is_registered(). > > I also removed the unused get_unmapped_area file op and I am now blocking > any further (unlocked_)ioctl calls after the device node is unregistered. > The only things an application can do legally after a disconnect is unmap() > and close(). > > This patch series also contains a small em28xx fix that I found while testing > the em28xx BKL removal patch. > > Regards, > > Hans > > The following changes since commit dace3857de7a16b83ae7d4e13c94de8e4b267d2a: > Hans Verkuil (1): > V4L/DVB: tvaudio: remove obsolete tda8425 initialization > > are available in the git repository at: > > ssh://linuxtv.org/git/hverkuil/v4l-dvb.git bkl Applied, thanks. > Hans Verkuil (10): > v4l2-dev: after a disconnect any ioctl call will be blocked. > v4l2-dev: remove get_unmapped_area > v4l2: add core serialization lock. > videobuf: prepare to make locking optional in videobuf > videobuf: add ext_lock argument to the queue init functions > videobuf: add queue argument to videobuf_waiton() You forgot two to add the extra parameter on two drivers that uses vmalloc, noticed when I tried to compile it for i386 arch. I've already added them. Could you please double check if everything is compiling fine on the other archs? Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] SoC Camera: add driver for OMAP1 camera interface
This is a V4L2 driver for TI OMAP1 SoC camera interface. Both videobuf-dma versions are supported, contig and sg, selectable with a module option. The former uses less processing power, but often fails to allocate contignuous buffer memory. The latter is free of this problem, but generates tens of DMA interrupts per frame. If contig memory allocation ever fails, the driver falls back to sg automatically on next open, but still can be switched back to contig manually. Both paths work stable for me, even under heavy load, on my OMAP1510 based Amstrad Delta videophone, that is the oldest, least powerfull OMAP1 implementation. The interface generally works in pass-through mode. Since input data byte endianess can be swapped, it provides up to two v4l2 pixel formats per each of several soc_mbus formats that have their swapped endian counterparts. Boards using this driver can provide it with the following platform data: - if and what freqency clock is expected by an on-board camera sensor, - what is the maximum pixel clock that should be accepted from the sensor, - what is the polarity of the sensor provided pixel clock, - if the interface GPIO line is connected to a sensor reset/powerdown input and what is the input polarity. Created and tested against linux-2.6.36-rc5 on Amstrad Delta. Signed-off-by: Janusz Krzysztofik --- Guennadi, Since 2.6.36-rc6 is out, I've decided to submit the driver, no longer trying to make it still better or waiting for more comments, in hope it's still not too late for it to be merged during the upcomming window. The redefined set_fmt and set_crop operations work for me as expected with the last version (v3) of my ov6650 sensor driver. There are still two SG mode specific corner cases to be corrected, previously not detected because of poor sensor driver functionality: 1) frame size not exceeding one page, resulting in "unexpected end of frame" message and capture restart every frame, and 2) last sgbuf lenght less than bytes_per_line, resulting in unstable picture. I'm going to address those two with fixes. Thanks, Janusz v2->v3 changes: requested, suggested or inspired by Guennadi Liakhovetski (thanks again!): - compare enum variable against one of enum values instead of relaying on the fact that a possible value can be either 0 or not 0, - put an argument inside parenthesis in a macro definition, - CONTIG and SG are not good enough names to be defined in a header under include/..., - still better explain why suspend capture if pcdev->ready is NULL on buffer completion, - ephasize the fact and explain why not fetch new buffer from the queue immediately, rather do it only when a next DMA interrupt occures, - use more correct English wording in a few cases, - explain why not yet call videobuf_done() right after the end of current sglist is detected, but only on next DMA interrupt, - don't split strings, even on print format specifier boudries, - v2 updated subdev_call_with_sense() macro return value was no longer stored in a variable which was still examined next, - don't store 1 in a bool variable, use "true" instead, - allocate register cache dynamically based on platform resource size instead of sizing it staticaly with a predefined OMAP1_CAMERA_IOSIZE macro; move this macro out of the , suggested by Ralph Corderoy (thanks again!): - a few hex print formats still not optimal, other: - correct a few typos still found, - if a block consisting of a single command contains a comment as well, it should be enclosed in braces, - make the driver less noisy by lowering down levels of a fwe more messages, - is_dma_aligned() and dma_align() results can depend on vb_mode istead of always assuming CONTIG mode as a worth case, - redefine set_crop and set_fmt algorithms to better follow the v4l2 model, drop a few no longer used helper functions, create a new one, set_mbus_format(), from the new code common to both operations. v1->v2 changes: requested by Guennadi Liakhovetski (thanks!): - first try contig, and if it fails - fall back to sg; invalidates next two, - invalidated: Kconfig: VIDEO_OMAP1_SG: not need "if", the "depends on" should suffice, - invalidated: include both and headers, - extensively document buffer manipulations, better yet clean it up, - a copyright / licence header was missing form a header file, - need to #include if using BIT() macro, - don't need macros representing frequencies - use numbers directly, - add a few missing "static" qualifiers, - use u32 type for register content handling, - some cached registers were unnecessarily read from the hardware directly, - use true/false constants instead of 0/1 for booleans, - avoid assigning variables inside other constructs, - don't need to test if RAZ_FIFO is cleared, - no need to split "\n" to a new line, don't worry about > 80 characters, - don't increment field_count in case of a VIDEOBUF_ERROR, - adjust mbus format codes to the new names, - m
Re: [GIT PULL for 2.6.36] V4L/DVB fixes
On 09/27/2010 05:36 PM, Mauro Carvalho Chehab wrote: I'll clean up the mess and prepare a new pull request in the next days. Can you look at including "ir-core: Fix null dereferences in the protocols sysfs interface"? I never got a response to that, and it's a regression fix for 2.6.36. https://patchwork.kernel.org/patch/199002/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l/dvb: add support for AVerMedia AVerTV Red HD+ (A850T)
On 09/30/2010 12:18 AM, Yann E. MORIN wrote: The AVerTV Red HD+ (A850T) is basically the same as the existing AVerTV Volar Black HD 9A850), but is specific to the french market. This is a collection of information gathered from the french support forums for Ubuntu, which I tried to properly format: http://forum.ubuntu-fr.org/viewtopic.php?pid=3322825 /* AverMedia AVerTV Volar Black HD (A850) device have bad EEPROM - content :-( Override some wrong values here. */ + content :-( Override some wrong values here. Ditto for the + AVerTV Red HD+ (A850T) device. */ if (le16_to_cpu(udev->descriptor.idVendor) == USB_VID_AVERMEDIA&& - le16_to_cpu(udev->descriptor.idProduct) == USB_PID_AVERMEDIA_A850) { + ((le16_to_cpu(udev->descriptor.idProduct) == USB_PID_AVERMEDIA_A850) || +(le16_to_cpu(udev->descriptor.idProduct) == USB_PID_AVERMEDIA_A850T))) { deb_info("%s: AverMedia A850: overriding config\n", __func__); /* disable dual mode */ af9015_config.dual_mode = 0; Are you sure it does also have such bad eeprom content? Is that really needed? What it happens without this hack? .name = "AverMedia AVerTV Volar Black HD " \ - "(A850)", - .cold_ids = {&af9015_usb_table[20], NULL}, + "(A850) / AVerTV Volar Red HD+ (A850T)", + .cold_ids = {&af9015_usb_table[20], + &af9015_usb_table[33], NULL}, Add new entry for that device (and leave A850 as untouched). Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PATCHES FOR 2.6.37] Rename video_device et al to v4l2_devnode
Hi Mauro, Most of the v4l2 framework has a prefix that starts with v4l2_ except for struct video_device in v4l2-dev.c. This name is becoming very confusing since it closely resembles struct v4l2_device. Since video_device really represents a v4l2 device node I propose to rename it to v4l2_devnode and rename the v4l2-dev.[ch] to v4l2-devnode.[ch]. To make the transition easier I created a v4l2-dev.h that includes the new v4l2-devnode.h and #defines the old names to the new names. This header is removed once the full conversion is finished. I also updated the documentation to reflect the new header and naming convention. Now that the v2.6.36 cycle is nearing the end I think it is a good time to apply this patch so that it has the least impact. This patch requires that the bkl patch I posted earlier ("Move V4L2 locking into the core framework") is applied first. Regards, Hans The following changes since commit af9c9bdd595ec0a2077f1ebd298d8a3a1db01b57: Hans Verkuil (1): radio-mr800: remove BKL are available in the git repository at: ssh://linuxtv.org/git/hverkuil/v4l-dvb.git devnode Hans Verkuil (19): v4l2-devnode: renamed from v4l2-dev videodev2.h: update comment v4l2 core: use v4l2-devnode.h instead of v4l2-dev.h v4l2: rename to_video_device to v4l2_devnode_from_device v4l2: rename video_device_alloc to v4l2_devnode_alloc v4l2: rename video_device_release_empty to v4l2_devnode_release_empty v4l2: rename video_device_release to v4l2_devnode_release v4l2: rename video_device_node_name to v4l2_devnode_name v4l2: rename video_register_device to v4l2_devnode_register v4l2: rename video_unregister_device to v4l2_devnode_unregister v4l2: rename video_is_registered to v4l2_devnode_is_registered v4l2: rename video_get/set_drvdata to v4l2_devnode_get/set_drvdata v4l2: rename video_devdata to v4l2_devnode_from_file v4l2: rename video_drvdata to v4l2_drvdata_from_file v4l2: rename video_device to v4l2_devnode tea575x: convert to v4l2-devnode.h v4l2: include v4l2-devnode.h instead of v4l2-dev.h gadget/uvc.h: remove the temporary v4l2-dev.h include v4l2-dev.h: remove obsolete header Documentation/DocBook/v4l/videodev2.h.xml|2 +- Documentation/video4linux/v4l2-controls.txt | 10 +- Documentation/video4linux/v4l2-framework.txt | 91 ++-- drivers/media/common/saa7146_fops.c | 26 +- drivers/media/dvb/ngene/ngene.h |2 +- drivers/media/dvb/ttpci/av7110.h |4 +- drivers/media/dvb/ttpci/budget-av.c |2 +- drivers/media/radio/dsbr100.c| 30 +- drivers/media/radio/radio-aimslab.c | 20 +- drivers/media/radio/radio-aztech.c | 20 +- drivers/media/radio/radio-cadet.c| 30 +- drivers/media/radio/radio-gemtek-pci.c | 22 +- drivers/media/radio/radio-gemtek.c | 20 +- drivers/media/radio/radio-maestro.c | 24 +- drivers/media/radio/radio-maxiradio.c| 22 +- drivers/media/radio/radio-miropcm20.c| 18 +- drivers/media/radio/radio-mr800.c| 18 +- drivers/media/radio/radio-rtrack2.c | 20 +- drivers/media/radio/radio-sf16fmi.c | 20 +- drivers/media/radio/radio-sf16fmr2.c | 22 +- drivers/media/radio/radio-si4713.c | 22 +- drivers/media/radio/radio-tea5764.c | 36 +- drivers/media/radio/radio-terratec.c | 20 +- drivers/media/radio/radio-timb.c | 28 +- drivers/media/radio/radio-trust.c| 22 +- drivers/media/radio/radio-typhoon.c | 20 +- drivers/media/radio/radio-zoltrix.c | 20 +- drivers/media/radio/si470x/radio-si470x-common.c | 24 +- drivers/media/radio/si470x/radio-si470x-i2c.c| 14 +- drivers/media/radio/si470x/radio-si470x-usb.c| 18 +- drivers/media/radio/si470x/radio-si470x.h|4 +- drivers/media/video/Makefile |2 +- drivers/media/video/arv.c| 26 +- drivers/media/video/au0828/au0828-video.c| 32 +- drivers/media/video/au0828/au0828.h |4 +- drivers/media/video/bt8xx/bttv-driver.c | 62 +- drivers/media/video/bt8xx/bttvp.h|6 +- drivers/media/video/bw-qcam.c| 22 +- drivers/media/video/c-qcam.c | 24 +- drivers/media/video/cafe_ccic.c | 14 +- drivers/media/video/cpia.c | 38 +- drivers/media/video/cpia.h |2 +- drivers/media/video/cpia2/cpia2.h|2 +- drivers/media/video/cpia2/cpia2_v4l.c| 40 +- drivers/media/video/cx18/cx18-driver.h |2 +- drivers/media/video/cx18/cx18-fileops.c