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
