Re: [PATCH 2/6] media: i2c: ov5670: Allow probing with OF

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

On Mon, Mar 14, 2022 at 09:42:08AM +0100, Jacopo Mondi wrote:
> On Sun, Mar 13, 2022 at 04:33:12PM +0200, Laurent Pinchart wrote:
> > On Thu, Mar 10, 2022 at 02:08:25PM +0100, Jacopo Mondi wrote:
> > > The ov5670 driver currently only supports probing using ACPI matching.
> > > Add support for OF and add a missing header inclusion.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  drivers/media/i2c/ov5670.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> > > index 02f75c18e480..39786f3c9489 100644
> > > --- a/drivers/media/i2c/ov5670.c
> > > +++ b/drivers/media/i2c/ov5670.c
> > > @@ -3,7 +3,9 @@
> > >
> > >  #include <linux/acpi.h>
> > >  #include <linux/i2c.h>
> > > +#include <linux/mod_devicetable.h>
> > >  #include <linux/module.h>
> > > +#include <linux/of.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <media/v4l2-ctrls.h>
> > >  #include <media/v4l2-device.h>
> > > @@ -2583,6 +2585,12 @@ static const struct acpi_device_id ov5670_acpi_ids[] = {
> > >  };
> > >
> > >  MODULE_DEVICE_TABLE(acpi, ov5670_acpi_ids);
> > > +#elif defined CONFIG_OF
> >
> > This should be
> >
> > #ifdef CONFIG_OF
> > ...
> > #endif
> >
> > to support kernels compiled with both CONFIG_ACPI and CONFIG_OF.
> 
> Actually, as kernel test robot reported, I should declare the id
> tables unconditionally, and let of_match_ptr() and ACPI_PTR() macros
> expand to NULL if the corresponding symbol is not defined

With a __maybe_unused that should work too. I don't mind either way.

> 
> https://patchwork.linuxtv.org/project/linux-media/patch/20220310130829.96001-3-jacopo@jmondi.org/#135841
> 
> > With this fixed,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks
> 
> > > +static const struct of_device_id ov5670_of_ids[] = {
> > > +	{ .compatible = "ovti,ov5670" },
> > > +	{ /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov5670_of_ids);
> > >  #endif
> > >
> > >  static struct i2c_driver ov5670_i2c_driver = {
> > > @@ -2590,6 +2598,7 @@ static struct i2c_driver ov5670_i2c_driver = {
> > >  		.name = "ov5670",
> > >  		.pm = &ov5670_pm_ops,
> > >  		.acpi_match_table = ACPI_PTR(ov5670_acpi_ids),
> > > +		.of_match_table = of_match_ptr(ov5670_of_ids),
> > >  	},
> > >  	.probe_new = ov5670_probe,
> > >  	.remove = ov5670_remove,

-- 
Regards,

Laurent Pinchart