diff mbox series

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

Message ID 20220310130829.96001-3-jacopo@jmondi.org (mailing list archive)
State Superseded
Delegated to: Sakari Ailus
Headers
Series media: i2c: ov5670: OF support, runtime_pm, regulators |

Commit Message

Jacopo Mondi March 10, 2022, 1:08 p.m. UTC 
  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(+)

Comments

kernel test robot March 10, 2022, 6:16 p.m. UTC | #1 
Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
base:   git://linuxtv.org/media_tree.git master
config: ia64-allmodconfig (https://download.01.org/0day-ci/archive/20220311/202203110200.BVFJJTh4-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f5425877ffa93005c9f71ce9ce4185a787db66eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
        git checkout f5425877ffa93005c9f71ce9ce4185a787db66eb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/media/i2c/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/irqdomain.h:35,
                    from include/linux/acpi.h:13,
                    from drivers/media/i2c/ov5670.c:4:
>> drivers/media/i2c/ov5670.c:2601:48: error: 'ov5670_of_ids' undeclared here (not in a function); did you mean 'ov5670_acpi_ids'?
    2601 |                 .of_match_table = of_match_ptr(ov5670_of_ids),
         |                                                ^~~~~~~~~~~~~
   include/linux/of.h:411:34: note: in definition of macro 'of_match_ptr'
     411 | #define of_match_ptr(_ptr)      (_ptr)
         |                                  ^~~~


vim +2601 drivers/media/i2c/ov5670.c

  2595	
  2596	static struct i2c_driver ov5670_i2c_driver = {
  2597		.driver = {
  2598			.name = "ov5670",
  2599			.pm = &ov5670_pm_ops,
  2600			.acpi_match_table = ACPI_PTR(ov5670_acpi_ids),
> 2601			.of_match_table = of_match_ptr(ov5670_of_ids),
  2602		},
  2603		.probe_new = ov5670_probe,
  2604		.remove = ov5670_remove,
  2605		.flags = I2C_DRV_ACPI_WAIVE_D0_PROBE,
  2606	};
  2607	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot March 10, 2022, 8:29 p.m. UTC | #2 
Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-a016 (https://download.01.org/0day-ci/archive/20220311/202203110433.anTyFE5N-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f5425877ffa93005c9f71ce9ce4185a787db66eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
        git checkout f5425877ffa93005c9f71ce9ce4185a787db66eb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/i2c/ov5670.c:2601:34: error: use of undeclared identifier 'ov5670_of_ids'
                   .of_match_table = of_match_ptr(ov5670_of_ids),
                                                  ^
   1 error generated.


vim +/ov5670_of_ids +2601 drivers/media/i2c/ov5670.c

  2595	
  2596	static struct i2c_driver ov5670_i2c_driver = {
  2597		.driver = {
  2598			.name = "ov5670",
  2599			.pm = &ov5670_pm_ops,
  2600			.acpi_match_table = ACPI_PTR(ov5670_acpi_ids),
> 2601			.of_match_table = of_match_ptr(ov5670_of_ids),
  2602		},
  2603		.probe_new = ov5670_probe,
  2604		.remove = ov5670_remove,
  2605		.flags = I2C_DRV_ACPI_WAIVE_D0_PROBE,
  2606	};
  2607	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot March 10, 2022, 8:39 p.m. UTC | #3 
Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220311/202203110425.5nMUZpmG-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f5425877ffa93005c9f71ce9ce4185a787db66eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220310-211143
        git checkout f5425877ffa93005c9f71ce9ce4185a787db66eb
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/i2c/ov5670.c:2601:34: error: use of undeclared identifier 'ov5670_of_ids'
                   .of_match_table = of_match_ptr(ov5670_of_ids),
                                                  ^
   1 error generated.


vim +/ov5670_of_ids +2601 drivers/media/i2c/ov5670.c

  2595	
  2596	static struct i2c_driver ov5670_i2c_driver = {
  2597		.driver = {
  2598			.name = "ov5670",
  2599			.pm = &ov5670_pm_ops,
  2600			.acpi_match_table = ACPI_PTR(ov5670_acpi_ids),
> 2601			.of_match_table = of_match_ptr(ov5670_of_ids),
  2602		},
  2603		.probe_new = ov5670_probe,
  2604		.remove = ov5670_remove,
  2605		.flags = I2C_DRV_ACPI_WAIVE_D0_PROBE,
  2606	};
  2607	

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Laurent Pinchart March 13, 2022, 2:33 p.m. UTC | #4 
Hi Jacopo,

Thank you for the patch.

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.

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +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,
Jacopo Mondi March 14, 2022, 8:42 a.m. UTC | #5 
Hi Laurent,

On Sun, Mar 13, 2022 at 04:33:12PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> 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

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
   j

>
> > +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
Laurent Pinchart March 14, 2022, 8:50 a.m. UTC | #6 
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,
Laurent Pinchart March 14, 2022, 8:51 a.m. UTC | #7 
On Mon, Mar 14, 2022 at 10:50:25AM +0200, Laurent Pinchart wrote:
> 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.

Actually, won't you always have the OF module device table then, even
when the kernel is compiled with !OF ? That may not hurt, but it's a
waste of space.

> > 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,
diff mbox series

Patch

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
+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,