Re: [PATCH v14 2/2] media: i2c: Add OV02A10 image sensor driver

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Mon, Sep 07, 2020 at 03:15:17PM +0200, Tomasz Figa wrote:
> On Fri, Sep 4, 2020 at 4:06 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Sep 4, 2020 at 4:48 PM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > On Wed, 2020-09-02 at 16:44 +0300, Andy Shevchenko wrote:
> > > > On Wed, Sep 02, 2020 at 08:01:22PM +0800, Dongchun Zhu wrote:

...

> > > > > +   struct i2c_client *client = to_i2c_client(dev);
> > > > > +   struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > >
> > > >       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > > >
> > > > Same for the rest similar cases.
> > >
> > > We've discussed the issue in DW9768 V2.
> > >
> > > For V4L2 sub-device drivers, dev_get_drvdata() shouldn't be used
> > > directly.
> > >
> > > More details please check the Google Issue:
> > > https://partnerissuetracker.corp.google.com/issues/147957975
> >
> > This is not a public link. Can you remind me what was the issue?
> >
> 
> v4l2-subdev framework uses dev drvdata for its own purposes. However,
> that problem was about the driver setting its own drvdata and having
> it overridden by the framework. There is nothing wrong in using
> dev_get_drvdata(), assuming the correct type is known and here it's
> guaranteed to be v4l2_subdev for the v4l2-subdev framework.
> 
> In fact i2c_get_clientdata() [1] is just a wrapper that calls
> dev_get_drvdata(&client->dev).

And funny future would be that if we don't do this now we will have plenty of
janitors to DoS attack us with the fix in the future.

> [1] https://elixir.bootlin.com/linux/v5.9-rc3/source/include/linux/i2c.h#L351

-- 
With Best Regards,
Andy Shevchenko