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

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Jacopo,

On Tue, Mar 29, 2022 at 11:01:27AM +0200, 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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/ov5670.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index 02f75c18e480..721441024598 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>
> @@ -2585,11 +2587,20 @@ static const struct acpi_device_id ov5670_acpi_ids[] = {
>  MODULE_DEVICE_TABLE(acpi, ov5670_acpi_ids);
>  #endif
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id ov5670_of_ids[] = {
> +	{ .compatible = "ovti,ov5670" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ov5670_of_ids);
> +#endif

No need for ifdefs here.

The compatible strings are also relevant on ACPI (as alternative for _HID
and _CID).

> +
>  static struct i2c_driver ov5670_i2c_driver = {
>  	.driver = {
>  		.name = "ov5670",
>  		.pm = &ov5670_pm_ops,
>  		.acpi_match_table = ACPI_PTR(ov5670_acpi_ids),
> +		.of_match_table = of_match_ptr(ov5670_of_ids),

Nor of_match_ptr() here.

>  	},
>  	.probe_new = ov5670_probe,
>  	.remove = ov5670_remove,

-- 
Regards,

Sakari Ailus