RE: [media-workshop] drivers without explicit MAINTAINERS entry - was: Re: Tentative Agenda for the November workshop
Thanks Hans and Mauro. On any of the CX drivers, if we do not have any maintainer or odd fixer, you could add me to the odd fixer list. Rgds, Palash -Original Message- From: media-workshop-boun...@linuxtv.org [mailto:media-workshop-boun...@linuxtv.org] On Behalf Of Mauro Carvalho Chehab Sent: Friday, November 02, 2012 7:35 AM To: Hans Verkuil Cc: media-works...@linuxtv.org; linux-media Subject: Re: [media-workshop] drivers without explicit MAINTAINERS entry - was: Re: Tentative Agenda for the November workshop Em Fri, 2 Nov 2012 14:47:49 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Fri November 2 2012 14:13:10 Mauro Carvalho Chehab wrote: Em Thu, 1 Nov 2012 14:12:44 -0200 Mauro Carvalho Chehab mche...@redhat.com escreveu: Em Thu, 1 Nov 2012 16:44:50 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Thu October 25 2012 19:27:01 Mauro Carvalho Chehab wrote: Hi Hans, Em Mon, 22 Oct 2012 10:35:56 +0200 Hans Verkuil hverk...@xs4all.nl escreveu: Hi all, This is the tentative agenda for the media workshop on November 8, 2012. If you have additional things that you want to discuss, or something is wrong or incomplete in this list, please let me know so I can update the list. Thank you for taking care of it. - Explain current merging process (Mauro) - Open floor for discussions on how to improve it (Mauro) - Write down minimum requirements for new V4L2 (and DVB?) drivers, both for staging and mainline acceptance: which frameworks to use, v4l2-compliance, etc. (Hans Verkuil) - V4L2 ambiguities (Hans Verkuil) - TSMux device (a mux rather than a demux): Alain Volmat - dmabuf status, esp. with regards to being able to test (Mauro/Samsung) - Device tree support (Guennadi, not known yet whether this topic is needed) - Creating/selecting contexts for hardware that supports this (Samsung, only if time is available) I have an extra theme for discussions there: what should we do with the drivers that don't have any MAINTAINERS entry. I've added this topic to the list. Thanks! It probably makes sense to mark them as Orphan (or, at least, have some criteria to mark them as such). Perhaps before doing that, we could try to see if are there any developer at the community with time and patience to handle them. This could of course be handled as part of the discussions about how to improve the merge process, but I suspect that this could generate enough discussions to be handled as a separate theme. Do we have a 'Maintainer-Light' category? I have a lot of hardware that I can test. So while I wouldn't like to be marked as 'The Maintainer of driver X' (since I simply don't have the time for that), I wouldn't mind being marked as someone who can at least test patches if needed. There are several maintainance status there: S: Status, one of the following: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do much other than throw the odd patch in. See below.. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete:Old code. Something tagged obsolete generally means it has been replaced by a better system and you should be using that. (btw, I just realized that I should be changing the EDAC drivers I maintain to Supported; the media drivers I maintain should be kept as Maintained). I suspect that the maintainer-light category for those radio and similar old stuff is likely Odd Fixes. There are some issues by not having a MAINTAINERS entry: - patches may not flow into the driver maintainer; - patches will likely be applied without tests/reviews or may stay for a long time queued; - ./scripts/get_maintainer.pl at --no-git-fallback won't return any maintainer[1]. [1] Letting get_maintainer.pl is very time/CPU consuming. Letting it would delay a lot the patch review process, if applied for every patch, with unreliable and doubtful results. I don't do it, due to the large volume of patches, and because the 'other' results aren't typically the driver maintainer. An example of this is the results for a patch I just applied (changeset 2866aed103b915ca8ba0ff76d5790caea4e62ced): $ git show --pretty=email|./scripts/get_maintainer.pl Mauro Carvalho Chehab mche...@infradead.org (maintainer:MEDIA INPUT
RE: [patch] [media] cx25821: testing the wrong variable
Mauro, This is to be able to upstream data from the host to the device to be pushed out. I agree this is not an optimal way of doing things and had discussed with Hans on this last month. I will be making changes to the entire upstream path and get rid of this mechanism that we have in place here. Rgds, Palash -Original Message- From: Mauro Carvalho Chehab [mailto:mche...@infradead.org] Sent: Sunday, October 07, 2012 5:50 AM To: Palash Bandyopadhyay; Sri Deevi Cc: Dan Carpenter; Leonid V. Fedorenchik; Thomas Meyer; Hans Verkuil; linux-media@vger.kernel.org; kernel-janit...@vger.kernel.org Subject: Re: [patch] [media] cx25821: testing the wrong variable Em Sat, 29 Sep 2012 10:12:53 +0300 Dan Carpenter dan.carpen...@oracle.com escreveu: -input_filename could be NULL here. The intent was to test -_filename. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- I'm not totally convinced that using /root/vid411.yuv is the right idea. Agreed. Palash, Sri, Why do we need those files? Regards, Mauro diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c index 52c13e0..6759fff 100644 --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select, } /* Default if filename is empty string */ - if (strcmp(dev-input_filename, ) == 0) { + if (strcmp(dev-_filename, ) == 0) { if (dev-_isNTSC) { dev-_filename = (dev-_pixel_format == PIXEL_FRMT_411) ? diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c index c8c94fb..d33fc1a 100644 --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select, } /* Default if filename is empty string */ - if (strcmp(dev-input_filename_ch2, ) == 0) { + if (strcmp(dev-_filename_ch2, ) == 0) { if (dev-_isNTSC_ch2) { dev-_filename_ch2 = (dev-_pixel_format_ch2 == PIXEL_FRMT_411) ? /root/vid411.yuv : -- 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 Cheers, Mauro Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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: [Workshop-2011] Media summit/KS-2012 proposals
Mauro, Can you add Sri Deevi to the list too? Thanks, Palash -Original Message- From: workshop-2011-boun...@linuxtv.org [mailto:workshop-2011-boun...@linuxtv.org] On Behalf Of Mauro Carvalho Chehab Sent: Tuesday, July 31, 2012 10:58 AM To: workshop-2...@linuxtv.org; Linux Media Mailing List Subject: [Workshop-2011] Media summit/KS-2012 proposals In order to sum-up the discussions around the media summit, this is what we've got so far: Proposals proposed by =|= Common device tree bindings for media devices Sylvester Nawrocki / Guennadi Liakhovetski ALSA and V4L/Media Controller Steven Toth / Laurent Pinchart ARM and needed features for V4L/DVB Steven Toth Intel media SDK Steven Toth V4L compiance tool Hans Verkuil V4L2 API ambiguities Hans Verkuil Media Controller library Laurent Pincart / Sakari Ailus SoC Vendors feedback - how to help them to go upstream - Android's V4L2 cam library Laurent Pincart / Guennadi Liakhovetski / Palash Bandyopadhyay / Naveen Krishnamurthy Synchronization, shared resource and optimizations Pawel Osciak V4L2/DVB issues from userspace perspective Rémi Denis-Courmont As we'll have only one day for the summit, we may need to remove some themes, or maybe to get an extra time during LPC for the remaining discussions. Possible attendents: === Guennadi Liakhovetski Laurent Pinchart Mauro Carvalho Chehab Michael Krufky Naveen Krishnamurthy +1 seat from ST (waiting Naveen to define who will be the other seat) Palash Bandyopadhyay Pawel Osciak Rémi Denis-Courmont Sakari Ailus Steven Toth Sylvester Nawrocki Am I missing something? Are there other proposals or people intending to participate? Regards, Mauro ___ Workshop-2011 mailing list workshop-2...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/workshop-2011 Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit
Android -palash -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Wednesday, July 18, 2012 4:37 AM To: Palash Bandyopadhyay Cc: workshop-2...@linuxtv.org; Linux Media Mailing List Subject: Re: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit Hi Palash, On Tuesday 17 July 2012 15:57:18 Palash Bandyopadhyay wrote: Count me in when we have this discussion as this is something we too are working on. Which one, the media controller library, or Android ? On Tuesday, July 17, 2012 3:47 PM Laurent Pinchart wrote: On Tuesday 17 July 2012 14:30:53 Mauro Carvalho Chehab wrote: As we did in 2012, we're planning to do a media summit again at KS/2012. Great news! The KS/2012 will happen in San Diego, CA, US, between Aug 26-28, just before the LinuxCon North America. In order to do it, I'd like to know who is interested on participate, and to get proposals about what subjects will be discussed there, in order to start planning the agenda. Sakari and I will spend time on the long-awaited media controller library in a couple of weeks, I hope to have results to present during the summit. Depending on the audience, I would also be interested in getting feedback from SoC vendors who are not (yet) active in the V4L2 community on our approach and how we could best help them. This could include discussions about Android, as I believe we need to push V4L2 on that platform. Guennadi's recent work on an Android V4L2 camera library is a good first step in that direction. -- Regards, Laurent Pinchart Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit
Count me in when we have this discussion as this is something we too are working on. Rgds, Palash -Original Message- From: workshop-2011-boun...@linuxtv.org [mailto:workshop-2011-boun...@linuxtv.org] On Behalf Of Laurent Pinchart Sent: Tuesday, July 17, 2012 3:47 PM To: workshop-2...@linuxtv.org Cc: Linux Media Mailing List Subject: Re: [Workshop-2011] Media summit at the Kernel Summit - was: Fwd: Re: [Ksummit-2012-discuss] Organising Mini Summits within the Kernel Summit Hi Mauro, On Tuesday 17 July 2012 14:30:53 Mauro Carvalho Chehab wrote: As we did in 2012, we're planning to do a media summit again at KS/2012. Great news! The KS/2012 will happen in San Diego, CA, US, between Aug 26-28, just before the LinuxCon North America. In order to do it, I'd like to know who is interested on participate, and to get proposals about what subjects will be discussed there, in order to start planning the agenda. Sakari and I will spend time on the long-awaited media controller library in a couple of weeks, I hope to have results to present during the summit. Depending on the audience, I would also be interested in getting feedback from SoC vendors who are not (yet) active in the V4L2 community on our approach and how we could best help them. This could include discussions about Android, as I believe we need to push V4L2 on that platform. Guennadi's recent work on an Android V4L2 camera library is a good first step in that direction. -- Regards, Laurent Pinchart ___ Workshop-2011 mailing list workshop-2...@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/workshop-2011 Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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 0/12] struct i2c_algo_bit_data cleanup on several drivers
Thanks Ezequiel and Dan. The changes look ok. I'll have someone check out the changes on the CX devices. Rgds, Palash -Original Message- From: Ezequiel Garcia [mailto:elezegar...@gmail.com] Sent: Monday, June 18, 2012 12:23 PM To: Mauro Carvalho Chehab Cc: linux-media; Dan Carpenter; Palash Bandyopadhyay; st...@kernellabs.com Subject: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers Hi Mauro, This patchset cleans the i2c part of some drivers. This issue was recently reported by Dan Carpenter [1], and revealed wrong (and harmless) usage of struct i2c_algo_bit. Also, I properly assigned bus-i2c_rc (return code variable) and replaced struct memcpy with struct assignment. The latter is, in my opinion, a much safer way for struct filling and I'm not aware of any drawbacks. The patches are based on today's linux-next; I hope this is okey. As I don't own any of these devices, I can't test the changes beyond compilation. Ezequiel Garcia (12): cx25821: Replace struct memcpy with struct assignment cx25821: Remove useless struct i2c_algo_bit_data usage cx25821: Use i2c_rc properly to store i2c register status cx231xx: Replace struct memcpy with struct assignment cx231xx: Remove useless struct i2c_algo_bit_data usage cx231xx: Use i2c_rc properly to store i2c register status cx23885: Replace struct memcpy with struct assignment cx23885: Remove useless struct i2c_algo_bit_data cx23885: Use i2c_rc properly to store i2c register status saa7164: Replace struct memcpy with struct assignment saa7164: Remove useless struct i2c_algo_bit_data saa7164: Use i2c_rc properly to store i2c register status drivers/media/video/cx231xx/cx231xx-i2c.c | 10 +++--- drivers/media/video/cx231xx/cx231xx.h |2 -- drivers/media/video/cx23885/cx23885-i2c.c | 12 +++- drivers/media/video/cx23885/cx23885.h |2 -- drivers/media/video/cx25821/cx25821-i2c.c | 12 +++- drivers/media/video/cx25821/cx25821.h |2 -- drivers/media/video/saa7164/saa7164-i2c.c | 13 +++-- drivers/media/video/saa7164/saa7164.h |2 -- 8 files changed, 12 insertions(+), 43 deletions(-) Thanks, Ezequiel. [1] http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553 Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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: V4L/DVB (12730): Add conexant cx25821 driver
Thanks Dan. Let me take a look and get back to you. Rgds, Palash From: Dan Carpenter [dan.carpen...@oracle.com] Sent: Saturday, June 16, 2012 6:16 AM To: Palash Bandyopadhyay; mche...@redhat.com Cc: linux-media@vger.kernel.org Subject: re: V4L/DVB (12730): Add conexant cx25821 driver Hello Palash Bandyopadhyay, The patch 02b20b0b4cde: V4L/DVB (12730): Add conexant cx25821 driver from Sep 15, 2009, leads to the following warning: drivers/media/video/cx25821/cx25821-i2c.c:310 cx25821_i2c_register() error: memcpy() 'cx25821_i2c_algo_template' too small (24 vs 64) 306 dprintk(1, %s(bus = %d)\n, __func__, bus-nr); 307 308 memcpy(bus-i2c_adap, cx25821_i2c_adap_template, 309 sizeof(bus-i2c_adap)); 310 memcpy(bus-i2c_algo, cx25821_i2c_algo_template, 311 sizeof(bus-i2c_algo)); 312 memcpy(bus-i2c_client, cx25821_i2c_client_template, 313 sizeof(bus-i2c_client)); 314 The problem is that bus-i2c_algo is a i2c_algo_bit_data struct and cx25821_i2c_algo_template is a i2c_algorithm struct. They are different sizes and the function pointers don't line up at all. I don't see how this can work at all. regards, dan carpenter Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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] Fix regression introduced which broke the Hauppauge USBLive 2
Mauro/Devin, Can someone give steps to reproduce the problem? Also if we need any particular h/w board to reproduce this problem. I dont seem to recall any delay requirement on the chip at power up/cycle time. Any I also dont recall seeing any problems with the Conexant evk boards. Mauro, have you been able to see this issue with a Conexant board? Thanks, Palash From: Mauro Carvalho Chehab [mche...@redhat.com] Sent: Sunday, July 24, 2011 5:57 AM To: Devin Heitmueller Cc: Linux Media Mailing List; Gerd Hoffmann; Sri Deevi; Palash Bandyopadhyay Subject: Re: [PATCH] Fix regression introduced which broke the Hauppauge USBLive 2 Hi Devin, Em 23-07-2011 22:17, Devin Heitmueller escreveu: The following patch addresses the regression introduced in the cx231xx driver which stopped the Hauppauge USBLive2 from working. Confirmed working by both myself and the user who reported the issue on the KernelLabs blog (Robert DeLuca). cx231xx: Fix regression introduced which broke the Hauppauge USBLive 2 From: Devin Heitmueller dheitmuel...@kernellabs.com At some point during refactoring of the cx231xx driver, the USBLive 2 device became broken. This patch results in the device working again. Thanks to Robert DeLuca for sponsoring this work. Signed-off-by: Devin Heitmueller dheitmuel...@kernellabs.com Cc: Robert DeLuca robertdel...@me.com diff --git a/drivers/media/video/cx231xx/cx231xx-cards.c b/drivers/media/video/cx231xx/cx231xx-cards.c index 4b22afe..d02c63a 100644 --- a/drivers/media/video/cx231xx/cx231xx-cards.c +++ b/drivers/media/video/cx231xx/cx231xx-cards.c @@ -387,6 +387,7 @@ struct cx231xx_board cx231xx_boards[] = { .norm = V4L2_STD_NTSC, .no_alt_vanc = 1, .external_av = 1, + .dont_use_port_3 = 1, .input = {{ .type = CX231XX_VMUX_COMPOSITE1, .vmux = CX231XX_VIN_2_1, I proposed the same fix sometime ago, when Gerd reported this issue for me. His feedback was that this partially fixed the issue, but he reported that he also needed to increase the set_power_mode delay from 5 to 50 ms in order to fully initialize the cx231xx hardware, as on the enclosed patch. It seems he tested with vanilla Fedora kernel. So, I suspect that HZ may be affecting this driver as well. As you know, if HZ is configured with 100, the minimum msleep() delay will be 10. so, instead of waiting for 5ms, it will wait for 10ms for the device to powerup. It would be great to configure HZ with 1000 and see the differences with and without Gerd's patch. Cheers, Mauro. - From a83a7574e07b48b1c6a222d833a7fa0a67133b5c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann kra...@gmail.com Date: Thu, 16 Dec 2010 17:39:17 +0100 Subject: [PATCH] cx231xx: raise delay after powerup. Wait a bit longer after power up so the chips have enougth time to come up before we try talking to them via i2c. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/media/video/cx231xx/cx231xx-avcore.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/cx231xx/cx231xx-avcore.c b/drivers/media/video/cx231xx/cx231xx-avcore.c index cf50faf..cf412cd 100644 --- a/drivers/media/video/cx231xx/cx231xx-avcore.c +++ b/drivers/media/video/cx231xx/cx231xx-avcore.c @@ -2412,7 +2412,7 @@ int cx231xx_set_power_mode(struct cx231xx *dev, enum AV_MODE mode) break; } - msleep(PWR_SLEEP_INTERVAL); + msleep(PWR_SLEEP_INTERVAL * 10); /* For power saving, only enable Pwr_resetout_n when digital TV is selected. */ -- Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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] Fix regression introduced which broke the Hauppauge USBLive 2
Mauro and Devin, Thanks for the explainations. The fix proposed is ok and as Devin mentioned, should have very low impact. I'll see if I can dig into the issue to root cause it. Thanks for your help. Rgds, Palash Signed off by Palash Bandyopadhyay From: Mauro Carvalho Chehab [mche...@redhat.com] Sent: Monday, July 25, 2011 5:58 AM To: Devin Heitmueller Cc: Palash Bandyopadhyay; Linux Media Mailing List; Gerd Hoffmann; Sri Deevi Subject: Re: [PATCH] Fix regression introduced which broke the Hauppauge USBLive 2 Hi Palash, Em 25-07-2011 07:24, Devin Heitmueller escreveu: On Mon, Jul 25, 2011 at 1:48 AM, Palash Bandyopadhyay palash.bandyopadh...@conexant.com wrote: Mauro/Devin, Can someone give steps to reproduce the problem? Also if we need any particular h/w board to reproduce this problem. I dont seem to recall any delay requirement on the chip at power up/cycle time. Any I also dont recall seeing any problems with the Conexant evk boards. Mauro, have you been able to see this issue with a Conexant board? This problem were reported with Hauppauge USBLive 2 by several users. I had a similar problem with another grabber device. My guess is that, on non-grabber devices, the additional steps needed to control other device elements spend some time, and, due to that, the delay timing issue is not noticed. The first strategy used to fix such bug is the following patch, that I've forwarded you, back in December: From a83a7574e07b48b1c6a222d833a7fa0a67133b5c Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann kra...@gmail.com Date: Thu, 16 Dec 2010 17:39:17 +0100 Subject: [PATCH] cx231xx: raise delay after powerup. Wait a bit longer after power up so the chips have enougth time to come up before we try talking to them via i2c. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- drivers/media/video/cx231xx/cx231xx-avcore.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/cx231xx/cx231xx-avcore.c b/drivers/media/video/cx231xx/cx231xx-avcore.c index cf50faf..cf412cd 100644 --- a/drivers/media/video/cx231xx/cx231xx-avcore.c +++ b/drivers/media/video/cx231xx/cx231xx-avcore.c @@ -2412,7 +2412,7 @@ int cx231xx_set_power_mode(struct cx231xx *dev, enum AV_MODE mode) break; } - msleep(PWR_SLEEP_INTERVAL); + msleep(PWR_SLEEP_INTERVAL * 10); /* For power saving, only enable Pwr_resetout_n when digital TV is selected. */ The problem occurs if the kernel is configured for CONFIG_HZ to 1000 and on the Hauppauge USBLIve 2 hardware design. I'm assuming it's tied to two factors: 1. Some aspect of the power supply tied to the Mako core being a little slow to startup. I haven't compared the schematic to the EVK, but I could do that if we *really* think that is a worthwhile exercise. 2. On typical kernels which have CONFIG_HZ set to 100, a call to msleep(5) will actually take approximately 10ms (due to the clock resolution). However, if you have CONFIG_HZ set to 1000 then the call *actually* takes 5ms which is too short for this design. In other words, you would be unlikely to notice the issue unless you had CONFIG_HZ set to a high enough resolution for the msleep(5) to actually take 5ms. The changed as proposed should be very low impact. For users who have CONFIG_HZ set to 100 (which is typical), there will be no visible increase in time (since the call was already taking 10ms). For users who have CONFIG_HZ set to 1000, the net effect is that it takes an extra 20ms to switch hardware modes (since the #define in question is used at most four times in sequence). I agree with Devin. The impact is probably not visible for the users whose boards are already working, but it is very significant for the ones that have grabber devices whose devices don't work due to the delay issue, as the device will start working. From my side, I don't mind if you want to solve it with another strategy, for example, like on a code that would check if the device was actually powered-up, but, if just increasing the delay to 10 ms is is enough and won't cause side effects, I would just get the simplest fix. Cheers, Mauro Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- To unsubscribe from this list: send the line unsubscribe linux-media in the body
RE: [git:v4l-dvb/v2.6.37] V4L/DVB: Staging: cx25821: fix braces and space coding style issues
Thanks. Looks ok. Signed off by Palash Bandyopadhyay -Original Message- From: Mauro Carvalho Chehab [mailto:mche...@redhat.com] Sent: Thursday, October 07, 2010 11:37 AM To: linuxtv-comm...@linuxtv.org Cc: Ruslan Pisarev; Palash Bandyopadhyay Subject: [git:v4l-dvb/v2.6.37] V4L/DVB: Staging: cx25821: fix braces and space coding style issues 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: Staging: cx25821: fix braces and space coding style issues Author: Ruslan Pisarev ruslanpisa...@gmail.com Date:Mon Sep 27 10:01:36 2010 -0300 Errors found by the checkpatch.pl tool. [mche...@redhat.com: merged a series of CodingStyle cleanup patches for cx25851. They're all from the same author, and patches the same driver] Signed-off-by: Ruslan Pisarev rus...@rpisarev.org.ua Cc: Palash Bandyopadhyay palash.bandyopadh...@conexant.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com drivers/staging/cx25821/cx25821-audio-upstream.c | 13 +- drivers/staging/cx25821/cx25821-audio.h| 10 +- drivers/staging/cx25821/cx25821-core.c | 62 +- drivers/staging/cx25821/cx25821-i2c.c |2 +- drivers/staging/cx25821/cx25821-medusa-reg.h | 10 +- drivers/staging/cx25821/cx25821-medusa-video.c |8 +- drivers/staging/cx25821/cx25821-reg.h |4 +- .../staging/cx25821/cx25821-video-upstream-ch2.c | 135 +--- .../staging/cx25821/cx25821-video-upstream-ch2.h | 14 +- drivers/staging/cx25821/cx25821-video-upstream.c | 28 ++-- drivers/staging/cx25821/cx25821-video-upstream.h | 10 +- drivers/staging/cx25821/cx25821.h | 51 12 files changed, 168 insertions(+), 179 deletions(-) --- http://git.linuxtv.org/media-tree.git?a=commitdiff;h=b9a1211dff08aa73fc26db66980ec0710a6c7134 diff --git a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c index cdff49f..6f32006 100644 --- a/drivers/staging/cx25821/cx25821-audio-upstream.c +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c @@ -40,8 +40,8 @@ MODULE_AUTHOR(Hiep Huynh hiep.hu...@conexant.com); MODULE_LICENSE(GPL); static int _intr_msk = -FLD_AUD_SRC_RISCI1 | FLD_AUD_SRC_OF | FLD_AUD_SRC_SYNC | -FLD_AUD_SRC_OPC_ERR; + FLD_AUD_SRC_RISCI1 | FLD_AUD_SRC_OF | FLD_AUD_SRC_SYNC | + FLD_AUD_SRC_OPC_ERR; int cx25821_sram_channel_setup_upstream_audio(struct cx25821_dev *dev, struct sram_channel *ch, @@ -506,7 +506,7 @@ int cx25821_audio_upstream_irq(struct cx25821_dev *dev, int chan_num, { int i = 0; u32 int_msk_tmp; - struct sram_channel *channel = dev-channels[chan_num].sram_channels; + struct sram_channel *channel = dev-channels[chan_num].sram_channels; dma_addr_t risc_phys_jump_addr; __le32 *rp; @@ -608,7 +608,7 @@ static irqreturn_t cx25821_upstream_irq_audio(int irq, void *dev_id) if (!dev) return -1; - sram_ch = dev-channels[dev-_audio_upstream_channel_select]. + sram_ch = dev-channels[dev-_audio_upstream_channel_select]. sram_channels; msk_stat = cx_read(sram_ch-int_mstat); @@ -733,7 +733,7 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) } dev-_audio_upstream_channel_select = channel_select; - sram_ch = dev-channels[channel_select].sram_channels; + sram_ch = dev-channels[channel_select].sram_channels; /* Work queue */ INIT_WORK(dev-_audio_work_entry, cx25821_audioups_handler); @@ -764,9 +764,8 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) str_length + 1); /* Default if filename is empty string */ - if (strcmp(dev-input_audiofilename, ) == 0) { + if (strcmp(dev-input_audiofilename, ) == 0) dev-_audiofilename = /root/audioGOOD.wav; - } } else { str_length = strlen(_defaultAudioName); dev-_audiofilename = kmalloc(str_length + 1, GFP_KERNEL); diff --git a/drivers/staging/cx25821/cx25821-audio.h b/drivers/staging/cx25821/cx25821-audio.h index 434b2a3..a702a0d 100644 --- a/drivers/staging/cx25821/cx25821-audio.h +++ b/drivers/staging/cx25821/cx25821-audio.h @@ -31,18 +31,18 @@ #define NUMBER_OF_PROGRAMS 8 /* - Max size of the RISC program for a buffer. - worst case is 2 writes per line - Space is also added for the 4 no-op instructions added on the end. -*/ + * Max size of the RISC program for a buffer. - worst case is 2 writes per line + * Space is also added for the 4 no-op instructions added on the end. + */ #ifndef USE_RISC_NOOP #define MAX_BUFFER_PROGRAM_SIZE \ -(2*LINES_PER_BUFFER
RE: [cx25821] Removed duplicate code and cleaned up
Thanks Mauro. I'll discuss with the team and come up with a solution for this. BTW, any update on the Polaris patches? I had resent the 1/2 an 2/2 patches. Thanks, Palash From: Mauro Carvalho Chehab [mche...@redhat.com] Sent: Sunday, July 04, 2010 10:26 PM To: Palash Bandyopadhyay Cc: linux-media@vger.kernel.org; Jay Guillory Subject: Re: [cx25821] Removed duplicate code and cleaned up Em 10-06-2010 01:27, Palash Bandyopadhyay escreveu: From 58fd4cad5f6acbe16bff86b8e878d2352cc73637 Mon Sep 17 00:00:00 2001 Message-Id: 58fd4cad5f6acbe16bff86b8e878d2352cc73637.1276143362.git.palash.bandyopadh...@conexant.com From: palash palash.bandyopadh...@conexant.com Date: Wed, 9 Jun 2010 21:11:20 -0700 Subject: [cx25821] Removed duplicate code and cleaned up To: linux-media@vger.kernel.org Signed-off-by: palash palash.bandyopadh...@conexant.com delete mode 100644 drivers/staging/cx25821/cx25821-audups11.c delete mode 100644 drivers/staging/cx25821/cx25821-video0.c delete mode 100644 drivers/staging/cx25821/cx25821-video1.c delete mode 100644 drivers/staging/cx25821/cx25821-video2.c delete mode 100644 drivers/staging/cx25821/cx25821-video3.c delete mode 100644 drivers/staging/cx25821/cx25821-video4.c delete mode 100644 drivers/staging/cx25821/cx25821-video5.c delete mode 100644 drivers/staging/cx25821/cx25821-video6.c delete mode 100644 drivers/staging/cx25821/cx25821-video7.c delete mode 100644 drivers/staging/cx25821/cx25821-videoioctl.c delete mode 100644 drivers/staging/cx25821/cx25821-vidups10.c delete mode 100644 drivers/staging/cx25821/cx25821-vidups9.c Hi Palash, I've applied your patch at staging/other branch, and added there a few patches to cleanup some troubles I saw on the driver. I was wanting to move it out of staging, but, unfortunately, the driver is not ready yet. One of the bad things I saw on the driver is this part: $ grep /root/ drivers/staging/cx25821/* drivers/staging/cx25821/cx25821-audio-upstream.c: dev-_audiofilename = /root/audioGOOD.wav; drivers/staging/cx25821/cx25821-audio-upstream.h:char *_defaultAudioName = /root/audioGOOD.wav; drivers/staging/cx25821/cx25821-video-upstream.c: PIXEL_FRMT_411) ? /root/vid411.yuv : drivers/staging/cx25821/cx25821-video-upstream.c: /root/vidtest.yuv; drivers/staging/cx25821/cx25821-video-upstream.c: PIXEL_FRMT_411) ? /root/pal411.yuv : drivers/staging/cx25821/cx25821-video-upstream.c: /root/pal422.yuv; drivers/staging/cx25821/cx25821-video-upstream-ch2.c: PIXEL_FRMT_411) ? /root/vid411.yuv : drivers/staging/cx25821/cx25821-video-upstream-ch2.c: /root/vidtest.yuv; drivers/staging/cx25821/cx25821-video-upstream-ch2.c: PIXEL_FRMT_411) ? /root/pal411.yuv : drivers/staging/cx25821/cx25821-video-upstream-ch2.c: /root/pal422.yuv; The driver should not get any file name from userspace, not should read directly to any file. Instead, it should be using V4L2 and ALSA API's for any kind of streams. Please fix it. Cheers, Mauro. Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you. ** - -- 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
FW: [cx231xx 1/2] Added support for s5h1432 demod
From 53df9742b92902b5fa9d28b2dcc949cb495725a5 Mon Sep 17 00:00:00 2001 Message-Id: 53df9742b92902b5fa9d28b2dcc949cb495725a5.1276143429.git.palash.bandyopadh...@conexant.com From: palash palash.bandyopadh...@conexant.com Date: Wed, 9 Jun 2010 21:13:17 -0700 Subject: [cx231xx 1/2] Added support for s5h1432 demod To: linux-media@vger.kernel.org Signed-off-by: palash palash.bandyopadh...@conexant.com create mode 100644 drivers/media/dvb/frontends/s5h1432.c create mode 100644 drivers/media/dvb/frontends/s5h1432.h diff --git a/drivers/media/dvb/frontends/Kconfig b/drivers/media/dvb/frontends/Kconfig index cd7f9b7..257c2fa 100644 --- a/drivers/media/dvb/frontends/Kconfig +++ b/drivers/media/dvb/frontends/Kconfig @@ -257,6 +257,13 @@ config DVB_CX22702 help A DVB-T tuner module. Say Y when you want to support this frontend. +config DVB_S5H1432 + tristate Samsung s5h1432 demodulator (OFDM) + depends on DVB_CORE I2C + default m if DVB_FE_CUSTOMISE + help + A DVB-T tuner module. Say Y when you want to support this frontend. + config DVB_DRX397XD tristate Micronas DRX3975D/DRX3977D based depends on DVB_CORE I2C diff --git a/drivers/media/dvb/frontends/Makefile b/drivers/media/dvb/frontends/Makefile index 874e8ad..faaa9aa 100644 --- a/drivers/media/dvb/frontends/Makefile +++ b/drivers/media/dvb/frontends/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_DVB_STB0899) += stb0899.o obj-$(CONFIG_DVB_STB6100) += stb6100.o obj-$(CONFIG_DVB_SP8870) += sp8870.o obj-$(CONFIG_DVB_CX22700) += cx22700.o +obj-$(CONFIG_DVB_S5H1432) += s5h1432.o obj-$(CONFIG_DVB_CX24110) += cx24110.o obj-$(CONFIG_DVB_TDA8083) += tda8083.o obj-$(CONFIG_DVB_L64781) += l64781.o diff --git a/drivers/media/dvb/frontends/s5h1432.c b/drivers/media/dvb/frontends/s5h1432.c new file mode 100644 index 000..92d134e --- /dev/null +++ b/drivers/media/dvb/frontends/s5h1432.c @@ -0,0 +1,446 @@ +/* +Samsung s5h1432 DVB-T demodulator driver + +Copyright (C) 2009 Bill Liu bill@conexant.com + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + +*/ + +#include linux/kernel.h +#include linux/init.h +#include linux/module.h +#include linux/string.h +#include linux/slab.h +#include linux/delay.h +#include dvb_frontend.h +#include s5h1432.h + +struct s5h1432_state { + + struct i2c_adapter *i2c; + + /* configuration settings */ + const struct s5h1432_config *config; + + struct dvb_frontend frontend; + + fe_modulation_t current_modulation; + unsigned int first_tune:1; + + u32 current_frequency; + int if_freq; + + u8 inversion; +}; + +static int debug; + +#define dprintk(arg...) do { \ + if (debug) \ + printk(arg);\ + } while (0) + + +static int s5h1432_writereg(struct s5h1432_state *state, + u8 addr, u8 reg, u8 data) +{ + int ret; + u8 buf[] = { reg, data }; + + struct i2c_msg msg = { .addr = addr, .flags = 0, .buf = buf, .len = 2 }; + + ret = i2c_transfer(state-i2c, msg, 1); + + if (ret != 1) + printk(KERN_ERR %s: writereg error 0x%02x 0x%02x 0x%04x, + ret == %i)\n, __func__, addr, reg, data, ret); + + return (ret != 1) ? -1 : 0; +} + +static u8 s5h1432_readreg(struct s5h1432_state *state, u8 addr, u8 reg) +{ + int ret; + u8 b0[] = { reg }; + u8 b1[] = { 0 }; + + struct i2c_msg msg[] = { + { .addr = addr, .flags = 0, .buf = b0, .len = 1 }, + { .addr = addr, .flags = I2C_M_RD, .buf = b1, .len = 1 } }; + + ret = i2c_transfer(state-i2c, msg, 2); + + if (ret != 2) + printk(KERN_ERR %s: readreg error (ret == %i)\n, + __func__, ret); + return b1[0]; +} + +static int s5h1432_sleep(struct dvb_frontend *fe) +{ + return 0; +} + +static int s5h1432_set_channel_bandwidth(struct dvb_frontend *fe, u32 bandwidth) +{ + + struct s5h1432_state *state = fe-demodulator_priv; + + u8 reg = 0; + + +/* Register[0x2E] bit 3:2 : 8MHz = 0; 7MHz = 1; 6MHz = 2*/ + reg = s5h1432_readreg(state, S5H1432_I2C_TOP_ADDR, 0x2E); + reg = ~(0x0C); + switch (bandwidth) { + case 6: + reg |= 0x08; + break; +
[cx231xx 1/2] Added support for s5h1432 demod
From 53df9742b92902b5fa9d28b2dcc949cb495725a5 Mon Sep 17 00:00:00 2001 Message-Id: 53df9742b92902b5fa9d28b2dcc949cb495725a5.1276143429.git.palash.bandyopadh...@conexant.com From: palash palash.bandyopadh...@conexant.com Date: Wed, 9 Jun 2010 21:13:17 -0700 Subject: [cx231xx 1/2] Added support for s5h1432 demod To: linux-media@vger.kernel.org Signed-off-by: palash palash.bandyopadh...@conexant.com create mode 100644 drivers/media/dvb/frontends/s5h1432.c create mode 100644 drivers/media/dvb/frontends/s5h1432.h diff --git a/drivers/media/dvb/frontends/Kconfig b/drivers/media/dvb/frontends/Kconfig index cd7f9b7..257c2fa 100644 --- a/drivers/media/dvb/frontends/Kconfig +++ b/drivers/media/dvb/frontends/Kconfig @@ -257,6 +257,13 @@ config DVB_CX22702 help A DVB-T tuner module. Say Y when you want to support this frontend. +config DVB_S5H1432 + tristate Samsung s5h1432 demodulator (OFDM) + depends on DVB_CORE I2C + default m if DVB_FE_CUSTOMISE + help + A DVB-T tuner module. Say Y when you want to support this frontend. + config DVB_DRX397XD tristate Micronas DRX3975D/DRX3977D based depends on DVB_CORE I2C diff --git a/drivers/media/dvb/frontends/Makefile b/drivers/media/dvb/frontends/Makefile index 874e8ad..faaa9aa 100644 --- a/drivers/media/dvb/frontends/Makefile +++ b/drivers/media/dvb/frontends/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_DVB_STB0899) += stb0899.o obj-$(CONFIG_DVB_STB6100) += stb6100.o obj-$(CONFIG_DVB_SP8870) += sp8870.o obj-$(CONFIG_DVB_CX22700) += cx22700.o +obj-$(CONFIG_DVB_S5H1432) += s5h1432.o obj-$(CONFIG_DVB_CX24110) += cx24110.o obj-$(CONFIG_DVB_TDA8083) += tda8083.o obj-$(CONFIG_DVB_L64781) += l64781.o diff --git a/drivers/media/dvb/frontends/s5h1432.c b/drivers/media/dvb/frontends/s5h1432.c new file mode 100644 index 000..92d134e --- /dev/null +++ b/drivers/media/dvb/frontends/s5h1432.c @@ -0,0 +1,446 @@ +/* +Samsung s5h1432 DVB-T demodulator driver + +Copyright (C) 2009 Bill Liu bill@conexant.com + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + +*/ + +#include linux/kernel.h +#include linux/init.h +#include linux/module.h +#include linux/string.h +#include linux/slab.h +#include linux/delay.h +#include dvb_frontend.h +#include s5h1432.h + +struct s5h1432_state { + + struct i2c_adapter *i2c; + + /* configuration settings */ + const struct s5h1432_config *config; + + struct dvb_frontend frontend; + + fe_modulation_t current_modulation; + unsigned int first_tune:1; + + u32 current_frequency; + int if_freq; + + u8 inversion; +}; + +static int debug; + +#define dprintk(arg...) do { \ + if (debug) \ + printk(arg);\ + } while (0) + + +static int s5h1432_writereg(struct s5h1432_state *state, + u8 addr, u8 reg, u8 data) +{ + int ret; + u8 buf[] = { reg, data }; + + struct i2c_msg msg = { .addr = addr, .flags = 0, .buf = buf, .len = 2 }; + + ret = i2c_transfer(state-i2c, msg, 1); + + if (ret != 1) + printk(KERN_ERR %s: writereg error 0x%02x 0x%02x 0x%04x, + ret == %i)\n, __func__, addr, reg, data, ret); + + return (ret != 1) ? -1 : 0; +} + +static u8 s5h1432_readreg(struct s5h1432_state *state, u8 addr, u8 reg) +{ + int ret; + u8 b0[] = { reg }; + u8 b1[] = { 0 }; + + struct i2c_msg msg[] = { + { .addr = addr, .flags = 0, .buf = b0, .len = 1 }, + { .addr = addr, .flags = I2C_M_RD, .buf = b1, .len = 1 } }; + + ret = i2c_transfer(state-i2c, msg, 2); + + if (ret != 2) + printk(KERN_ERR %s: readreg error (ret == %i)\n, + __func__, ret); + return b1[0]; +} + +static int s5h1432_sleep(struct dvb_frontend *fe) +{ + return 0; +} + +static int s5h1432_set_channel_bandwidth(struct dvb_frontend *fe, u32 bandwidth) +{ + + struct s5h1432_state *state = fe-demodulator_priv; + + u8 reg = 0; + + +/* Register[0x2E] bit 3:2 : 8MHz = 0; 7MHz = 1; 6MHz = 2*/ + reg = s5h1432_readreg(state, S5H1432_I2C_TOP_ADDR, 0x2E); + reg = ~(0x0C); + switch (bandwidth) { + case 6: + reg |= 0x08; + break; +
RE: -next: staging/cx25821: please revert 7a02f549fcc
Will do. Thanks for passing over the info. Regards, Palash -Original Message- From: Mauro Carvalho Chehab [mailto:mche...@redhat.com] Sent: Wednesday, May 05, 2010 8:30 AM To: Dan Carpenter; Palash Bandyopadhyay Cc: Greg Kroah-Hartman; linux-media@vger.kernel.org Subject: Re: -next: staging/cx25821: please revert 7a02f549fcc Dan Carpenter wrote: This was my patch: cx25821: fix double unlock in medusa_video_init() It accidentally got merged two times. The version from the staging tree is not correct. Please can you revert it: 7a02f549fcc30fe6be0c0024beae9a3db22e1af6 Staging: cx25821: fix double unlock in medusa_video_init() Hmm... true. I guess this is going through the V4L/DVB so it needs to be reverted there as well as in the -staging tree. There's no need to touch at staging tree, since this change come from v4l-dvb tree. After reviewing the logic at the function, instead of just adding a patch to revert the wrong one, the better is to apply a different logic: add a goto that will always unlock and return the error. This simplifies the code a little bit, and, instead of just return -EINVAL, it will return the error condition reported by the called functions. Btw, cx25821-core currently doesn't handle the error returned by medusa_video_init(). Palash, Could you please add the proper validation code for the error conditions that may happen during device init? Cheers, Mauro -- V4L/DVB: cx25821: fix error condition logic at medusa_video_init() Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/staging/cx25821/cx25821-medusa-video.c b/drivers/staging/cx25821/cx25821-medusa-video.c index 77ccef4..7545314 100644 --- a/drivers/staging/cx25821/cx25821-medusa-video.c +++ b/drivers/staging/cx25821/cx25821-medusa-video.c @@ -778,9 +778,9 @@ int medusa_set_saturation(struct cx25821_dev *dev, int saturation, int decoder) int medusa_video_init(struct cx25821_dev *dev) { - u32 value = 0, tmp = 0; - int ret_val = 0; - int i = 0; + u32 value, tmp = 0; + int ret_val; + int i; mutex_lock(dev-lock); @@ -790,20 +790,15 @@ int medusa_video_init(struct cx25821_dev *dev) value = cx25821_i2c_read(dev-i2c_bus[0], MON_A_CTRL, tmp); value = 0xF0FF; ret_val = cx25821_i2c_write(dev-i2c_bus[0], MON_A_CTRL, value); + if (ret_val 0) + goto error; - if (ret_val 0) { - mutex_unlock(dev-lock); - return -EINVAL; - } /* Turn off Master source switch enable */ value = cx25821_i2c_read(dev-i2c_bus[0], MON_A_CTRL, tmp); value = 0xFFDF; ret_val = cx25821_i2c_write(dev-i2c_bus[0], MON_A_CTRL, value); - - if (ret_val 0) { - mutex_unlock(dev-lock); - return -EINVAL; - } + if (ret_val 0) + goto error; mutex_unlock(dev-lock); @@ -817,31 +812,25 @@ int medusa_video_init(struct cx25821_dev *dev) value = 0xFF70FF70; value |= 0x00090008;/* set en_active */ ret_val = cx25821_i2c_write(dev-i2c_bus[0], DENC_AB_CTRL, value); + if (ret_val 0) + goto error; - if (ret_val 0) { - mutex_unlock(dev-lock); - return -EINVAL; - } /* enable input is VIP/656 */ value = cx25821_i2c_read(dev-i2c_bus[0], BYP_AB_CTRL, tmp); value |= 0x00040100;/* enable VIP */ ret_val = cx25821_i2c_write(dev-i2c_bus[0], BYP_AB_CTRL, value); - if (ret_val 0) { - mutex_unlock(dev-lock); - return -EINVAL; - } + if (ret_val 0) + goto error; + /* select AFE clock to output mode */ value = cx25821_i2c_read(dev-i2c_bus[0], AFE_AB_DIAG_CTRL, tmp); value = 0x83FF; - ret_val = - cx25821_i2c_write(dev-i2c_bus[0], AFE_AB_DIAG_CTRL, - value | 0x1000); + ret_val = cx25821_i2c_write(dev-i2c_bus[0], AFE_AB_DIAG_CTRL, + value | 0x1000); + if (ret_val 0) + goto error; - if (ret_val 0) { - mutex_unlock(dev-lock); - return -EINVAL; - } /* Turn on all of the data out and control output pins. */ value = cx25821_i2c_read(dev-i2c_bus[0], PIN_OE_CTRL, tmp); value = 0xFEF0FE00; @@ -868,9 +857,9 @@ int medusa_video_init(struct cx25821_dev *dev) mutex_unlock(dev-lock); ret_val = medusa_set_videostandard(dev); + return ret_val; - if (ret_val 0) - return -EINVAL; - - return 1; +error: + mutex_unlock(dev-lock); + return ret_val; } Conexant E-mail Firewall (Conexant.Com) made the following annotations - ** Legal Disclaimer