Hi Stefan,
On Thu, Oct 01, 2020 at 10:56:24AM +0200, Stefan Riedmüller wrote:
> On 30.09.20 13:38, Laurent Pinchart wrote:
> > On Wed, Sep 30, 2020 at 12:51:31PM +0200, Stefan Riedmueller wrote:
> >> From: Enrico Scholz <enrico.sch...@sigma-chemnitz.de>
> >>
> >> Implement g_register and s_register v4l2_subdev_core_ops to access
> >> camera register directly from userspace for debug purposes.
> >
> > As the name of the operations imply, this is meant for debug purpose
> > only. They are however prone to be abused to configure the sensor from
> > userspace in production, which isn't a direction we want to take.
> > What's your use case for this ? I'd rather drop this patch and see the
> > driver extended with support for more controls if needed
>
> thanks for your feedback.
>
> I get your point. I myself solely use these operations for debugging
> purposes but I'm aware that others like to abuse them.
>
> I thought I send it anyway since for me the DEBUG config is enough to
> signalize that these operations are not to be used with a productive system.
> But I'm OK with dropping this patch if you think it might send the wrong
> signal.
I'd rather avoid this patch due to the risk of abuse if it's OK with
you.
> >> Signed-off-by: Enrico Scholz <enrico.sch...@sigma-chemnitz.de>
> >> Signed-off-by: Stefan Riedmueller <s.riedmuel...@phytec.de>
> >> ---
> >> No changes in v2
> >> ---
> >> drivers/media/i2c/mt9p031.c | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> >> index b4c042f418c1..de36025260a8 100644
> >> --- a/drivers/media/i2c/mt9p031.c
> >> +++ b/drivers/media/i2c/mt9p031.c
> >> @@ -703,6 +703,30 @@ static int mt9p031_restore_blc(struct mt9p031
> >> *mt9p031)
> >> return 0;
> >> }
> >>
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> +static int mt9p031_g_register(struct v4l2_subdev *sd,
> >> + struct v4l2_dbg_register *reg)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> + int ret;
> >> +
> >> + ret = mt9p031_read(client, reg->reg);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + reg->val = ret;
> >> + return 0;
> >> +}
> >> +
> >> +static int mt9p031_s_register(struct v4l2_subdev *sd,
> >> + struct v4l2_dbg_register const *reg)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + return mt9p031_write(client, reg->reg, reg->val);
> >> +}
> >> +#endif
> >> +
> >> static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
> >> {
> >> struct mt9p031 *mt9p031 =
> >> @@ -1000,6 +1024,10 @@ static int mt9p031_close(struct v4l2_subdev
> >> *subdev, struct v4l2_subdev_fh *fh)
> >>
> >> static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = {
> >> .s_power = mt9p031_set_power,
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> + .s_register = mt9p031_s_register,
> >> + .g_register = mt9p031_g_register,
> >> +#endif
> >> };
> >>
> >> static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = {
--
Regards,
Laurent Pinchart
