Re: DViCo Dual Fusion Express (cx23885) remote control issue
On 23/04/2010, at 17:22, Timothy D. Lenz wrote: >> I haven't found any problems with tuners not working although I don't >> often fire them both up at once. >> > > [PATCH] FusionHDTV: Use quick reads for I2C IR device probing I finally found some time to look at this and found.. https://patchwork.kernel.org/patch/86939/ However I don't see anything in /sys/bus/i2c/devices so I presume it's another issue :( -- Daniel O'Connor software and network engineer for Genesis Software - http://www.gsoft.com.au "The nice thing about standards is that there are so many of them to choose from." -- Andrew Tanenbaum GPG Fingerprint - 5596 B766 97C0 0E94 4347 295E E593 DC20 7B3F CE8C -- 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: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
Jarod Wilson wrote: On Fri, May 21, 2010 at 11:40:34AM +0800, Ang Way Chuang wrote: Hi Jarod, Thanks for the review. My answers are inlined. Jarod Wilson wrote: On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote: ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation code has a bug that incorrectly treats ULE SNDU packed into the remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer field on the subsequent MPEG2-TS frame. This patch was generated and tested against v2.6.34-rc6. I suspect that this bug was introduced in kernel version 2.6.15, but had not verified it. Care has been taken not to introduce more bug by fixing this bug, but please scrutinize the code because I always produces buggy code. ... @@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len ) from_where += 2; } + priv->ule_sndu_remain = priv->ule_sndu_len + 2; /* * State of current TS: * ts_remain (remaining bytes in the current TS cell) Is this *always* true? Your description says "...the remaining 2 or 3 bytes", indicating this could sometimes need to be +3. Is +0 also a possibility? Not sure whether I understand your question correctly. Here is my attempt to answer your question. The encapsulation format always mandate that at least of 2 bytes of ULE SNDU (the LENGTH field) must be present within a MPEG2-TS frame. So, if only 1 byte of the ULE SNDU get packed into the remaining MPEG2-TS frame, then it is invalid. Of course, there is no issue regarding 0 byte as that would be the case of filling MPEG2-TS frame up to its boundary. New ULE SNDU will have to packed into the next MPEG2-TS frame in that case. Now the problem with existing code is the interpretation of remainder length when 2 or 3 bytes of ULE SNDU are packed into the remainder of MPEG2-TS frame. In the 2 bytes case, only the LENGTH field is available while in the case 3 bytes, only the 1st octet of the 2-octets TYPE field and the LENGTH field are available. The ule_sndu_remain should carry the value of length of ULE SNDU following the the TYPE field. Technically, this would mean that remainder byte of ULE SNDU that need to be received is going to be: Value(LENGTH) + 2 (We owe 2 bytes of TYPE field here) if only 2 bytes of ULE SNDU is packed (as in the case of case 0: at line 550). This is addressed by adding the priv->ule_sndu_remain = priv->ule_sndu_len + 2; Value(LENGTH) + 1 (We owe 1 byte of TYPE field here) if 3 bytes of ULE SNDU is packed (as in the case of case 1: at 545). This is addressed by adding priv->ule_sndu_remain--; If complete ULE header (>= 4 bytes) is available: priv->ule_sndu_remain = priv->ule_sndu_len; at line 582 takes care of the rest and it works just fine in the existing code. Due to the wrong interpretation of remaining length of ULE SNDU when 2 or 3 bytes of ULE SNDU are packed into a MPEG2-TS frame, the subsequent checking of payload pointer (line 455) always fails leading to unnecessary packet drops. Looking back at the fix after a few months, I had trouble understanding how these few lines fixed the problem at first glance. Yeah, my question was whether or not the +2 would account for both the +2 bytes and +3 bytes situations, and it seems that's handled appropriately by the ts_remain switch. Thank you for the detailed explanation. If you'd alter that nested check for freeing the skb and give it a quick test, I'm happy to throw an acked-by or reviewed-by on a followup submission. Got it. Thank you. I shall get that patch to you next week because I'm not in the lab now. -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: [snip] > > The point when the kernel started complaining about the use of a stack > > based USB I/O buffers is the relevant point, which was not back in > > 2.6.12. I learned of this behavior (that is, receiving warnings about > > the usage) as being new in the 2.6.34 timeframe, the point when a user > > pointed out the complaint message in his kernel log; at that time I had > > not yet tested against that kernel version. > > Older kernels also complain if the stack were actually out of the > DMA range and you try to program DMA there. Yet, only now we've seen > consumer PC's with lots of RAM. One of my test targets is a PC in 32 bit mode with 4GB of RAM; strange then that I've never seen the kernel complain there. The bad buffer has been in the driver for even longer than that and nobody raised the issue to me until about a month ago (the fix was created back then but for other reasons that you already know it didn't become available in -hg until last week). > > > Leave it as is. What's done is done. Your replaced comment is of > > course still correct. I would just appreciate some better sensitivity > > in the future about replacing authors' comments, especially since in > > this case your interpretation of my original comment was wrong. > > Ok. Thanks. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > >> Mike Isely wrote: >>> On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: >>> Mike Isely wrote: > Mauro: > > You are reading too much into that comment. > > I never said it was valid to do what had been done, only that for the > longest time this is what the driver did and it never caused a problem > that I was made aware of. What I said there was correct, that this is > what the driver had been doing in the past, that it's definitely causing > a problem now and thus that is why this patch exists. As I said, this is not right: "Apparently later kernels don't like this behavior" >>> Mauro: >>> >>> That statement was in reference to the fact that previously the problem >>> had gone undetected, but now later kernels can notice and complain about >>> this, thus "later kernels don't like this behavior". >>> >>> We can debate that perhaps the statement can be worded better, but that >>> doesn't make it *wrong*. >>> >> Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was >> about the kernel were em28xx driver were introduced). > > The point when the em28xx driver appeared has nothing to do with this. > > The point when the kernel started complaining about the use of a stack > based USB I/O buffers is the relevant point, which was not back in > 2.6.12. I learned of this behavior (that is, receiving warnings about > the usage) as being new in the 2.6.34 timeframe, the point when a user > pointed out the complaint message in his kernel log; at that time I had > not yet tested against that kernel version. Older kernels also complain if the stack were actually out of the DMA range and you try to program DMA there. Yet, only now we've seen consumer PC's with lots of RAM. > Leave it as is. What's done is done. Your replaced comment is of > course still correct. I would just appreciate some better sensitivity > in the future about replacing authors' comments, especially since in > this case your interpretation of my original comment was wrong. Ok. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > > > >> Mike Isely wrote: > >>> Mauro: > >>> > >>> You are reading too much into that comment. > >>> > >>> I never said it was valid to do what had been done, only that for the > >>> longest time this is what the driver did and it never caused a problem > >>> that I was made aware of. What I said there was correct, that this is > >>> what the driver had been doing in the past, that it's definitely causing > >>> a problem now and thus that is why this patch exists. > >> As I said, this is not right: > >>"Apparently later kernels don't like this behavior" > > > > Mauro: > > > > That statement was in reference to the fact that previously the problem > > had gone undetected, but now later kernels can notice and complain about > > this, thus "later kernels don't like this behavior". > > > > We can debate that perhaps the statement can be worded better, but that > > doesn't make it *wrong*. > > > > Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was > about the kernel were em28xx driver were introduced). The point when the em28xx driver appeared has nothing to do with this. The point when the kernel started complaining about the use of a stack based USB I/O buffers is the relevant point, which was not back in 2.6.12. I learned of this behavior (that is, receiving warnings about the usage) as being new in the 2.6.34 timeframe, the point when a user pointed out the complaint message in his kernel log; at that time I had not yet tested against that kernel version. > > > > >> It is not "later kernels". DMA over stack were never supported. Your driver > >> had a bug that you didn't noticed for long time, probably because nobody > >> reported you this issue, since it appears only on some non-Intel archs and > >> on i386 with more than 3.12 Gb of RAM, and when the stack happens to be > >> after > >> the first 3.12 Gb (with is a somewhat rare condition). > > > > I understand your point perfectly that this was never right or valid. > > In fact, I also understood that point long before you decided to explain > > it to me here - after all my realization of this problem in the driver > > is why I wrote the patch in the first place. Absolutely no argument > > there about the importance of the change. > > > > None of that however justifies putting words into an author's mouth, > > which is effectively what you did by replacing that commit comment. > > > > First of all, it is clearly stated at the patch that the description > were changed and by whom: > [mche...@redhat.com: fix patch description] > > Second, it is an usual upstream practice to fix descriptions where needed. > The patch description is a precious resource for people that are seeking > for similar bugs, and for those that are trying to understand some code. > So, it is important to not send broken/incomplete comments to kernel, > or comments that may have a dubious interpretation. So, subsystem maintainers > frequently need to fix comments. > > Anyway, to end this discussion, I can simply revert the patch from the > staging tree, waiting for a new patch from you with a fixed comment. > > Also, if you prefer, next time, I won't apply any patch from you if I > found a bad comment without your ack, even if it means that you'll > probably loose a merge window. Leave it as is. What's done is done. Your replaced comment is of course still correct. I would just appreciate some better sensitivity in the future about replacing authors' comments, especially since in this case your interpretation of my original comment was wrong. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > >> Mike Isely wrote: >>> Mauro: >>> >>> You are reading too much into that comment. >>> >>> I never said it was valid to do what had been done, only that for the >>> longest time this is what the driver did and it never caused a problem >>> that I was made aware of. What I said there was correct, that this is >>> what the driver had been doing in the past, that it's definitely causing >>> a problem now and thus that is why this patch exists. >> As I said, this is not right: >> "Apparently later kernels don't like this behavior" > > Mauro: > > That statement was in reference to the fact that previously the problem > had gone undetected, but now later kernels can notice and complain about > this, thus "later kernels don't like this behavior". > > We can debate that perhaps the statement can be worded better, but that > doesn't make it *wrong*. > Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was about the kernel were em28xx driver were introduced). > >> It is not "later kernels". DMA over stack were never supported. Your driver >> had a bug that you didn't noticed for long time, probably because nobody >> reported you this issue, since it appears only on some non-Intel archs and >> on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after >> the first 3.12 Gb (with is a somewhat rare condition). > > I understand your point perfectly that this was never right or valid. > In fact, I also understood that point long before you decided to explain > it to me here - after all my realization of this problem in the driver > is why I wrote the patch in the first place. Absolutely no argument > there about the importance of the change. > > None of that however justifies putting words into an author's mouth, > which is effectively what you did by replacing that commit comment. > First of all, it is clearly stated at the patch that the description were changed and by whom: [mche...@redhat.com: fix patch description] Second, it is an usual upstream practice to fix descriptions where needed. The patch description is a precious resource for people that are seeking for similar bugs, and for those that are trying to understand some code. So, it is important to not send broken/incomplete comments to kernel, or comments that may have a dubious interpretation. So, subsystem maintainers frequently need to fix comments. Anyway, to end this discussion, I can simply revert the patch from the staging tree, waiting for a new patch from you with a fixed comment. Also, if you prefer, next time, I won't apply any patch from you if I found a bad comment without your ack, even if it means that you'll probably loose a merge window. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > Mauro: > > > > You are reading too much into that comment. > > > > I never said it was valid to do what had been done, only that for the > > longest time this is what the driver did and it never caused a problem > > that I was made aware of. What I said there was correct, that this is > > what the driver had been doing in the past, that it's definitely causing > > a problem now and thus that is why this patch exists. > > As I said, this is not right: > "Apparently later kernels don't like this behavior" Mauro: That statement was in reference to the fact that previously the problem had gone undetected, but now later kernels can notice and complain about this, thus "later kernels don't like this behavior". We can debate that perhaps the statement can be worded better, but that doesn't make it *wrong*. > > It is not "later kernels". DMA over stack were never supported. Your driver > had a bug that you didn't noticed for long time, probably because nobody > reported you this issue, since it appears only on some non-Intel archs and > on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after > the first 3.12 Gb (with is a somewhat rare condition). I understand your point perfectly that this was never right or valid. In fact, I also understood that point long before you decided to explain it to me here - after all my realization of this problem in the driver is why I wrote the patch in the first place. Absolutely no argument there about the importance of the change. None of that however justifies putting words into an author's mouth, which is effectively what you did by replacing that commit comment. When I've propagated commits from other authors who have sent me patches, I try very hard to ensure that their comments are preserved verbatim. Those are after all the author's words not mine. If I think such a comment has a problem, then I will discuss it with the author and let him/her provide the replacement statement (or at least explain to me why (s)he thinks it is correct). Failing that then I will at least make it clear in the comment which edits came from me. I would never want to be accused of unilaterally putting words into peoples' mouths and I would hope others will treat me the same way. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote:
> Mauro:
>
> You are reading too much into that comment.
>
> I never said it was valid to do what had been done, only that for the
> longest time this is what the driver did and it never caused a problem
> that I was made aware of. What I said there was correct, that this is
> what the driver had been doing in the past, that it's definitely causing
> a problem now and thus that is why this patch exists.
As I said, this is not right:
"Apparently later kernels don't like this behavior"
It is not "later kernels". DMA over stack were never supported. Your driver
had a bug that you didn't noticed for long time, probably because nobody
reported you this issue, since it appears only on some non-Intel archs and
on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after
the first 3.12 Gb (with is a somewhat rare condition).
If you take a look at the old logs, you'll see, at commit
a6c2ba283565dbc9f055dcb2ecba1971460bb535 (nov, 2005) the fix on one of the
drivers:
+int em2820_write_regs_req(struct em2820 *dev, u8 req, u16 reg, char *buf,
+ int len)
+{
+ int ret;
+
+ /*usb_control_msg seems to expect a kmalloced buffer */
+ unsigned char *bufs = kmalloc(len, GFP_KERNEL);
The same bug hit me in 2006/2007 when writing tm6000 driver. Due to a problem
at the videobuf free logic, on that time, it were very frequent to hit this
bug, as, with memory being spent, eventually stack address would be above the
3Gb address.
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mauro: You are reading too much into that comment. I never said it was valid to do what had been done, only that for the longest time this is what the driver did and it never caused a problem that I was made aware of. What I said there was correct, that this is what the driver had been doing in the past, that it's definitely causing a problem now and thus that is why this patch exists. I'd really rather you not mess with my comment. Probably too late however. -Mike On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the > > following pvrusb2 driver fixes / improvements: > > > > - pvrusb2: Minor debug code fixup > > - pvrusb2: Fix Gotview hardware support > > - pvrusb2: Avoid using stack allocated buffers when performing USB I/O > > Your comment for this patch is wrong: > > pvrusb2: The pvrusb2 driver has for the longest time used a (tiny) > stack allocated buffer for some of its I/O with the hardware. > Apparently later kernels don't like this behavior and trap it at > run-time, causing nasty complaints to the kernel log. This trivial > change fixes the one case in the driver where this had been happening. > > It were never valid to use stack for DMA, as kernel provides > no warranty that the stack would be on a page that can do DMA. > In a matter of fact, as most x86 USB drivers accept DMA at the first > 3Gb of RAM space, this bug is generally not noticed on i386/x86_64 > archs. Yet, if your machine has more than 3Gb, there are some chances that > the stack would be at the HIGHMEM area, where DMA is not supported by the > processor. > > As this is a common error, newer kernels have some instrumentation support to > warn about such troubles. > > I'll be fixing the comment. > > > -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
Warning when loading technisat driver (2.6.34)
I've just upgraded my media box, and get the following warning on startup. All seems well otherwise, so this is a FYI Cheers May 21 19:05:54 server kernel: [7.950291] [ cut here ] May 21 19:05:54 server kernel: [7.950296] WARNING: at fs/proc/generic.c:317 __xlate_proc_name+0xa5/0xd0() May 21 19:05:54 server kernel: [7.950298] Hardware name: System Product Name May 21 19:05:54 server kernel: [7.950299] name 'Technisat/B2C2 FlexCop II/IIb/III Digital TV PCI Driver' May 21 19:05:54 server kernel: [7.950300] Modules linked in: b2c2_flexcop_pci(+) snd_hda_codec dvb_usb snd_pcm pl2303 ppdev crc_ccitt tulip b2c2_flexcop snd_timer snd soundcore r8169 ohci1394 parport_pc dvb_core shpchp mii i2c_piix4 ieee1394 parport cx24123 cx24113 s5h1420 k10temp snd_page_alloc pcspkr isdn May 21 19:05:54 server kernel: [7.950311] Pid: 942, comm: modprobe Not tainted 2.6.34 #1 May 21 19:05:54 server kernel: [7.950312] Call Trace: May 21 19:05:54 server kernel: [7.950316] [] ? __xlate_proc_name+0xa5/0xd0 May 21 19:05:54 server kernel: [7.950319] [] warn_slowpath_common+0x78/0xd0 May 21 19:05:54 server kernel: [7.950322] [] warn_slowpath_fmt+0x64/0x70 May 21 19:05:54 server kernel: [7.950325] [] ? snprintf+0x59/0x60 May 21 19:05:54 server kernel: [7.950327] [] __xlate_proc_name+0xa5/0xd0 May 21 19:05:54 server kernel: [7.950329] [] __proc_create+0x74/0x150 May 21 19:05:54 server kernel: [7.950331] [] proc_mkdir_mode+0x29/0x60 May 21 19:05:54 server kernel: [7.950333] [] proc_mkdir+0x11/0x20 May 21 19:05:54 server kernel: [7.950336] [] register_handler_proc+0xf4/0x120 May 21 19:05:54 server kernel: [7.950338] [] __setup_irq+0x1d9/0x350 May 21 19:05:54 server kernel: [7.950341] [] ? flexcop_pci_isr+0x0/0x190 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950343] [] request_threaded_irq+0x18b/0x210 May 21 19:05:54 server kernel: [7.950347] [] flexcop_pci_probe+0x18e/0x360 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950349] [] local_pci_probe+0x12/0x20 May 21 19:05:54 server kernel: [7.950351] [] pci_device_probe+0x80/0xa0 May 21 19:05:54 server kernel: [7.950354] [] ? driver_sysfs_add+0x63/0x90 May 21 19:05:54 server kernel: [7.950356] [] driver_probe_device+0x92/0x1a0 May 21 19:05:54 server kernel: [7.950359] [] __driver_attach+0x93/0xa0 May 21 19:05:54 server kernel: [7.950361] [] ? __driver_attach+0x0/0xa0 May 21 19:05:54 server kernel: [7.950363] [] bus_for_each_dev+0x63/0x90 May 21 19:05:54 server kernel: [7.950365] [] driver_attach+0x1c/0x20 May 21 19:05:54 server kernel: [7.950367] [] bus_add_driver+0x1a5/0x2e0 May 21 19:05:54 server kernel: [7.950369] [] ? flexcop_pci_module_init+0x0/0x20 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950371] [] driver_register+0x79/0x160 May 21 19:05:54 server kernel: [7.950374] [] ? flexcop_pci_module_init+0x0/0x20 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950376] [] __pci_register_driver+0x5a/0xe0 May 21 19:05:54 server kernel: [7.950378] [] ? flexcop_pci_module_init+0x0/0x20 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950381] [] flexcop_pci_module_init+0x1e/0x20 [b2c2_flexcop_pci] May 21 19:05:54 server kernel: [7.950383] [] do_one_initcall+0x38/0x190 May 21 19:05:54 server kernel: [7.950386] [] sys_init_module+0xdd/0x260 May 21 19:05:54 server kernel: [7.950388] [] system_call_fastpath+0x16/0x1b May 21 19:05:54 server kernel: [7.950389] ---[ end trace da2237741ef2a8c0 ]--- -- 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/3] Driver for the i.MX2x CMOS Sensor Interface
On Fri, 21 May 2010, Baruch Siach wrote: > Hi Sascha, > > On Fri, May 21, 2010 at 09:20:45AM +0200, Sascha Hauer wrote: > > On Thu, May 06, 2010 at 04:09:38PM +0300, Baruch Siach wrote: > > > This series contains a soc_camera driver for the i.MX25/i.MX27 CSI > > > device, and > > > platform code for the i.MX25 and i.MX27 chips. This driver is based on a > > > driver > > > for i.MX27 CSI from Sascha Hauer, that Alan Carvalho de Assis has posted > > > in > > > linux-media last December[1]. Since all I have is a i.MX25 PDK paltform I > > > can't > > > test the mx27 specific code. Testers and comment are welcome. > > > > > > [1] https://patchwork.kernel.org/patch/67636/ > > > > > > Baruch Siach (3): > > > mx2_camera: Add soc_camera support for i.MX25/i.MX27 > > > mx27: add support for the CSI device > > > mx25: add support for the CSI device > > > > With the two additions I sent I can confirm this working on i.MX27, so > > no need to remove the related code. > > Thanks. I'll add your patches to my queue and resend the series next week. Firstly, Sascha, unfortunately, you've forgotten to CC the maintainer, that will have to deal with these patches. Secondly, I don't think that's a good idea to submit mx27 fixes as incremental patches. I'd prefer to have them rolled into the actual driver submission patches, where Sascha would just add his Sob / acked-by / tested-by / whatever... Or you can first submit an mx25-only driver and let Sascha add mx27 to it, in which case this would be a functionality extension, but not a fix of a broken driver. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
On Fri, May 21, 2010 at 11:40:34AM +0800, Ang Way Chuang wrote: > Hi Jarod, >Thanks for the review. My answers are inlined. > > Jarod Wilson wrote: > >On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote: > >>ULE (Unidirectional Lightweight Encapsulation RFC 4326) > >>decapsulation code has a bug that incorrectly treats ULE SNDU > >>packed into the remaining 2 or 3 bytes of a MPEG2-TS frame as > >>having invalid pointer field on the subsequent MPEG2-TS frame. > >> > >>This patch was generated and tested against v2.6.34-rc6. I > >>suspect that this bug was introduced in kernel version 2.6.15, > >>but had not verified it. > >> > >>Care has been taken not to introduce more bug by fixing this bug, but > >>please scrutinize the code because I always produces buggy code. ... > >>@@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const > >>u8 *buf, size_t buf_len ) > >>from_where += 2; > >>} > >> > >>+ priv->ule_sndu_remain = priv->ule_sndu_len + 2; > >>/* > >> * State of current TS: > >> * ts_remain (remaining bytes in the current TS cell) > > > >Is this *always* true? Your description says "...the remaining 2 or 3 > >bytes", indicating this could sometimes need to be +3. Is +0 also a > >possibility? > > > > Not sure whether I understand your question correctly. Here is my > attempt to answer your question. The encapsulation format always > mandate that at least of 2 bytes of ULE SNDU (the LENGTH field) must > be present within a MPEG2-TS frame. So, if only 1 byte of the ULE > SNDU get packed into the remaining MPEG2-TS frame, then it is > invalid. Of course, there is no issue regarding 0 byte as that would > be the case of filling MPEG2-TS frame up to its boundary. New ULE > SNDU will have to packed into the next MPEG2-TS frame in that case. > > Now the problem with existing code is the interpretation of > remainder length when 2 or 3 bytes of ULE SNDU are packed into the > remainder of MPEG2-TS frame. In the 2 bytes case, only the LENGTH > field is available while in the case 3 bytes, only the 1st octet of > the 2-octets TYPE field and the LENGTH field are available. The > ule_sndu_remain should carry the value of length of ULE SNDU > following the the TYPE field. Technically, this would mean that > remainder byte of ULE SNDU that need to be received is going to be: > > Value(LENGTH) + 2 (We owe 2 bytes of TYPE field here) if only 2 > bytes of ULE SNDU is packed (as in the case of case 0: at line 550). > This is addressed by adding the priv->ule_sndu_remain = > priv->ule_sndu_len + 2; > > Value(LENGTH) + 1 (We owe 1 byte of TYPE field here) if 3 bytes of > ULE SNDU is packed (as in the case of case 1: at 545). This is > addressed by adding priv->ule_sndu_remain--; > > If complete ULE header (>= 4 bytes) is available: > priv->ule_sndu_remain = priv->ule_sndu_len; at line 582 takes care > of the rest and it works just fine in the existing code. > > Due to the wrong interpretation of remaining length of ULE SNDU when > 2 or 3 bytes of ULE SNDU are packed into a MPEG2-TS frame, the > subsequent checking of payload pointer (line 455) always fails > leading to unnecessary packet drops. > > Looking back at the fix after a few months, I had trouble > understanding how these few lines fixed the problem at first glance. Yeah, my question was whether or not the +2 would account for both the +2 bytes and +3 bytes situations, and it seems that's handled appropriately by the ts_remain switch. Thank you for the detailed explanation. If you'd alter that nested check for freeing the skb and give it a quick test, I'm happy to throw an acked-by or reviewed-by on a followup submission. -- Jarod Wilson ja...@redhat.com -- 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: [PULL] http://linuxtv.org/hg/~dougsland/v4l-dvb
Douglas Schilling Landgraf wrote: > Hello, > > On Fri, May 14, 2010 at 12:54 AM, Douglas Schilling Landgraf > wrote: >> Hello Mauro, >> >> Please pull from http://linuxtv.org/hg/~dougsland/v4l-dvb for >> the following: >> >> - ir-core-priv.h: error: field 'rx_work' has incomplete type > > Added: > >- cx231xx.h: Add include Douglas, I'm noticing no compilation errors upstream. So, this is probably a backport-only ussue. Please merge the upstream diffs at hg, since there were several changes there that should be applied at the backport tree. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the > following pvrusb2 driver fixes / improvements: > > - pvrusb2: Minor debug code fixup > - pvrusb2: Fix Gotview hardware support > - pvrusb2: Avoid using stack allocated buffers when performing USB I/O Your comment for this patch is wrong: pvrusb2: The pvrusb2 driver has for the longest time used a (tiny) stack allocated buffer for some of its I/O with the hardware. Apparently later kernels don't like this behavior and trap it at run-time, causing nasty complaints to the kernel log. This trivial change fixes the one case in the driver where this had been happening. It were never valid to use stack for DMA, as kernel provides no warranty that the stack would be on a page that can do DMA. In a matter of fact, as most x86 USB drivers accept DMA at the first 3Gb of RAM space, this bug is generally not noticed on i386/x86_64 archs. Yet, if your machine has more than 3Gb, there are some chances that the stack would be at the HIGHMEM area, where DMA is not supported by the processor. As this is a common error, newer kernels have some instrumentation support to warn about such troubles. I'll be fixing the comment. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New procedures for git - was Re: [GIT PATCHES FOR 2.6.35] Convert bw-qcam and c-qcam to V4L2
Laurent Pinchart wrote: > Hi Mauro, > > On Friday 21 May 2010 17:03:04 Mauro Carvalho Chehab wrote: >> Laurent Pinchart wrote: >>> On Friday 21 May 2010 16:05:29 Mauro Carvalho Chehab wrote: Hans Verkuil wrote: > I had hoped to get hardware to test it with, but no such luck (yet). > But there seems to be no sense in holding these patches back on the > off-chance that I can get hardware. > > Gerard, you mentioned earlier that you actually had a c-qcam. It would > be nice if you can test this. Pulled, thanks. I'm now trying a new way of merging patches: I'm creating some topic branches, as /staging/. This is the first merge of the new procedure ;) So, basically, those patches went to /staging/v4l1. I'll periodically merge the staging patches at devel/for_v42.6.35 branches, for patches that I'll send for the current merge window, and at devel/for_v2.6.36, for the patches to the next merge window. >>> When will they be merged to master ? >> There's a very good question. The current master branch has a very dirty >> history, with almost all 2.6.33 and 2.6.34 patches merged twice. I'm not >> sure yet what would be the better way to handle the master branch. >> To avoid make things worse, I'm considering to merge back at master only >> after having the patches merged upstream. > > Isn't that even worse than what we did for 2.6.34 ? How are developers > supposed to test their drivers against changes in the v4l-dvb repository ? By > merging all staging branches manually every time ? You can merge from devel/for_v42.6.35, where all the staging repositories will be merged. The main difference is that I won't merge the "temporary" hashes at master (as patches might be rebased when sending upstream, generating a new hash). I'm considering also to have a v2.6.34+devel branch with 2.6.34 + all staging branches, to help people to test the drivers using git. > This new procedure is still experimental, but I think it will be better than the previous one. > -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New procedures for git - was Re: [GIT PATCHES FOR 2.6.35] Convert bw-qcam and c-qcam to V4L2
Hi Mauro, On Friday 21 May 2010 17:03:04 Mauro Carvalho Chehab wrote: > Laurent Pinchart wrote: > > On Friday 21 May 2010 16:05:29 Mauro Carvalho Chehab wrote: > >> Hans Verkuil wrote: > >>> I had hoped to get hardware to test it with, but no such luck (yet). > >>> But there seems to be no sense in holding these patches back on the > >>> off-chance that I can get hardware. > >>> > >>> Gerard, you mentioned earlier that you actually had a c-qcam. It would > >>> be nice if you can test this. > >> > >> Pulled, thanks. > >> > >> I'm now trying a new way of merging patches: I'm creating some topic > >> branches, as /staging/. This is the first merge of the new > >> procedure ;) > >> > >> So, basically, those patches went to /staging/v4l1. > >> > >> I'll periodically merge the staging patches at devel/for_v42.6.35 > >> branches, for patches that I'll send for the current merge window, and > >> at devel/for_v2.6.36, for the patches to the next merge window. > > > > When will they be merged to master ? > > There's a very good question. The current master branch has a very dirty > history, with almost all 2.6.33 and 2.6.34 patches merged twice. I'm not > sure yet what would be the better way to handle the master branch. > To avoid make things worse, I'm considering to merge back at master only > after having the patches merged upstream. Isn't that even worse than what we did for 2.6.34 ? How are developers supposed to test their drivers against changes in the v4l-dvb repository ? By merging all staging branches manually every time ? > >> This new procedure is still experimental, but I think it will be better > >> than the previous one. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New procedures for git - was Re: [GIT PATCHES FOR 2.6.35] Convert bw-qcam and c-qcam to V4L2
Laurent Pinchart wrote: > Hi Mauro, > > On Friday 21 May 2010 16:05:29 Mauro Carvalho Chehab wrote: >> Hans Verkuil wrote: >>> I had hoped to get hardware to test it with, but no such luck (yet). But >>> there seems to be no sense in holding these patches back on the >>> off-chance that I can get hardware. >>> >>> Gerard, you mentioned earlier that you actually had a c-qcam. It would be >>> nice if you can test this. >> Pulled, thanks. >> >> I'm now trying a new way of merging patches: I'm creating some topic >> branches, as /staging/. This is the first merge of the new procedure >> ;) >> >> So, basically, those patches went to /staging/v4l1. >> >> I'll periodically merge the staging patches at devel/for_v42.6.35 branches, >> for patches that I'll send for the current merge window, and at >> devel/for_v2.6.36, for the patches to the next merge window. > > When will they be merged to master ? There's a very good question. The current master branch has a very dirty history, with almost all 2.6.33 and 2.6.34 patches merged twice. I'm not sure yet what would be the better way to handle the master branch. To avoid make things worse, I'm considering to merge back at master only after having the patches merged upstream. > >> This new procedure is still experimental, but I think it will be better >> than the previous one. > -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New procedures for git - was Re: [GIT PATCHES FOR 2.6.35] Convert bw-qcam and c-qcam to V4L2
Hi Mauro, On Friday 21 May 2010 16:05:29 Mauro Carvalho Chehab wrote: > Hans Verkuil wrote: > > I had hoped to get hardware to test it with, but no such luck (yet). But > > there seems to be no sense in holding these patches back on the > > off-chance that I can get hardware. > > > > Gerard, you mentioned earlier that you actually had a c-qcam. It would be > > nice if you can test this. > > Pulled, thanks. > > I'm now trying a new way of merging patches: I'm creating some topic > branches, as /staging/. This is the first merge of the new procedure > ;) > > So, basically, those patches went to /staging/v4l1. > > I'll periodically merge the staging patches at devel/for_v42.6.35 branches, > for patches that I'll send for the current merge window, and at > devel/for_v2.6.36, for the patches to the next merge window. When will they be merged to master ? > This new procedure is still experimental, but I think it will be better > than the previous one. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-ci_init-fix
Hi Mauro,
On Fri, 2010-05-21 at 11:45 -0300, Mauro Carvalho Chehab wrote:
> The fix seems ok, but it is at the wrong place: if DVB bus fails, it makes
> no sense on keep running any post-register initialization, like
> calling netup_get_card_info() and copying the mac address.
> The better is to return the fail.
>
> So, I moved the return to the proper place. See bellow.
ok, accepted.
thanks
>
> Cheers,
> Mauro
>
> ---
>
> commit 94096e78ed500d424153da0ecbc69273753f2ee3
> Author: Abylay Ospan
> Date: Wed May 12 04:24:09 2010 -0300
>
> V4L/DVB: cx23885: Check register errors
>
> Fix kernel Oops when number of NetUP Dual DVB-S2-CI cards more than
> DVB_MAX_ADAPTERS limit.
>
> [mche...@redhat.com: move the return to the proper place]
>
> Signed-off-by: Abylay Ospan
> Signed-off-by: Mauro Carvalho Chehab
>
> diff --git a/drivers/media/video/cx23885/cx23885-dvb.c
> b/drivers/media/video/cx23885/cx23885-dvb.c
> index 939079d..1ed058f 100644
> --- a/drivers/media/video/cx23885/cx23885-dvb.c
> +++ b/drivers/media/video/cx23885/cx23885-dvb.c
> @@ -991,6 +991,8 @@ static int dvb_register(struct cx23885_tsport *port)
> ret = videobuf_dvb_register_bus(&port->frontends, THIS_MODULE, port,
> &dev->pci->dev, adapter_nr, 0,
> cx23885_dvb_fe_ioctl_override);
> + if (!ret)
> + return ret;
>
> /* init CI & MAC */
> switch (dev->board) {
> --
> 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
--
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: [PULL] http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-ci_init-fix
Abylai Ospan wrote:
> Mauro,
>
> Please pull follwing fix:
>
> http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-ci_init-fix
>
> "Fix kernel Oops when number of NetUP Dual DVB-S2-CI cards more than
> DVB_MAX_ADAPTERS limit"
>
> Thanks.
>
The fix seems ok, but it is at the wrong place: if DVB bus fails, it makes
no sense on keep running any post-register initialization, like
calling netup_get_card_info() and copying the mac address.
The better is to return the fail.
So, I moved the return to the proper place. See bellow.
Cheers,
Mauro
---
commit 94096e78ed500d424153da0ecbc69273753f2ee3
Author: Abylay Ospan
Date: Wed May 12 04:24:09 2010 -0300
V4L/DVB: cx23885: Check register errors
Fix kernel Oops when number of NetUP Dual DVB-S2-CI cards more than
DVB_MAX_ADAPTERS limit.
[mche...@redhat.com: move the return to the proper place]
Signed-off-by: Abylay Ospan
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/video/cx23885/cx23885-dvb.c
b/drivers/media/video/cx23885/cx23885-dvb.c
index 939079d..1ed058f 100644
--- a/drivers/media/video/cx23885/cx23885-dvb.c
+++ b/drivers/media/video/cx23885/cx23885-dvb.c
@@ -991,6 +991,8 @@ static int dvb_register(struct cx23885_tsport *port)
ret = videobuf_dvb_register_bus(&port->frontends, THIS_MODULE, port,
&dev->pci->dev, adapter_nr, 0,
cx23885_dvb_fe_ioctl_override);
+ if (!ret)
+ return ret;
/* init CI & MAC */
switch (dev->board) {
--
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
New procedures for git - was Re: [GIT PATCHES FOR 2.6.35] Convert bw-qcam and c-qcam to V4L2
Hans Verkuil wrote: > I had hoped to get hardware to test it with, but no such luck (yet). But > there seems to be no sense in holding these patches back on the off-chance > that I can get hardware. > > Gerard, you mentioned earlier that you actually had a c-qcam. It would be > nice if you can test this. Pulled, thanks. I'm now trying a new way of merging patches: I'm creating some topic branches, as /staging/. This is the first merge of the new procedure ;) So, basically, those patches went to /staging/v4l1. I'll periodically merge the staging patches at devel/for_v42.6.35 branches, for patches that I'll send for the current merge window, and at devel/for_v2.6.36, for the patches to the next merge window. This new procedure is still experimental, but I think it will be better than the previous one. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug in konicawc webcam driver
I have a problem with usb camera driver. It's konicawc. It was working in previous kernels 2.4 for sure (I have an old installation of Slackware 2.4.26) and perhaps in 2.6. Now in the latest kernel it doesn't work. The file is /drivers/media/video/usbvideo/konicawc.c I couldn't find anyone in MAINTAINERS for this driver and also can't google how to fix this bug. The error message is "Lost sync on frames" the same as here https://lists.linux-foundation.org/pipermail/bugme-new/2004-August/010977.html There is a patch but it seems it's already applied in current source. If it could help.I can try older version of 2.6 to find out when this driver broke. -- Patryk -- 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: [RFC] V4L2 Controls State Store/Restore File Format
Martin Hi, 2010/5/21 : > Hi Paulo, > > I haven't looked into the details, but why not use an XML format? It's less > concise, yes, but it's standard, more extensible, and you don't have to > worry about parsing it too much - after all, we already use libxml in > libwebcam. > I've thought about this initially, xml would provide a standard format without the strict rules of the one I'm proposing and yes it would be very extensible and would also make it very simple to group controls, however it as some drawbacks. Mainly it is very hard to parse with standard c code and it would almost certainly require an external lib (libxml). This would be ok for libwebcam and probably some other apps, but the idea was to create a format that requires only very simple code to use. When we finally decide on a format I'll right a small example that can easily be integrated into any application, so I would rather not use any external libs. IMHO xml would be an overkill in this case, but if the general opinion is that such format should be used instead I can easily change the proposal to a xml format. Regards, Paulo > Cheers, > Martin > > > Paulo Assis wrote on 2010-05-20 21:11:36: > >> From: Paulo Assis >> To: Hans de Goede > >> Cc: Laurent Pinchart , >> martin_ru...@logitech.com >> Date: 2010-05-20 21:11 >> Subject: [RFC] V4L2 Controls State Store/Restore File Format >> >> Hans, >> Below is the RFC with my proposed control state file format for >> store/restore functionality. >> I have several doubts, mostly regarding controls that must be set >> atomically with the extended control API. >> The main question is: >> How does an application know that a group of controls must be set >> atomically ? >> Is this reported by the driver or is it something that the application >> must know. >> >> Also for string controls, I've only seen two implementations on RDS >> controls, so I've set these with low precedence/priority order >> compared with other control types. >> >> Awaiting comments, bash it all you want :-) >> >> Regards, >> Paulo >> __ >> >> [RFC] V4L2 Controls State Store/Restore File Format >> >> VERSION >> >> 0.0.1 >> >> ABSTRACT >> >> This document proposes a standard for the file format used by v4l2 >> applications to store/restore the controls state. >> This unified file format allows sharing control profiles between >> applications, making it much easier on both developers and users. >> >> INTRODUCTION >> >> V4l2 controls can be divided by classes and types. >> Controls in different classes are not dependent between themselves, on >> the other end if two controls belong to the same class they may or may >> not be dependent. >> A good example are automatic controls and their absolute counterparts, >> e.g.: V4L2_CID_AUTOGAIN and V4L2_CID_GAIN. >> Controls must be set following the dependency order, automatic >> controls must be set first or else setting the absolute value may >> fail, when that was not the intended behavior (auto disabled). >> After a quick analyses of the v4l2 controls, we are left to conclude >> that auto controls are in most cases of the >> boolean type, with some exceptions like V4L2_CID_EXPOSURE_AUTO, that >> is of the menu type. >> So ordering control priority by control type seems logical and it can >> be done in the following order: >> >> 1-V4L2_CTRL_TYPE_BOOLEAN >> 2-V4L2_CTRL_TYPE_MENU >> 3-V4L2_CTRL_TYPE_INTEGER >> 4-V4L2_CTRL_TYPE_INTEGER64 >> 5-V4L2_CTRL_TYPE_STRING >> >> Button controls are stateless so they can't be stored and thus are out >> of the scope of this document. >> Relative controls are also in effect stateless, since they will always >> depend on their current state and thus can't be stored. >> >> There are also groups of controls that must be set atomically, so >> these need to be grouped together and properly identified when loading >> the controls state from a file. >> >> The proposed file format takes all of this into account and tries to >> make implementation of both store and restore functionality as easy as >> possible. >> >> FILE FORMAT >> >> The proposed file format is a regular text file with lines terminating >> with the newline character '\n'. >> Comments can be inserted by adding '#' at the beginning of the line, >> and can safely be ignored when parsing the file. >> >> FILE EXTENSION >> >> Although not much relevant, the file extension makes it easy to >> visually identify the file type and also for applications to list >> relevant files, so we propose that v4l2 control state files be >> terminated by the suffix: ".v4l" >> >> FILE HEADER >> >> The file must always start with a commented line containing the file >> type identification and the version of this document on which it is >> based: >> >> #V4L2/CTRL/0.0.1 >> >> Additionally it may contain extra information like the application >> name that generated the file and for usb devices the vid and pid of >> the device to whom the controls relate in hexadecimal notation: >> >> APP_NAM
Re: [PATCH 0/3] Driver for the i.MX2x CMOS Sensor Interface
Hi Sascha,
On Fri, May 21, 2010 at 09:20:45AM +0200, Sascha Hauer wrote:
> On Thu, May 06, 2010 at 04:09:38PM +0300, Baruch Siach wrote:
> > This series contains a soc_camera driver for the i.MX25/i.MX27 CSI device,
> > and
> > platform code for the i.MX25 and i.MX27 chips. This driver is based on a
> > driver
> > for i.MX27 CSI from Sascha Hauer, that Alan Carvalho de Assis has posted
> > in
> > linux-media last December[1]. Since all I have is a i.MX25 PDK paltform I
> > can't
> > test the mx27 specific code. Testers and comment are welcome.
> >
> > [1] https://patchwork.kernel.org/patch/67636/
> >
> > Baruch Siach (3):
> > mx2_camera: Add soc_camera support for i.MX25/i.MX27
> > mx27: add support for the CSI device
> > mx25: add support for the CSI device
>
> With the two additions I sent I can confirm this working on i.MX27, so
> no need to remove the related code.
Thanks. I'll add your patches to my queue and resend the series next week.
baruch
--
~. .~ Tk Open Systems
=}ooO--U--Ooo{=
- bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
--
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/3] Driver for the i.MX2x CMOS Sensor Interface
On Thu, May 06, 2010 at 04:09:38PM +0300, Baruch Siach wrote: > This series contains a soc_camera driver for the i.MX25/i.MX27 CSI device, > and > platform code for the i.MX25 and i.MX27 chips. This driver is based on a > driver > for i.MX27 CSI from Sascha Hauer, that Alan Carvalho de Assis has posted in > linux-media last December[1]. Since all I have is a i.MX25 PDK paltform I > can't > test the mx27 specific code. Testers and comment are welcome. > > [1] https://patchwork.kernel.org/patch/67636/ > > Baruch Siach (3): > mx2_camera: Add soc_camera support for i.MX25/i.MX27 > mx27: add support for the CSI device > mx25: add support for the CSI device With the two additions I sent I can confirm this working on i.MX27, so no need to remove the related code. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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 1/3] mx2_camera: Add soc_camera support for i.MX25/i.MX27
On Thu, May 06, 2010 at 04:09:39PM +0300, Baruch Siach wrote:
> This is the soc_camera support developed by Sascha Hauer for the i.MX27. Alan
> Carvalho de Assis modified the original driver to get it working on more
> recent
> kernels. I modified it further to add support for i.MX25. This driver has only
> been tested on the i.MX25 platform.
>
> Signed-off-by: Baruch Siach
> ---
> arch/arm/plat-mxc/include/mach/memory.h |4 +-
> arch/arm/plat-mxc/include/mach/mx2_cam.h | 41 +
> drivers/media/video/Kconfig | 14 +
> drivers/media/video/Makefile |1 +
> drivers/media/video/mx2_camera.c | 1396
> ++
> 5 files changed, 1454 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm/plat-mxc/include/mach/mx2_cam.h
> create mode 100644 drivers/media/video/mx2_camera.c
>
On i.MX27 in DMA mode you pass the kernel virtual address to the DMA
engine. Should be a physical address, so please amend the following:
>From 8a1e1ac8b2448a5c83ed933e427cccd00d470308 Mon Sep 17 00:00:00 2001
From: Sascha Hauer
Date: Fri, 21 May 2010 09:15:22 +0200
Subject: [PATCH 2/2] mx2_camera: pass physical address to DMA engine
Signed-off-by: Sascha Hauer
---
drivers/media/video/mx2_camera.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 5d6fb08..6436968 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -216,6 +216,7 @@ struct mx2_camera_dev {
unsigned intirq_csi, irq_emma;
void __iomem*base_csi, *base_emma;
+ unsigned long base_dma;
struct mx2_camera_platform_data *pdata;
struct resource *res_csi, *res_emma;
@@ -550,7 +551,7 @@ static void mx2_videobuf_queue(struct videobuf_queue *vq,
if (!pcdev->active) {
ret = imx_dma_setup_single(pcdev->dma,
videobuf_to_dma_contig(vb), vb->size,
- (u32)pcdev->base_csi + 0x10,
+ (u32)pcdev->base_dma + 0x10,
DMA_MODE_READ);
if (ret) {
vb->state = VIDEOBUF_ERROR;
@@ -976,7 +977,8 @@ static void mx27_camera_frame_done(struct mx2_camera_dev
*pcdev, int state)
vb->state = VIDEOBUF_ACTIVE;
ret = imx_dma_setup_single(pcdev->dma, videobuf_to_dma_contig(vb),
- vb->size, (u32)pcdev->base_csi + 0x10, DMA_MODE_READ);
+ vb->size, (u32)pcdev->base_dma + 0x10, DMA_MODE_READ);
+
if (ret) {
vb->state = VIDEOBUF_ERROR;
wake_up(&vb->done);
@@ -1273,6 +1275,7 @@ static int mx2_camera_probe(struct platform_device *pdev)
}
pcdev->irq_csi = irq_csi;
pcdev->base_csi = base_csi;
+ pcdev->base_dma = res_csi->start;
pcdev->dev = &pdev->dev;
err = request_irq(pcdev->irq_csi, mx2_cam_irq_handler, 0,
--
1.7.0
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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 2/3] mx27: add support for the CSI device
On Thu, May 06, 2010 at 04:09:40PM +0300, Baruch Siach wrote:
> Signed-off-by: Baruch Siach
> ---
> arch/arm/mach-mx2/clock_imx27.c |2 +-
> arch/arm/mach-mx2/devices.c | 31 +++
> arch/arm/mach-mx2/devices.h |1 +
> 3 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-mx2/clock_imx27.c b/arch/arm/mach-mx2/clock_imx27.c
> index 0f0823c..5a1aa15 100644
> --- a/arch/arm/mach-mx2/clock_imx27.c
> +++ b/arch/arm/mach-mx2/clock_imx27.c
> @@ -644,7 +644,7 @@ static struct clk_lookup lookups[] = {
> _REGISTER_CLOCK("spi_imx.1", NULL, cspi2_clk)
> _REGISTER_CLOCK("spi_imx.2", NULL, cspi3_clk)
> _REGISTER_CLOCK("imx-fb.0", NULL, lcdc_clk)
> - _REGISTER_CLOCK(NULL, "csi", csi_clk)
> + _REGISTER_CLOCK("mx2-camera.0", NULL, csi_clk)
> _REGISTER_CLOCK("fsl-usb2-udc", "usb", usb_clk)
> _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usb_clk1)
> _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk)
> diff --git a/arch/arm/mach-mx2/devices.c b/arch/arm/mach-mx2/devices.c
> index b91e412..de501ac 100644
> --- a/arch/arm/mach-mx2/devices.c
> +++ b/arch/arm/mach-mx2/devices.c
> @@ -40,6 +40,37 @@
>
> #include "devices.h"
>
> +#ifdef CONFIG_MACH_MX27
> +static struct resource mx27_camera_resources[] = {
> + {
> +.start = CSI_BASE_ADDR,
> +.end = CSI_BASE_ADDR + 0x1f,
> +.flags = IORESOURCE_MEM,
> + }, {
> +.start = EMMA_PRP_BASE_ADDR,
> +.end = EMMA_PRP_BASE_ADDR + 0x1f,
> +.flags = IORESOURCE_MEM,
> + }, {
> +.start = MXC_INT_CSI,
> +.end = MXC_INT_CSI,
> +.flags = IORESOURCE_IRQ,
> + },{
> +.start = MXC_INT_EMMAPRP,
> +.end = MXC_INT_EMMAPRP,
> +.flags = IORESOURCE_IRQ,
> + },
> +};
> +struct platform_device mx27_camera_device = {
> + .name = "mx2-camera",
> + .id = 0,
> + .num_resources = ARRAY_SIZE(mx27_camera_resources),
> + .resource = mx27_camera_resources,
> + .dev = {
> + .coherent_dma_mask = 0x,
> + },
> +};
> +#endif
> +
> /*
> * SPI master controller
> *
> diff --git a/arch/arm/mach-mx2/devices.h b/arch/arm/mach-mx2/devices.h
> index 84ed513..8bdf018 100644
> --- a/arch/arm/mach-mx2/devices.h
> +++ b/arch/arm/mach-mx2/devices.h
> @@ -29,6 +29,7 @@ extern struct platform_device mxc_i2c_device1;
> extern struct platform_device mxc_sdhc_device0;
> extern struct platform_device mxc_sdhc_device1;
> extern struct platform_device mxc_otg_udc_device;
> +extern struct platform_device mx27_camera_device;
> extern struct platform_device mxc_otg_host;
> extern struct platform_device mxc_usbh1;
> extern struct platform_device mxc_usbh2;
> --
> 1.7.0
>
>
Please amend the following to this patch to make it compile on i.MX27:
>From f1dc7e4c35ea0847e5527d9db50b6343c655de8c Mon Sep 17 00:00:00 2001
From: Sascha Hauer
Date: Thu, 20 May 2010 13:36:11 +0200
Subject: [PATCH 1/2] mx27 devices: Fix CSI/EMMA base addresses
Signed-off-by: Sascha Hauer
---
arch/arm/mach-mx2/devices.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-mx2/devices.c b/arch/arm/mach-mx2/devices.c
index de501ac..6a49c79 100644
--- a/arch/arm/mach-mx2/devices.c
+++ b/arch/arm/mach-mx2/devices.c
@@ -43,20 +43,20 @@
#ifdef CONFIG_MACH_MX27
static struct resource mx27_camera_resources[] = {
{
- .start = CSI_BASE_ADDR,
- .end = CSI_BASE_ADDR + 0x1f,
+ .start = MX27_CSI_BASE_ADDR,
+ .end = MX27_CSI_BASE_ADDR + 0x1f,
.flags = IORESOURCE_MEM,
}, {
- .start = EMMA_PRP_BASE_ADDR,
- .end = EMMA_PRP_BASE_ADDR + 0x1f,
+ .start = MX27_EMMA_PRP_BASE_ADDR,
+ .end = MX27_EMMA_PRP_BASE_ADDR + 0x1f,
.flags = IORESOURCE_MEM,
}, {
- .start = MXC_INT_CSI,
- .end = MXC_INT_CSI,
+ .start = MX27_INT_CSI,
+ .end = MX27_INT_CSI,
.flags = IORESOURCE_IRQ,
},{
- .start = MXC_INT_EMMAPRP,
- .end = MXC_INT_EMMAPRP,
+ .start = MX27_INT_EMMAPRP,
+ .end = MX27_INT_EMMAPRP,
.flags = IORESOURCE_IRQ,
},
};
--
1.7.0
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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
