Re: [PULL] http://linuxtv.org/hg/~awalls/cx23888-ir-part2
Em Sun, 11 Oct 2009 08:21:24 -0400 Andy Walls awa...@radix.net escreveu: On Sun, 2009-10-11 at 07:37 -0300, Mauro Carvalho Chehab wrote: Em Sun, 27 Sep 2009 20:12:12 -0400 Andy Walls awa...@radix.net escreveu: Mauro, Please pull from http://linuxtv.org/hg/~awalls/cx23888-ir-part2 for the following 5 changesets: 01/05: v4l2-subdev: Add v4l2_subdev_ir_ops and IR notify defines for v4l2_device http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=8cbb951bbb9f 02/05: cx23885: Complete CX23888 IR subdev implementation for Rx almost for Tx http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=a2d8d3d88c6d Andy, there are some coding style issues here, not properly reported by the current checkpatch.pl version we had at the tree, nor by the newer version. I've opened a separate thread at LKML about that. For example, it didn't got any CodingStyle troubles on the formulas like: + if (d RXCLK_RCD+1) +CX23888_IR_REFCLK_FREQ/100); + if (rem = CX23888_IR_REFCLK_FREQ/100/2) + clocks = CX23888_IR_REFCLK_FREQ/100 * (u64) ns; /* millicycles */ + return DIV_ROUND_CLOSEST((n+1) * 100, 16); As there are no related changes on Documentation/CodingStyle, changing or relaxing whitespacing rules, it is better to wait for the checkpatch.pl Maintainer (also called Andy ;) ) for him to check what's going wrong and hopefully provide a fix for the regression. I'm afraid that more relevant codingstyle checks like the usage of deprecated functions could eventually be broken. So, better to wait for his answer, before proceed with patch merging. OK. I'm going to use alot of this code as is in the cx25840 module, it makes sense to get this version cleaned up first. cx23885: Complete CX23888 IR subdev implementation for Rx almost for Tx There are some bad whitespacing here violating CodingStyle that checkpatch.pl didn't get. Please submit a latter patch fixing it. I suspect it is a checkpatch.pl regression, but it seems that we won't have a fix for it soon. I do have a concern that this set of changes may need revision if the cx23885 module or ir-common.h or ir-functions.c change significantly. However you have positive control over those. Yes. I don't think we should have significant changes at rc5 decode routine. The better would be to add there also the pulse-distance and nec decoding code to ir-functions (implemented on some drivers), to have there all common IR code. My other concern is kfifo being changed, requiring rework of my second set of changes. There were some patches on the LKML for fixing kfifo with some positive discussions. Last I checked, nothing had moved forward on kfifo. The current kfifo implementation, IMO, need a few changes for it to be widely used. 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/~awalls/cx23888-ir-part2
Em Sun, 27 Sep 2009 20:12:12 -0400 Andy Walls awa...@radix.net escreveu: Mauro, Please pull from http://linuxtv.org/hg/~awalls/cx23888-ir-part2 for the following 5 changesets: 01/05: v4l2-subdev: Add v4l2_subdev_ir_ops and IR notify defines for v4l2_device http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=8cbb951bbb9f 02/05: cx23885: Complete CX23888 IR subdev implementation for Rx almost for Tx http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=a2d8d3d88c6d Andy, there are some coding style issues here, not properly reported by the current checkpatch.pl version we had at the tree, nor by the newer version. I've opened a separate thread at LKML about that. For example, it didn't got any CodingStyle troubles on the formulas like: + if (d RXCLK_RCD+1) +CX23888_IR_REFCLK_FREQ/100); + if (rem = CX23888_IR_REFCLK_FREQ/100/2) + clocks = CX23888_IR_REFCLK_FREQ/100 * (u64) ns; /* millicycles*/ + return DIV_ROUND_CLOSEST((n+1) * 100, 16); As there are no related changes on Documentation/CodingStyle, changing or relaxing whitespacing rules, it is better to wait for the checkpatch.pl Maintainer (also called Andy ;) ) for him to check what's going wrong and hopefully provide a fix for the regression. I'm afraid that more relevant codingstyle checks like the usage of deprecated functions could eventually be broken. So, better to wait for his answer, before proceed with patch merging. 03/05: cx23885: Add integrated IR subdevice interrupt and notification handling http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=1eb199665dbc 04/05: ir-functions: Export ir_rc5_decode() for use by the cx23885 module http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=55a1e2e8128f 05/05: cx23885: Add IR input keypress handling and enable for the HVR-1850 http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=b05a093688a2 b/linux/drivers/media/video/cx23885/cx23885-input.c | 422 +++ b/linux/drivers/media/video/cx23885/cx23885-input.h | 30 b/linux/drivers/media/video/cx23885/cx23885-ir.c| 128 ++ b/linux/drivers/media/video/cx23885/cx23885-ir.h| 36 linux/drivers/media/common/ir-functions.c |3 linux/drivers/media/video/cx23885/Makefile |4 linux/drivers/media/video/cx23885/cx23885-cards.c | 23 linux/drivers/media/video/cx23885/cx23885-core.c| 84 + linux/drivers/media/video/cx23885/cx23885-ir.c |4 linux/drivers/media/video/cx23885/cx23885-reg.h |5 linux/drivers/media/video/cx23885/cx23885.h | 13 linux/drivers/media/video/cx23885/cx23888-ir.c | 1139 +++- linux/include/media/ir-common.h |1 linux/include/media/v4l2-subdev.h | 94 + 14 files changed, 1932 insertions(+), 54 deletions(-) This is the remainder of the changes required to get IR Rx input keypress handling working for the HVR-1850. This builds upon my previous pull request for my cx23888-ir-part1 repo, which include Steven's patch. (This cx23888-ir-part2 repo also has all the patches in the cx23888-ir-part1 repo.) The changes to v4l2-subdev.h include the struct v4l2_subdev_ir_ops definition that Hans had OK'ed in previous e-mails plus a few other defines I felt needed to be common eventually. When both this and the previous pull request are merged, I can then work on IR support for CX23885 and CX23418 cards that use essentially the same integrated IR controller. Thanks, Andy 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/~awalls/cx23888-ir-part2
On Sun, 2009-10-11 at 07:37 -0300, Mauro Carvalho Chehab wrote: Em Sun, 27 Sep 2009 20:12:12 -0400 Andy Walls awa...@radix.net escreveu: Mauro, Please pull from http://linuxtv.org/hg/~awalls/cx23888-ir-part2 for the following 5 changesets: 01/05: v4l2-subdev: Add v4l2_subdev_ir_ops and IR notify defines for v4l2_device http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=8cbb951bbb9f 02/05: cx23885: Complete CX23888 IR subdev implementation for Rx almost for Tx http://linuxtv.org/hg/~awalls/cx23888-ir-part2?cmd=changeset;node=a2d8d3d88c6d Andy, there are some coding style issues here, not properly reported by the current checkpatch.pl version we had at the tree, nor by the newer version. I've opened a separate thread at LKML about that. For example, it didn't got any CodingStyle troubles on the formulas like: + if (d RXCLK_RCD+1) +CX23888_IR_REFCLK_FREQ/100); + if (rem = CX23888_IR_REFCLK_FREQ/100/2) + clocks = CX23888_IR_REFCLK_FREQ/100 * (u64) ns; /* millicycles */ + return DIV_ROUND_CLOSEST((n+1) * 100, 16); As there are no related changes on Documentation/CodingStyle, changing or relaxing whitespacing rules, it is better to wait for the checkpatch.pl Maintainer (also called Andy ;) ) for him to check what's going wrong and hopefully provide a fix for the regression. I'm afraid that more relevant codingstyle checks like the usage of deprecated functions could eventually be broken. So, better to wait for his answer, before proceed with patch merging. OK. I'm going to use alot of this code as is in the cx25840 module, it makes sense to get this version cleaned up first. I do have a concern that this set of changes may need revision if the cx23885 module or ir-common.h or ir-functions.c change significantly. However you have positive control over those. My other concern is kfifo being changed, requiring rework of my second set of changes. There were some patches on the LKML for fixing kfifo with some positive discussions. Last I checked, nothing had moved forward on kfifo. Regards, Andy -- 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
