Status of the patches under review (85 patches) and some misc notes about the devel procedures
Hi,
My original idea were to send one of such emails per week, in order to allow people to review my pending list and point me for missing things. Fortunately, the number of patches for the next merge window is so high that, even passing -hg maintainership to Douglas, I was still very busy. Good to see so much work at the subsystem. Congrats to the media developers that worked really hard!
With respect to -hg/-git pull requests, my queue is currently empty.
While I was working at the patchwork queue, we've received a lot of contributions on the last few days. So, I gave up of just applying everything before sending this email. I'll just list those patches that weren't yet read. I'll likely handle those during the weekend.
Also, as usual, there are probably some patches listed that were already applied, or that I missed the current status. In this case, just ping me.
It is expected that the current development kernel (2.6.33-rc6) to be the last one before the next merge window. Let's see. If this is the case, this means that we'll stop patch receiving for 2.6.35 very soon.
With respect to -git tree, I'll likely change the procedure a little bit for 2.6.35:
instead of merging everything at master, I intend to create some topics branches, merging patches there, and reserving "master" to be in sync with the latest stable + accepted fixes (or, some -rc + fixes). This will mean that it will have a cleaner history. I expect that the merge procedure from developers tree to be simpler, as each topic maintainer could decide either to keep using the latest stable, or to go to the very latest upstream tree.
With respect to the backport tree, Douglas is the maintainer for it. He's likely having a very bad time merging those high volume of patches, while trying to keep the tree compiling with the legacy kernel. Douglas, thanks for your work! As usual, if you find something broken there, instead of complain, just send him a patch fixing it ;)
Thanks,
Mauro.
---
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
P.S.: This email is c/c to the developers that some review action is expected.
== New or updated patches starting from May, 5 or newer - not handled yet ==
May, 5 2010: [-next] input: unlock on error paths http://patchwork.kernel.org/patch/96982 May, 5 2010: [-next] dvb/stv6110x: cleanup error handling http://patchwork.kernel.org/patch/96983 May, 5 2010: [-next] media/mem2mem: dereferencing free memory http://patchwork.kernel.org/patch/96984 May, 5 2010: media/ov511: cleanup: remove unneeded null check http://patchwork.kernel.org/patch/96985 May, 5 2010: [-next,1/2] media/s2255drv: return if vdev not found http://patchwork.kernel.org/patch/96987 May, 5 2010: [-next,2/2] media/s2255drv: remove dead code http://patchwork.kernel.org/patch/96986 May, 5 2010: tda10048: fix the uncomplete function tda10048_read_ber http://patchwork.kernel.org/patch/97058 May, 5 2010: media/IR: Add missing include file to rc-map.c http://patchwork.kernel.org/patch/97133 May, 5 2010: -next: staging/cx25821: please revert 7a02f549fcc http://patchwork.kernel.org/patch/97134 May, 5 2010: fix dvb frontend lockup http://patchwork.kernel.org/patch/97171 May, 5 2010: videobuf: remove external function videobuf_dma_sync() http://patchwork.kernel.org/patch/97175 May, 5 2010: -next: staging/cx25821: please revert 7a02f549fcc http://patchwork.kernel.org/patch/97177 May, 5 2010: videobuf-vmalloc: remove __videobuf_sync() http://patchwork.kernel.org/patch/97197 May, 5 2010: [1/5] viafb: fold via_io.h into via-core.h http://patchwork.kernel.org/patch/97224 May, 5 2010: [2/5] viafb: get rid of i2c debug cruft http://patchwork.kernel.org/patch/97230 May, 5 2010: [3/5] viafb: Eliminate some global.h references http://patchwork.kernel.org/patch/97233 May, 5 2010: [4/5] viafb: move some include files to include/linux http://patchwork.kernel.org/patch/97232 May, 5 2010: [5/5] Add the viafb video capture driver http://patchwork.kernel.org/patch/97231 Mar,17 2010: [2/2] V4L/DVB: buf-dma-sg.c: support non-pageable user-allocated memor http://patchwork.kernel.org/patch/97263 May, 6 2010: [1/1] staging: cx25821: cx25821-alsa.c: cleanup http://patchwork.kernel.org/patch/97295 May, 6 2010: tda10048: fix bitmask for the transmission mode http://patchwork.kernel.org/patch/97340 May, 6 2010: tda10048: clear the uncorrected packet registers when saturated http://patchwork.kernel.org/patch/97341 May, 6 2010: dvb_frontend: fix typos in comments and one function http://patchwork.kernel.org/patch/97343 May, 6 2010: [1/3] mx2_camera: Add soc_camera support for i.MX25/i.MX27 http://patchwork.kernel.org/patch/97345 May, 6 2010: [2/3] mx27: add support for the CSI device http://patchwork.kernel.org/patch/97352 May, 6 2010: [3/3] mx25: add support for the CSI device http://patchwork.kernel.org/patch/97353 May, 6 2010: tm6000: README - add vbi http://patchwork.kernel.org/patch/97354 May, 6 2010: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU http://patchwork.kernel.org/patch/97358 May, 6 2010: TechnoTrend TT-budget T-3000 http://patchwork.kernel.org/patch/97436 May, 4 2010: [v2] IR/imon: remove dead IMON_KEY_RELEASE_OFFSET http://patchwork.kernel.org/patch/96854 May, 6 2010: [-next] IR: add header file to fix build http://patchwork.kernel.org/patch/97502 May, 7 2010: stv6110x Fix kernel null pointer deref when plugging two TT s2-1600 http://patchwork.kernel.org/patch/97603 May, 7 2010: TT CT-3650 DVB-C support http://patchwork.kernel.org/patch/97606
May, 7 2010: [v2] stv6110x Fix kernel null pointer deref when plugging two TT s2-16 http://patchwork.kernel.org/patch/97612
== Port mantis IR to the new API - waiting for Manu Abraham <[email protected]> ack or alternative patch ==
Apr,15 2010: [5/8] ir-core: convert mantis from ir-functions.c http://patchwork.kernel.org/patch/92961
== dsbr100 patchess waiting for more review at the ML ==
May, 5 2010: dsbr100: implement proper locking http://patchwork.kernel.org/patch/97155 May, 5 2010: dsbr100: fix potential use after free http://patchwork.kernel.org/patch/97154 May, 5 2010: dsbr100: only change frequency upon success http://patchwork.kernel.org/patch/97156 May, 5 2010: dsbr100: remove disconnected indicator http://patchwork.kernel.org/patch/97158 May, 5 2010: dsbr100: properly initialize the radio http://patchwork.kernel.org/patch/97153 May, 5 2010: dsbr100: cleanup usb probe routine http://patchwork.kernel.org/patch/97157 May, 5 2010: dsbr100: simplify access to radio device http://patchwork.kernel.org/patch/97159
== Moorestown patches waiting for more review at the ML ==
Mar,28 2010: This patch is first part of the intel moorestown isp driver and header http://patchwork.kernel.org/patch/88734
Mar,28 2010: This patch is second part of intel moorestown isp driver and c files c http://patchwork.kernel.org/patch/88775
Mar,28 2010: This patch is second part of intel moorestown isp driver and c files c http://patchwork.kernel.org/patch/88776
Mar,28 2010: This patch is the flash subdev driver for intel moorestown camera imag http://patchwork.kernel.org/patch/88735
Mar,28 2010: this patch is 2mp camera (ov2650) sensor subdev driver for intel moore http://patchwork.kernel.org/patch/88738
Mar,28 2010: this patch is the 1.3mp camera (ov9665) sensor subdev driver fro inter http://patchwork.kernel.org/patch/88736
Mar,28 2010: this patch is the 5mp camera (ov5630) sensor subdev driver for intel m http://patchwork.kernel.org/patch/88737
Mar,28 2010: this patch is the 5mp camera (ov5630) sensor lens subdev driver for in http://patchwork.kernel.org/patch/88739
Mar,28 2010: this patch is the 5mp camera (s5k4e1) sensor subdev driver for intel m http://patchwork.kernel.org/patch/88740
Mar,28 2010: this patch is the make/kconfig files changes to enable the camera driv http://patchwork.kernel.org/patch/88741
== Arm s5p patches waiting for more review at the ML ==
Apr,19 2010: [v1,1/3] ARM: S5P: Add FIMC driver platform helpers http://patchwork.kernel.org/patch/93460 Apr,19 2010: [v1,2/3] ARM: S5PC100: Add FIMC driver platform helpers http://patchwork.kernel.org/patch/93466 Apr,19 2010: [v1,3/3] ARM: S5P: Add Camera interface (video postprocessor) driver http://patchwork.kernel.org/patch/95064
== Waiting for more DVB acks ==
May, 2 2010: Add FE_CAN_TURBO_FEC (was: Add FE_CAN_PSK_8 to allow apps to identify http://patchwork.kernel.org/patch/96341
== Waiting for my fixes on Docbook ==
Feb,25 2010: DocBook/Makefile: Make it less verbose http://patchwork.kernel.org/patch/82076
Feb,25 2010: DocBook: Add rules to auto-generate some media docbooks http://patchwork.kernel.org/patch/82075
Feb,25 2010: DocBook/v4l/pixfmt.xml: Add missing formats for gspca cpia1 and sn9c20 http://patchwork.kernel.org/patch/82074
Feb,25 2010: v4l: document new Bayer and monochrome pixel formats http://patchwork.kernel.org/patch/82073
== Waiting for Antti Palosaari <[email protected]> review ==
Mar,21 2010: af9015 : more robust eeprom parsing http://patchwork.kernel.org/patch/87243
== Waiting for Tobias Lorenz <[email protected]> review ==
Feb, 3 2010: radio-si470x-common: -EINVAL overwritten in si470x_vidioc_s_tuner() http://patchwork.kernel.org/patch/76786
== Waiting for its addition at linux-firmware and driver test by David Wong <[email protected]> ==
Nov, 1 2009: lgs8gxx: remove firmware for lgs8g75 http://patchwork.kernel.org/patch/56822
== Andy Walls <[email protected]> and Aleksandr Piskunov <[email protected]> are discussing around the solution ==
Oct,11 2009: AVerTV MCE 116 Plus radio http://patchwork.kernel.org/patch/52981
== Waiting for Andy Walls <[email protected]> ==
Apr,10 2010: cx18: "missing audio" for analog recordings http://patchwork.kernel.org/patch/91879
== Patches waiting for Patrick Boettcher <[email protected]> review ==
Jan,15 2010: remove obsolete conditionalizing on DVB_DIBCOM_DEBUG http://patchwork.kernel.org/patch/73147
== Gspca patches - Waiting Hans de Goede <[email protected]> submission/review ==
Jan,29 2010: [gspca_jf,tree] gspca zc3xx: signal when unknown packet received http://patchwork.kernel.org/patch/75837
== Gspca patches - Waiting Jean-Francois Moine <[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on heuristics http://patchwork.kernel.org/patch/81612
May, 3 2010: gspca: Try a less bandwidth-intensive alt setting. http://patchwork.kernel.org/patch/96514
== soc_camera patches - Waiting Guennadi <[email protected]> submission/review ==
Feb, 2 2010: [2/3] soc-camera: mt9t112: modify delay time after initialize http://patchwork.kernel.org/patch/76213
Feb, 2 2010: [3/3] soc-camera: mt9t112: The flag which control camera-init is remov http://patchwork.kernel.org/patch/76214
Mar, 5 2010: [v2] V4L/DVB: mx1-camera: compile fix http://patchwork.kernel.org/patch/83742
Apr,20 2010: pxa_camera: move fifo reset direct before dma start http://patchwork.kernel.org/patch/93619
== Waiting for Steven Toth <[email protected]> comments on it ==
Feb, 6 2010: cx23885: Enable Message Signaled Interrupts(MSI). http://patchwork.kernel.org/patch/77492
== Waiting for Henrik Kurelid <[email protected]> comments about an userspace patch for MythTV ==
Mar, 1 2010: firedtv: add parameter to fake ca_system_ids in CA_INFO http://patchwork.kernel.org/patch/82912
== Videobuf patches - Need more tests before committing it - Volunteers? ==
Apr,21 2010: [v1, 1/2] v4l: videobuf: Add support for out-of-order buffer dequeuing http://patchwork.kernel.org/patch/93901
Apr,21 2010: [v1, 2/2] v4l: vivi: adapt to out-of-order buffer dequeuing in videobu http://patchwork.kernel.org/patch/93903
== Waiting for Mike Isely <[email protected]> review ==
Apr,25 2010: Problem with cx25840 and Terratec Grabster AV400 http://patchwork.kernel.org/patch/94960
Apr,27 2010: V4L: Events: Include slab.h explicitly http://patchwork.kernel.org/patch/95449
== Waiting for Igor M. Liplianin <[email protected]> comments/review ==
Mar,10 2010: DM1105: could not attach frontend 195d:1105 http://patchwork.kernel.org/patch/84549
== Need some fixes from Uwe Kleine-Koenig <[email protected]> ==
Mar,16 2010: mx1-camera: compile fix http://patchwork.kernel.org/patch/86106
== Patch(broken) Lock fixes. Patch is wrong but Siano has lock problems - Waiting for developers with time to fix it ==
Nov,12 2009: Locking in Siano driver (untested) http://patchwork.kernel.org/patch/59590
== Patch(broken) - waiting for Manu Abraham <[email protected]> submission ==
Feb,16 2010: Twinhan 1027 + IR Port support http://patchwork.kernel.org/patch/79753
== Patch(broken) - waiting for Herton Ronaldo Krzesinski <[email protected]> new submission ==
Apr, 5 2010: saa7134: add support for Avermedia M733A http://patchwork.kernel.org/patch/90692
Mar,19 2010: saa7134: add support for one more remote control for Avermedia M135A http://patchwork.kernel.org/patch/86989
Cheers,
Mauro
---
All patches sent to [email protected] are automatically stored at Patchwork. The driver maintainer then analyzes it it and apply or reply with comments. Others also may comment about it.
When the patch is is too trivial, or touches on a driver without a specific maintainer, I analyze and apply it directly. Otherwise, it will be taged as under review and the proper maintainer will be warned.
If you submitted or are waiting for the review of some patchset that weren't applied at the git://git.kernel.org/v4l-dvb.git nor it is at the list bellow, please check the status of the patch at the patchwork:
http://patchwork.kernel.org/project/linux-media/list/
To check for a patch that is not New or Under Review, you'll need to click at the "Filters" and pass some parameters to find the patch. Don't forget to change the "State" to "any".
Eventually, your patch were rejected for some reason, or someone asked for changes on it. If the patch is there, with a status different than New/Under Review, double check for the received comments at the mailing list. If it got Rejected, marked as RFC or as Changes Requested, please review it according with the requests and send it again. In the very unlikely case that the patch were wrongly tagged, please ping me.
If you discover any patch submitted via email that weren't caught by Patchwork, this means that the patch got mangled by your emailer. The more likely cause is that the emailer converted tabs into spaces or broke long lines.
If you're using Thunderbird, the solution is to install Asalted Patches extension, available at:
https://hg.mozilla.org/users/clarkbw_gnome.org/asalted-patches/
Other emailers will need you to disable the wrapping long lines feature.
Hi Mauro
Quoted 3 lines · Mauro Carvalho Chehab
May, 6 2010: [1/3] mx2_camera: Add soc_camera support for i.MX25/i.MX27 http://patchwork.kernel.org/patch/97345 May, 6 2010: [2/3] mx27: add support for the CSI device http://patchwork.kernel.org/patch/97352 May, 6 2010: [3/3] mx25: add support for the CSI device http://patchwork.kernel.org/patch/97353
I'll be reviewing these
Quoted 4 lines
== soc_camera patches - Waiting Guennadi <[email protected]> submission/review ==Feb, 2 2010: [2/3] soc-camera: mt9t112: modify delay time after initialize http://patchwork.kernel.org/patch/76213
Feb, 2 2010: [3/3] soc-camera: mt9t112: The flag which control camera-init is remov http://patchwork.kernel.org/patch/76214
These two are still on hold, I think, I'll have to ask the author if we can drop them.
Quoted 1 line
Mar, 5 2010: [v2] V4L/DVB: mx1-camera: compile fix http://patchwork.kernel.org/patch/83742
An updated version of this one is already in your fixes tree:
http://git.linuxtv.org/fixes.git?a=commit;h=f6c22d4cff27a4bbb76d899b58b79dd311b7603f
Quoted 1 line
Apr,20 2010:
pxa_camera: move fifo reset direct before dma start http://patchwork.kernel.org/patch/93619
Ditto for this one:
http://git.linuxtv.org/fixes.git?a=commit;h=80cef8eb49c9689664a31b8a21f83517042d9763
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Quoted 22 lines · Guennadi Liakhovetski
Hi Mauro
Quoted 3 lines · Mauro Carvalho Chehab
May, 6 2010: [1/3] mx2_camera: Add soc_camera support for i.MX25/i.MX27 http://patchwork.kernel.org/patch/97345 May, 6 2010: [2/3] mx27: add support for the CSI device http://patchwork.kernel.org/patch/97352 May, 6 2010: [3/3] mx25: add support for the CSI device http://patchwork.kernel.org/patch/97353I'll be reviewing these
Quoted 4 lines
== soc_camera patches - Waiting Guennadi <[email protected]> submission/review ==Feb, 2 2010: [2/3] soc-camera: mt9t112: modify delay time after initialize http://patchwork.kernel.org/patch/76213
Feb, 2 2010: [3/3] soc-camera: mt9t112: The flag which control camera-init is remov http://patchwork.kernel.org/patch/76214These two are still on hold, I think, I'll have to ask the author if we can drop them.
Quoted 1 line
Mar, 5 2010: [v2] V4L/DVB: mx1-camera: compile fix http://patchwork.kernel.org/patch/83742
An updated version of this one is already in your fixes tree:
Quoted 7 lines
http://git.linuxtv.org/fixes.git?a=commit;h=f6c22d4cff27a4bbb76d899b58b79dd311b7603f
Quoted 1 line
Apr,20 2010:
pxa_camera: move fifo reset direct before dma start http://patchwork.kernel.org/patch/93619Ditto for this one:
http://git.linuxtv.org/fixes.git?a=commit;h=80cef8eb49c9689664a31b8a21f83517042d9763
Thanks for the input. I've updated patchwork accordingly.
Signature · 2 lines
Cheers, Mauro
Quoted 7 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
Quoted 7 lines
P.S.: This email is c/c to the developers that some review action is expected.== New or updated patches starting from May, 5 or newer - not handled yet ==
May, 5 2010: [-next] dvb/stv6110x: cleanup error handling http://patchwork.kernel.org/patch/96983
Mauro Carvalho Chehab
AppliedQuoted 13 lines · Manu Abraham
Quoted 11 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
P.S.: This email is c/c to the developers that some review action is expected.
Quoted 7 lines
Quoted 3 lines
== New or updated patches starting from May, 5 or newer - not handled yet ==May, 5 2010: [-next] dvb/stv6110x: cleanup error handling http://patchwork.kernel.org/patch/96983
Reviewed-by Manu Abraham <[email protected]>Acked-by Manu Abraham <[email protected]>
Applied, thanks.
Signature · 2 lines
Cheers, Mauro
Quoted 11 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
P.S.: This email is c/c to the developers that some review action is expected.May, 7 2010: [v2] stv6110x Fix kernel null pointer deref when plugging two TT s2-16 http://patchwork.kernel.org/patch/97612
How is this patch going to fix a NULL ptr dereference when more than 1 card is plugged in ? The patch doesn't seem to do what the patch title implies. At least the patch title seems to be wrong. Maybe the patch is supposed to check for a possible NULL ptr dereference when put to sleep ?
Quoted 11 lines · Manu Abraham
Quoted 9 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
P.S.: This email is c/c to the developers that some review action is expected.
Quoted 8 lines
Quoted 1 line
May, 7 2010: [v2] stv6110x Fix kernel null pointer deref when plugging two TT s2-16 http://patchwork.kernel.org/patch/97612
How is this patch going to fix a NULL ptr dereference when more than 1 card is plugged in ? The patch doesn't seem to do what the patch title implies. At least the patch title seems to be wrong. Maybe the patch is supposed to check for a possible NULL ptr dereference when put to sleep ?
(c/c patch author, to be sure that he'll see your explanation request)
His original patch is at:
https://patchwork.kernel.org/patch/91929/
The original description with the bug were much better than version 2.
Quoted 1 line
From his OOPS log and description, I suspect that he's facing some
sort of race condition with the two cards.
This fix seems still valid (with an updated comment), as his dump proofed that there are some cases where fe->tuner_priv can be null, generating an OOPS, but it seems that his patch is combating the effect, and not the cause.
So, I am for adding his patch for now, and then work on a more complete approach for the two cards environment.
Signature · 2 lines
Cheers, Mauro
Hi Mauro,
Sorry for the delay, I was abroad.
Let me detail the issue a bit more.
In budget.c, the the frontend is attached this way :
budget->dvb_frontend = dvb_attach(stv090x_attach, &tt1600_stv090x_config, &budget->i2c_adap, STV090x_DEMODULATOR_0);
This means that the tt1600_stv090x_config structure will be common for all the cards.
Then the tuner is attached to the frontend :
ctl = dvb_attach(stv6110x_attach, budget->dvb_frontend, &tt1600_stv6110x_config, &budget->i2c_adap);
Once the tuner is attached, the ops are copied to the config :
tt1600_stv090x_config.tuner_sleep = ctl->tuner_sleep;
This results in the ops being set for subsequently attached cards while fe->tuner_priv is NULL.
This is why a check for tuner_priv being set is mandatory when calling tuner_sleep(). However as pointed out, it may not be the best fix.
Regards,
Guy
Quoted 41 lines · On Fri, 07 May 2010 22:26:13 -0300 Mauro Carvalho Chehab <[email protected]>…
Quoted 16 lines · Manu Abraham
Quoted 7 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
Quoted 5 lines
P.S.: This email is c/c to the developers that some review action is expected.May, 7 2010: [v2] stv6110x Fix kernel null pointer deref when plugging two TT s2-16 http://patchwork.kernel.org/patch/97612
Quoted 5 lines
How is this patch going to fix a NULL ptr dereference when more than 1 card is plugged in ? The patch doesn't seem to do what the patch title implies. At least the patch title seems to be wrong. Maybe the patch is supposed to check for a possible NULL ptr dereference when put to sleep ?
(c/c patch author, to be sure that he'll see your explanation request)
His original patch is at:
https://patchwork.kernel.org/patch/91929/The original description with the bug were much better than version 2.
From his OOPS log and description, I suspect that he's facing some sort of race condition with the two cards.
This fix seems still valid (with an updated comment), as his dump proofed that there are some cases where
fe->tuner_privcan be null, generating an OOPS, but it seems that his patch is combating the effect, and not the cause.So, I am for adding his patch for now, and then work on a more complete approach for the two cards environment.
Em Thu, 27 May 2010 16:05:48 +0200
Guy Martin <[email protected]> escreveu:
Quoted 20 lines
Hi Mauro,
Sorry for the delay, I was abroad.
Let me detail the issue a bit more.In
budget.c, the the frontend is attached this way :
budget->dvb_frontend=dvb_attach(stv090x_attach, &tt1600_stv090x_config, &budget->i2c_adap, STV090x_DEMODULATOR_0);This means that the
tt1600_stv090x_configstructure will be common for all the cards.Then the tuner is attached to the frontend :
ctl =
dvb_attach(stv6110x_attach, budget->dvb_frontend, &tt1600_stv6110x_config, &budget->i2c_adap);
Quoted 8 lines
Once the tuner is attached, the ops are copied to the config :
tt1600_stv090x_config.tuner_sleep =ctl->tuner_sleep;This results in the ops being set for subsequently attached cards while
fe->tuner_privis NULL.This is why a check for
tuner_privbeing set is mandatory when callingtuner_sleep(). However as pointed out, it may not be the best fix.
Ok. I'll commit it for now, as it is a simple patch and can be easily be applied at stable. The proper patch would be to avoid calling the function at the second board with tuner_priv = NULL.
Quoted 51 lines
Regards,
GuyQuoted 41 lines · On Fri, 07 May 2010 22:26:13 -0300 Mauro Carvalho Chehab <[email protected]>…
Quoted 16 lines · Manu Abraham
Quoted 7 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
Quoted 5 lines
P.S.: This email is c/c to the developers that some review action is expected.May, 7 2010: [v2] stv6110x Fix kernel null pointer deref when plugging two TT s2-16 http://patchwork.kernel.org/patch/97612
Quoted 5 lines
How is this patch going to fix a NULL ptr dereference when more than 1 card is plugged in ? The patch doesn't seem to do what the patch title implies. At least the patch title seems to be wrong. Maybe the patch is supposed to check for a possible NULL ptr dereference when put to sleep ?
(c/c patch author, to be sure that he'll see your explanation request)
His original patch is at:
https://patchwork.kernel.org/patch/91929/The original description with the bug were much better than version 2.
From his OOPS log and description, I suspect that he's facing some sort of race condition with the two cards.
This fix seems still valid (with an updated comment), as his dump proofed that there are some cases where
fe->tuner_privcan be null, generating an OOPS, but it seems that his patch is combating the effect, and not the cause.So, I am for adding his patch for now, and then work on a more complete approach for the two cards environment.
Signature · 2 lines
Cheers, Mauro
Quoted 4 lines · Mauro Carvalho Chehab
Hi,
Quoted 8 lines
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
== Port mantis IR to the new API - waiting for Manu Abraham <[email protected]> ack or alternative patch ==
Apr,15 2010: [5/8] ir-core: convert mantis from
ir-functions.c
This needs to wait for an alternate patch, which depends on linuxtv.org/hg/v4l-dvb proper build
Quoted 10 lines · Manu Abraham
Quoted 7 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
Quoted 7 lines
Quoted 3 lines
== Port mantis IR to the new API - waiting for Manu Abraham <[email protected]> ack or alternative patch ==Apr,15 2010: [5/8] ir-core: convert mantis from
ir-functions.cThis needs to wait for an alternate patch, which depends on linuxtv.org/hg/v4l-dvb proper build
Ok. I think Douglas already backported the IR changes. Not sure if his tree is now in sync with git.
Signature · 2 lines
Cheers, Mauro
Quoted 9 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
May, 2 2010: Add
FE_CAN_TURBO_FEC(was: AddFE_CAN_PSK_8to allow apps to identify http://patchwork.kernel.org/patch/96341
Quoted 10 lines · Manu Abraham
Quoted 8 lines · Mauro Carvalho Chehab
Hi,
This is the summary of the patches that are currently under review. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email).
Quoted 6 lines
Quoted 1 line
May, 2 2010: Add
FE_CAN_TURBO_FEC(was: AddFE_CAN_PSK_8to allow apps to identify http://patchwork.kernel.org/patch/96341Reviewed-by Manu Abraham <[email protected]>Acked-by Manu Abraham <[email protected]>
Could you please reply with your reviewed-by tag at the original thread at the ML? I intend to add for a while for more review, and, by replying at the thread, Patchwork will automatically add your tag to the patch.
Signature · 2 lines
Cheers, Mauro
Em Sex 07 Mai 2010, às 09:39:16, Mauro Carvalho Chehab escreveu:
Quoted 3 lines
== Patch(broken) - waiting for Herton Ronaldo Krzesinski <[email protected]> new submission ==Apr, 5 2010: saa7134: add support for Avermedia M733A http://patchwork.kernel.org/patch/90692
Hi, I submitted now a new fixed version of the patch to mailing list, under title "[PATCH v2] saa7134: add support for Avermedia M733A"
Quoted 1 line
Mar,19 2010: saa7134: add support for one more remote control for Avermedia M135A http://patchwork.kernel.org/patch/86989
This was the addition of RM-K6 remote control to M135A too, I think we can drop this, since adding this to the kernel is deprecated now in favour of assigning keymaps in userspace (keytable tool from v4l-utils), right?
--
[]'s
Herton
Quoted 7 lines · Herton Ronaldo Krzesinski
Em Sex 07 Mai 2010, às 09:39:16, Mauro Carvalho Chehab escreveu:
Quoted 3 lines
== Patch(broken) - waiting for Herton Ronaldo Krzesinski <[email protected]> new submission ==Apr, 5 2010: saa7134: add support for Avermedia M733A http://patchwork.kernel.org/patch/90692
Hi, I submitted now a new fixed version of the patch to mailing list, under title "[PATCH v2] saa7134: add support for Avermedia M733A"
OK, thanks!
Quoted 5 lines
Quoted 1 line
Mar,19 2010: saa7134: add support for one more remote control for Avermedia M135A http://patchwork.kernel.org/patch/86989
This was the addition of RM-K6 remote control to M135A too, I think we can drop this, since adding this to the kernel is deprecated now in favour of assigning keymaps in userspace (keytable tool from v4l-utils), right?
For now, I prefer to add the keytab there, since there are some scripts that syncs v4l-util keytables with the kernel ones. If you prefer, you may the put RM-K6 table together with the other M135A keytable. I intend to group the non-conflicting keytables soon, and it makes kense to group both the original and the RM-K6 into the same table, in order to provide an easier hot-plug support for this device.
Signature · 2 lines
Cheers, Mauro
Em Sáb 08 Mai 2010, às 19:41:32, Mauro Carvalho Chehab escreveu:
Quoted 17 lines
Quoted 7 lines · Herton Ronaldo Krzesinski
Em Sex 07 Mai 2010, às 09:39:16, Mauro Carvalho Chehab escreveu:
Quoted 3 lines
== Patch(broken) - waiting for Herton Ronaldo Krzesinski <[email protected]> new submission ==Apr, 5 2010: saa7134: add support for Avermedia M733A http://patchwork.kernel.org/patch/90692
Hi, I submitted now a new fixed version of the patch to mailing list, under title "[PATCH v2] saa7134: add support for Avermedia M733A"
OK, thanks!
Quoted 5 lines
Quoted 1 line
Mar,19 2010: saa7134: add support for one more remote control for Avermedia M135A http://patchwork.kernel.org/patch/86989
This was the addition of RM-K6 remote control to M135A too, I think we can drop this, since adding this to the kernel is deprecated now in favour of assigning keymaps in userspace (keytable tool from v4l-utils), right?
Quoted 5 lines
For now, I prefer to add the keytab there, since there are some scripts that syncs v4l-util keytables with the kernel ones. If you prefer, you may the put RM-K6 table together with the other M135A keytable. I intend to group the non-conflicting keytables soon, and it makes kense to group both the original and the RM-K6 into the same table, in order to provide an easier hot-plug support for this device.
Ok, I updated and tested a new patch now, and sent with title "[PATCH] saa7134: add RM-K6 remote control support for Avermedia M135A"
--
[]'s
Herton
Quoted 14 lines · Herton Ronaldo Krzesinski
Em Sáb 08 Mai 2010, às 19:41:32, Mauro Carvalho Chehab escreveu:
Quoted 13 lines
Quoted 6 lines · Herton Ronaldo Krzesinski
Em Sex 07 Mai 2010, às 09:39:16, Mauro Carvalho Chehab escreveu:
Quoted 3 lines
== Patch(broken) - waiting for Herton Ronaldo Krzesinski <[email protected]> new submission ==Apr, 5 2010: saa7134: add support for Avermedia M733A http://patchwork.kernel.org/patch/90692
Hi, I submitted now a new fixed version of the patch to mailing list, under title "[PATCH v2] saa7134: add support for Avermedia M733A"
OK, thanks!
Quoted 4 lines
Quoted 1 line
Mar,19 2010: saa7134: add support for one more remote control for Avermedia M135A http://patchwork.kernel.org/patch/86989
This was the addition of RM-K6 remote control to M135A too, I think we can drop this, since adding this to the kernel is deprecated now in favour of assigning keymaps in userspace (keytable tool from v4l-utils), right?
Quoted 8 lines
Quoted 5 lines
For now, I prefer to add the keytab there, since there are some scripts that syncs v4l-util keytables with the kernel ones. If you prefer, you may the put RM-K6 table together with the other M135A keytable. I intend to group the non-conflicting keytables soon, and it makes kense to group both the original and the RM-K6 into the same table, in order to provide an easier hot-plug support for this device.
Ok, I updated and tested a new patch now, and sent with title "[PATCH] saa7134: add RM-K6 remote control support for Avermedia M135A"
Thanks!
Signature · 2 lines
Cheers, Mauro
Quoted 7 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on
heuristics http://patchwork.kernel.org/patch/81612
May, 3 2010: gspca: Try a less bandwidth-intensive alt
setting. http://patchwork.kernel.org/patch/96514
Hello Mauro,
I don't think the patch about pac7302 should be applied.
The patch about the gspca main is in my last git pull request.
Cheers.
Signature · 2 lines
Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
Quoted 11 lines · Jean-Francois Moine
Quoted 7 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on
heuristics http://patchwork.kernel.org/patch/81612
May, 3 2010: gspca: Try a less bandwidth-intensive alt
setting. http://patchwork.kernel.org/patch/96514
Quoted 6 lines
Hello Mauro,
I don't think the patch about pac7302 should be applied.
The patch about the gspca main is in my last git pull request.
(c/c Sarah)
I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix.
I'll mark this patch as rejected, and wait for a proper fix.
Signature · 2 lines
Cheers, Mauro
Quoted 17 lines · Mauro Carvalho Chehab
Quoted 16 lines · Jean-Francois Moine
Quoted 7 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on
heuristics http://patchwork.kernel.org/patch/81612
May, 3 2010: gspca: Try a less bandwidth-intensive alt
setting. http://patchwork.kernel.org/patch/96514Hello Mauro,
I don't think the patch about pac7302 should be applied.
Quoted 8 lines
Quoted 1 line
The patch about the gspca main is in my last git pull request.
(c/c Sarah)
I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix.
I'll mark this patch as rejected, and wait for a proper fix.
This isn't a work around for a bug in the xHCI host. The bandwidth checking is a feature. It allows the host to reject a new interface if other devices are already taking up too much of the bus bandwidth. I expect that all drivers that use interrupt or isochronous will have to fall back to a different alternate interface setting if they can.
Now, Alan Stern and I have been talking about making a different API for drivers to request a specific polling rate and frame list length for an endpoint. However, I expect that the call would have to be either before or part of the call to usb_set_interface(), because of how the xHCI hardware tracks endpoints. So even if we had the ideal interface, the drivers would still need code like this to fall back to less-bandwidth intensive alternate settings.
Is there a different way you think we should handle running out of bandwidth?
Sarah Sharp
Quoted 37 lines · Sarah Sharp
Quoted 13 lines · Mauro Carvalho Chehab
Quoted 12 lines · Jean-Francois Moine
Quoted 7 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on
heuristics http://patchwork.kernel.org/patch/81612
May, 3 2010: gspca: Try a less bandwidth-intensive alt
setting. http://patchwork.kernel.org/patch/96514Hello Mauro,
Quoted 8 lines
Quoted 2 lines
I don't think the patch about pac7302 should be applied. The patch about the gspca main is in my last git pull request.
(c/c Sarah)
I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix.
I'll mark this patch as rejected, and wait for a proper fix.
This isn't a work around for a bug in the xHCI host. The bandwidth checking is a feature. It allows the host to reject a new interface if other devices are already taking up too much of the bus bandwidth. I expect that all drivers that use interrupt or isochronous will have to fall back to a different alternate interface setting if they can.
Now, Alan Stern and I have been talking about making a different API for drivers to request a specific polling rate and frame list length for an endpoint. However, I expect that the call would have to be either before or part of the call to
usb_set_interface(), because of how the xHCI hardware tracks endpoints. So even if we had the ideal interface, the drivers would still need code like this to fall back to less-bandwidth intensive alternate settings.
Quoted 2 lines
Is there a different way you think we should handle running out of bandwidth?
Sarah,
A loop like the above doesn't work for video streams. The point is that the hardware got programmed to some resolution and frame rate. For example, a typical case is to program the hardware to do 640x480x30fps. Assuming a device with 16 bits per pixel, this means a bandwidth of 18.432 Mbps.
With V4L API, the resolution is configured before starting stream (so, before the place where usb_set_interface is called). After having all video stream parameters configured via several different ioctls, a call to VIDIOC_REQBUFS is done, causing the allocation of the streaming buffers and the corresponding USB interface setup. On that moment, it is too late to negotiate the bandwidth. So, if xHCI is not capable of supporting at least 18.432 Mbps (assuming my example), the call should simply fail.
In thesis, all V4L/DVB USB drivers have a code where it plays with USB interface alternates, until they found the minimum alternate that is capable of handing the bandwidth needed by the stream. As there's no standard way, at USB core, to set an isoc bandwidth [1], each driver has its own logic for doing that, and maybe some strategies are better than the others.
I'm not sure if it would be simpler or possible, but an alternative way would be to have something like usb_request_bandwidth(), where the USB core would have the logic to set the interface alternates to fulfill the bandwidth needs, or return -EINVAL if it is not possible. The advantage is that you won't need to fix all the different logics used by the V4L/DVB drivers.
[1] On some devices, even needing a constant bandwitdh, they only support bulk transfers, due to hardware limitation, but, from the driver's perspective, the seek for the alternate has an identical need.
Signature · 2 lines
Cheers, Mauro
Quoted 75 lines · On Mon, 10 May 2010 13:25:00 -0300 Mauro Carvalho Chehab <[email protected]>…
Quoted 40 lines · Sarah Sharp
Quoted 14 lines · Mauro Carvalho Chehab
Quoted 13 lines · Jean-Francois Moine
Quoted 8 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on heuristics
http://patchwork.kernel.org/patch/81612 May, 3 2010: gspca: Try a less bandwidth-intensive alt setting.
http://patchwork.kernel.org/patch/96514Hello Mauro,
Quoted 8 lines
Quoted 2 lines
I don't think the patch about pac7302 should be applied. The patch about the gspca main is in my last git pull request.
(c/c Sarah)
I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix.
I'll mark this patch as rejected, and wait for a proper fix.
This isn't a work around for a bug in the xHCI host. The bandwidth checking is a feature. It allows the host to reject a new interface if other devices are already taking up too much of the bus bandwidth. I expect that all drivers that use interrupt or isochronous will have to fall back to a different alternate interface setting if they can.
Now, Alan Stern and I have been talking about making a different API for drivers to request a specific polling rate and frame list length for an endpoint. However, I expect that the call would have to be either before or part of the call to
usb_set_interface(), because of how the xHCI hardware tracks endpoints. So even if we had the ideal interface, the drivers would still need code like this to fall back to less-bandwidth intensive alternate settings.Quoted 2 lines
Is there a different way you think we should handle running out of bandwidth?
Sarah,
A loop like the above doesn't work for video streams. The point is that the hardware got programmed to some resolution and frame rate. For example, a typical case is to program the hardware to do 640x480x30fps. Assuming a device with 16 bits per pixel, this means a bandwidth of 18.432 Mbps.
With V4L API, the resolution is configured before starting stream (so, before the place where
usb_set_interfaceis called). After having all video stream parameters configured via several different ioctls, a call toVIDIOC_REQBUFSis done, causing the allocation of the streaming buffers and the corresponding USB interface setup. On that moment, it is too late to negotiate the bandwidth. So, if xHCI is not capable of supporting at least 18.432 Mbps (assuming my example), the call should simply fail.In thesis, all V4L/DVB USB drivers have a code where it plays with USB interface alternates, until they found the minimum alternate that is capable of handing the bandwidth needed by the stream. As there's no standard way, at USB core, to set an isoc bandwidth [1], each driver has its own logic for doing that, and maybe some strategies are better than the others.
I'm not sure if it would be simpler or possible, but an alternative way would be to have something like
usb_request_bandwidth(), where the USB core would have the logic to set the interface alternates to fulfill the bandwidth needs, or return-EINVALif it is not possible. The advantage is that you won't need to fix all the different logics used by the V4L/DVB drivers.
Quoted 4 lines
[1] On some devices, even needing a constant bandwitdh, they only support bulk transfers, due to hardware limitation, but, from the driver's perspective, the seek for the alternate has an identical need.
Hi Mauro,
Contrary to other webcam drivers, gspca already has a fallback to a different altsetting. Actually, this is done at URB submit time and it works correctly, mainly with USB-1.1. It seems that the bandwidth may be now checked when setting the altsetting. The proposed patch does not change the gspca logic at all. Maybe it could be done in a clearer way.
A first problem with webcams is that the bandwidth is not fully known when starting the capture. The frame rate may be changed during streaming either by direct video control (rare) or by changing the exposure time (manual or automatic - AEC). Also, the video stream is often compressed and the compression ratio is not well known (some webcams automatically change the JPEG quality).
The second problem is the USB interface. If a driver could calculate a good estimation of the needed bandwidth, it cannot actually tell it to the USB subsystem. The USB subsystem uses the packet length and the message interval to calculate the bandwidth that the device will use. This is indeed not correct because the device does not send data at the full interface speed. In an other way, a driver could search the alternate setting which matches the best its estimated bandwidth, but I am not sure that this will not slow down the frame rate (more USB packets for the same data volume).
Hi Sarah,
Your patch should be changed to follow the global altsetting fallback mechanism. This mechanism is inside the function gspca_init_transfer(). If usb_set_interface() returns an error when the bandwidth is not wide enough (BTW, which is this error?), it should called inside the loop and, so, removed from get_ep().
Cheers.
Signature · 2 lines
Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
Quoted 47 lines · Jean-Francois Moine
Quoted 44 lines · On Mon, 10 May 2010 13:25:00 -0300 Mauro Carvalho Chehab <[email protected]>…
Quoted 42 lines · Sarah Sharp
Quoted 14 lines · Mauro Carvalho Chehab
Quoted 13 lines · Jean-Francois Moine
Quoted 8 lines · On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab <[email protected]>…
== Gspca patches - Waiting Jean-Francois Moine<[email protected]> submission/review ==
Feb,24 2010: gspca pac7302: add USB PID range based on heuristics
http://patchwork.kernel.org/patch/81612 May, 3 2010: gspca: Try a less bandwidth-intensive alt setting.
http://patchwork.kernel.org/patch/96514Hello Mauro,
Quoted 8 lines
Quoted 2 lines
I don't think the patch about pac7302 should be applied. The patch about the gspca main is in my last git pull request.
(c/c Sarah)
I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix.
I'll mark this patch as rejected, and wait for a proper fix.
This isn't a work around for a bug in the xHCI host. The bandwidth checking is a feature. It allows the host to reject a new interface if other devices are already taking up too much of the bus bandwidth. I expect that all drivers that use interrupt or isochronous will have to fall back to a different alternate interface setting if they can.
Now, Alan Stern and I have been talking about making a different API for drivers to request a specific polling rate and frame list length for an endpoint. However, I expect that the call would have to be either before or part of the call to
usb_set_interface(), because of how the xHCI hardware tracks endpoints. So even if we had the ideal interface, the drivers would still need code like this to fall back to less-bandwidth intensive alternate settings.Is there a different way you think we should handle running out of bandwidth?
Quoted 7 lines
Quoted 7 lines
Sarah,
A loop like the above doesn't work for video streams. The point is that the hardware got programmed to some resolution and frame rate. For example, a typical case is to program the hardware to do 640x480x30fps. Assuming a device with 16 bits per pixel, this means a bandwidth of 18.432 Mbps.
The raw bandwidth number (18.432 Mbps) isn't useful to the xHCI hardware. It wants to know the maximum packet size the driver needs, along with the number of additional opportunities per microframe the device can handle and the periodic interval to poll the endpoint at.
Quoted 2 lines
Quoted 2 lines
With V4L API, the resolution is configured before starting stream (so, before the place where
usb_set_interfaceis called).
Are you talking about programming the USB device to have a specific resolution and frame rate? If so, is that done through control transfers or some other method?
Quoted 15 lines
Quoted 15 lines
After
having all video stream parameters configured via several different ioctls, a call toVIDIOC_REQBUFSis done, causing the allocation of the streaming buffers and the corresponding USB interface setup. On that moment, it is too late to negotiate the bandwidth. So, if xHCI is not capable of supporting at least 18.432 Mbps (assuming my example), the call should simply fail.In thesis, all V4L/DVB USB drivers have a code where it plays with USB interface alternates, until they found the minimum alternate that is capable of handing the bandwidth needed by the stream. As there's no standard way, at USB core, to set an isoc bandwidth [1], each driver has its own logic for doing that, and maybe some strategies are better than the others.
Quoted 6 lines
Quoted 6 lines
I'm not sure if it would be simpler or possible, but an alternative way would be to have something like
usb_request_bandwidth(), where the USB core would have the logic to set the interface alternates to fulfill the bandwidth needs, or return-EINVALif it is not possible. The advantage is that you won't need to fix all the different logics used by the V4L/DVB drivers.
That could be do-able.
Can you guarantee that the drivers won't attempt to use the old alternate setting between a call to usb_request_bandwidth() in V4L and a call to usb_set_interface() in the drivers? The problem is that we actually need to install the new alternate interface in the xHCI hardware to find out if we have enough bandwidth. Drivers can't issue URBs to the old alternate setting after this bandwidth check.
Quoted 4 lines
Quoted 4 lines
[1] On some devices, even needing a constant bandwitdh, they only support bulk transfers, due to hardware limitation, but, from the driver's perspective, the seek for the alternate has an identical need.
Bulk transfers can't have guaranteed bandwidth under xHCI. The xHCI host controller driver has no control over when the bulk transfers are scheduled, since it is all done in hardware.
Quoted 8 lines
Hi Mauro,
Contrary to other webcam drivers, gspca already has a fallback to a different altsetting. Actually, this is done at URB submit time and it works correctly, mainly with USB-1.1. It seems that the bandwidth may be now checked when setting the altsetting. The proposed patch does not change the gspca logic at all. Maybe it could be done in a clearer way.
Quoted 6 lines
A first problem with webcams is that the bandwidth is not fully known when starting the capture. The frame rate may be changed during streaming either by direct video control (rare) or by changing the exposure time (manual or automatic - AEC). Also, the video stream is often compressed and the compression ratio is not well known (some webcams automatically change the JPEG quality).
Hmm, can you give an accurate estimate of the maximum bandwidth you'll use? For example, if you don't know whether compression is used, just assume it won't and calculate the max bandwidth with that assumption.
If there was an API to change the required bandwidth during capture (say when the exposure time is changed), what kind of latencies could the user tolerate? Could they tolerate dropping a couple frames of data?
I ask because it takes some time to check whether the hardware can support the new bandwidth requirements. The xHCI driver must stop all transfers to the effected endpoints, issue a command, and wait for it to be processed (which takes a variable length of time, based on how many commands are already being processed). It's probably better to allocate the maximum bandwidth possible instead.
Quoted 6 lines
The second problem is the USB interface. If a driver could calculate a good estimation of the needed bandwidth, it cannot actually tell it to the USB subsystem. The USB subsystem uses the packet length and the message interval to calculate the bandwidth that the device will use. This is indeed not correct because the device does not send data at the full interface speed.
How does it differ? Is the length in each usb_iso_packet_descriptor consistently smaller than the max packet size the endpoint advertises? Or are there bursts of the maximum packet size and then zero length transfers?
Quoted 4 lines
In an other way, a driver could search the
alternate setting which matches the best its estimated bandwidth, but I am not sure that this will not slow down the frame rate (more USB packets for the same data volume).
Can you explain what you mean by this? I understand that there's more bus bandwidth overhead for transferring many small packets versus transferring few big packets, but I'm not sure what you're getting at.
Quoted 7 lines
Hi Sarah,
Your patch should be changed to follow the global altsetting fallback mechanism. This mechanism is inside the function
gspca_init_transfer(). Ifusb_set_interface()returns an error when the bandwidth is not wide enough (BTW, which is this error?), it should called inside the loop and, so, removed fromget_ep().
I'll look at how gspca_init_transfer() does the fallback and try to update the patch. I'm fine with it being dropped, now that the reasons have been explained.
Sarah Sharp
Quoted 5 lines
From: Mauro Carvalho Chehab <[email protected]>
== New or updated patches starting from May, 5 or newer - not handled yet ==May, 5 2010: [-next] media/mem2mem: dereferencing free memory
http://patchwork.kernel.org/patch/96984
This one is obvious, but might as well:
Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center
Hi,
Quoted 8 lines
Mauro Carvalho Chehab <[email protected]> wrote:
== Videobuf patches - Need more tests before committing it - Volunteers? ==Apr,21 2010: [v1, 1/2] v4l: videobuf: Add support for out-of-order buffer dequeuing
http://patchwork.kernel.org/patch/93901
Apr,21 2010: [v1, 2/2] v4l: vivi: adapt to out-of-order buffer dequeuing in videobu http://patchwork.kernel.org/patch/93903
it'd be great if there was a driver/device that would be benefiting from this, i.e. that may want to:
- return buffers in a different order than queued,
- process them in parallel,
- process them in a different order,
or anybody interested in those features. Is anybody aware of any such devices in the V4L tree?
This is also the first step to simplifying videobuf and making it lighter by removing superficial (to my best knowledge) waitqueues in every videobuf_buffer structure, as discussed at the Oslo meeting.
Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center
Quoted 1 line · Mauro Carvalho Chehab
My original idea were to send one of such emails per week,
Nearly two months has passed since this message. I apologize if I missed something, but I have not seen another status update. Is it just me?
Anyway, since the last status I've seen, 2.6.34 has been released, the 2.6.35 merge window has been open and then closed, and a number of new patches have been collected in
https://patchwork.kernel.org/project/linux-media/list/
, many of which seem rather trivial.
Any chance of a new status update anytime soon? I'm particularily interested in getting a forced status change on any patch which was "under review" at the time of the last status message. I believe it's reasonable to expect two months "review" to be more than enough. If the patches are found unacceptable, then it's much better to have them rejected with a "please fix foo and resubmit" than the current total silence called "review".
Thanks,
Bjørn
Em 01-07-2010 08:46, Bjørn Mork escreveu:
Quoted 9 lines
Quoted 1 line · Mauro Carvalho Chehab
My original idea were to send one of such emails per week,
Nearly two months has passed since this message. I apologize if I missed something, but I have not seen another status update. Is it just me?
Quoted 5 lines
Anyway, since the last status I've seen, 2.6.34 has been released, the 2.6.35 merge window has been open and then closed, and a number of new patches have been collected in
https://patchwork.kernel.org/project/linux-media/list/
, many of which seem rather trivial.
Ideally, the patches that are OK should already be catched by a driver maintainer and submitted me via git or mercurial pull request, and the bad patches should already be nacked. The ones that are not merged from the normal process are the ones that are not trivial, among others, due to one of the reasons bellow:
- there's no driver maintainer;
- the driver maintainer is lazy or overloaded;
- there are some concerns about that patch;
- there are some changes undergoing that will affect/be affected by the patch;
- the patch got forgotten/lost in the middle of the emails.
The fact is that the number of those patches are very high, causes me a lot of overload to handle them and to send emails to people requesting the review of those patches.
Quoted 1 line
Any chance of a new status update anytime soon?
Updated today, after two or three weeks spent to handle the backlog.
I still have a few pull requests pending for 2.6.35-rc (bug fixes) that I'll be handling this week.
Quoted 7 lines
I'm particularily
interested in getting a forced status change on any patch which was "under review" at the time of the last status message. I believe it's reasonable to expect two months "review" to be more than enough. If the patches are found unacceptable, then it's much better to have them rejected with a "please fix foo and resubmit" than the current total silence called "review".
The patches marked as under review means that I'm expecting an action from someone else (the patch author or the driver author/maintainer).
So, if you have patches there still under review, you're helping us if you direct your complains to the one that it is sitting on the top of them.
Cheers,
Mauro.
Quoted 4 lines · Mauro Carvalho Chehab
Em 01-07-2010 08:46, Bjørn Mork escreveu:
Quoted 1 line
Any chance of a new status update anytime soon?
Updated today, after two or three weeks spent to handle the backlog.
Great! Thanks. It's really appreciated, and I do note that it made quite a few people finally ack/nak the patches they were supposed to review.
Quoted 10 lines
Quoted 7 lines
I'm particularily
interested in getting a forced status change on any patch which was "under review" at the time of the last status message. I believe it's reasonable to expect two months "review" to be more than enough. If the patches are found unacceptable, then it's much better to have them rejected with a "please fix foo and resubmit" than the current total silence called "review".The patches marked as under review means that I'm expecting an action from someone else (the patch author or the driver author/maintainer).
Well, I'm of course not in a position to tell you how to do your job, so please regard this as a humble suggestion only...
But I believe you make your job much harder by defining a number of "unofficial" driver maintainers and giving them indefinite slack, while at the same time *you* are the one having to keep track of all their outstanding patches. Either you delegate the maintainance properly, documenting it in MAINTAINERS and pointing there whenever someone sends a patch directly to you, or you might as well just do the ack/nak yourself based on the mailing list feedback.
Putting yourself in the middle, taking the patch queue responsibility, but not the ack/nak responsibility, is just wasting your time on accounting and other boring work...
I do believe that having the original author(s) maintain a driver is a very good idea as long as they are still actively maintaining it. But this must be based on actual maintainance, and not some misunderstood "ownership based on previous contributions". That's what the CREDITS file is for.
Please look at other subsystems with a large number of old drivers, like e.g. networking. It's not like it's possible to have every tiny patch approved by the original author all the time. This does not hinder some newer drivers having very active official maintainers, like the Intel e1000(e) drivers, nor does it hinder the original authors from participating on the mailing list giving their comments and ack/nak if they want. But if they don't respond on the list, davem will just make a decision for himself without waiting for it.
Quoted 3 lines
So, if you have patches there still under review, you're helping us if you direct your complains to the one that it is sitting on the top of them.
Oh, it's not so much my submissions bothering me (I have received some very good feedback on this list), but the fact that some drivers do not get any updates at all, even though patches are submitted to this mailing list. Not to mention the problem that patch submissions will (and do) stop due to the lack of any feedback whatsoever. Most people have better things to do than writing to /dev/null, and that's the feeling this queuing-for-original-author-review system leaves.
Bjørn