Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH v2 3/5] media: mt9p031: Implement [gs]_register debug calls | From | Stefan Riedmüller <> | Date | Mon, 5 Oct 2020 11:33:00 +0200 |
| |
Hi Laurent,
On 02.10.20 02:06, Laurent Pinchart wrote: > 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.scholz@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.
Yes, that's fine. I will drop it in v3.
Regards, Stefan
> >>>> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> >>>> Signed-off-by: Stefan Riedmueller <s.riedmueller@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 = { >
| ![\](/images/icornerr.gif) |