Re: [PATCH v2 11/11] mt9m111: make use of testpattern
Guennadi Liakhovetski g.liakhovet...@gmx.de writes: Yes, but this has another disadvantage - if you do not use s_register / g_register, maybe you just have CONFIG_VIDEO_ADV_DEBUG off, then, once you load the module with the testpattern parameter, you cannot switch using testpatterns off again (without a reboot or a power cycle). With the original version you can load the driver with the parameter set, then unload it, load it without the parameter and testpattern would be cleared. In general, I think, using direct register access is discouraged, especially if there's a way to set the same functionality using driver's supported interfaces. I agree. If there is a way without debug registers, let's use it. Hm, if I'm not mistaken, it has once been mentioned, that these test-patterns can be nicely implemented using the S_INPUT ioctl(). Am I right? How about that? But we'd need a confirmation for that, I'm not 100% sure. I can't remember that. But if there is a standard ioctl (as seems to show videodev2.h), and that its use could mean camera's input is a testpattern or camera input is the normal optical flow, then we should use it. If not, the old way with debug registers is the only alternative I see without having to unload/reload the module (if it's a module and not statically embedded in the kernel). Cheers. -- Robert -- 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 v2 11/11] mt9m111: make use of testpattern
Michael Grzeschik m.grzesc...@pengutronix.de writes: Signed-off-by: Philipp Wiesner p.wies...@phytec.de Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de I would require a small change here. I am using the testpattern for non regression tests. This change implies that the test pattern can only be set up by module parameters, and blocks the usage through V4L2 debug, registers, see below: memset(set_reg, 0, sizeof(set_reg)); set_reg.match.type = V4L2_CHIP_MATCH_I2C_ADDR; set_reg.match.addr = 0x5d; set_reg.reg = 0x148; set_reg.val = test_pattern; set_reg.size = 1; if (test_pattern != -1) if (-1 == xioctl (fd, VIDIOC_DBG_S_REGISTER, set_reg)) { fprintf (stderr, %s could set test pattern %x\n, dev_name, test_pattern); exit (EXIT_FAILURE); } But, the idea is not bad. Therefore, I'd like you to change: + dev_dbg(client-dev, %s: using testpattern %d\n, __func__, + testpattern); + + if (!ret) + ret = mt9m111_reg_set(client, + MT9M111_TEST_PATTERN_GEN, pattern); into + dev_dbg(client-dev, %s: using testpattern %d\n, __func__, + testpattern); + + if (!ret pattern) + ret = mt9m111_reg_set(client, + MT9M111_TEST_PATTERN_GEN, pattern); + This way, the V4L2 debug registers usage is still allowed, and your module parameter works too. Cheers. -- Robert -- 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 v2 11/11] mt9m111: make use of testpattern
On Sun, 29 Aug 2010, Robert Jarzmik wrote: Michael Grzeschik m.grzesc...@pengutronix.de writes: Signed-off-by: Philipp Wiesner p.wies...@phytec.de Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de I would require a small change here. I am using the testpattern for non regression tests. This change implies that the test pattern can only be set up by module parameters, and blocks the usage through V4L2 debug, registers, see below: memset(set_reg, 0, sizeof(set_reg)); set_reg.match.type = V4L2_CHIP_MATCH_I2C_ADDR; set_reg.match.addr = 0x5d; set_reg.reg = 0x148; set_reg.val = test_pattern; set_reg.size = 1; if (test_pattern != -1) if (-1 == xioctl (fd, VIDIOC_DBG_S_REGISTER, set_reg)) { fprintf (stderr, %s could set test pattern %x\n, dev_name, test_pattern); exit (EXIT_FAILURE); } But, the idea is not bad. Therefore, I'd like you to change: + dev_dbg(client-dev, %s: using testpattern %d\n, __func__, + testpattern); + + if (!ret) + ret = mt9m111_reg_set(client, + MT9M111_TEST_PATTERN_GEN, pattern); into + dev_dbg(client-dev, %s: using testpattern %d\n, __func__, + testpattern); + + if (!ret pattern) + ret = mt9m111_reg_set(client, + MT9M111_TEST_PATTERN_GEN, pattern); + This way, the V4L2 debug registers usage is still allowed, and your module parameter works too. Yes, but this has another disadvantage - if you do not use s_register / g_register, maybe you just have CONFIG_VIDEO_ADV_DEBUG off, then, once you load the module with the testpattern parameter, you cannot switch using testpatterns off again (without a reboot or a power cycle). With the original version you can load the driver with the parameter set, then unload it, load it without the parameter and testpattern would be cleared. In general, I think, using direct register access is discouraged, especially if there's a way to set the same functionality using driver's supported interfaces. Hm, if I'm not mistaken, it has once been mentioned, that these test-patterns can be nicely implemented using the S_INPUT ioctl(). Am I right? How about that? But we'd need a confirmation for that, I'm not 100% sure. 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