Re: [PATCH v2 2/2] media: i2c: add support for ov4689

[Sakari Ailus]

Hi Krzysztof, Mikhail,

On Fri, Sep 16, 2022 at 10:43:11AM +0100, Krzysztof Kozlowski wrote:
> On 15/09/2022 21:40, Mikhail Rudenko wrote:
> > 
> > Hi Krzysztof,
> > 
> > On 2022-09-12 at 12:56 +02, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >> On 11/09/2022 22:01, Mikhail Rudenko wrote:
> >>> +static const struct i2c_device_id ov4689_id[] = {
> >>> +	{ "ov4689", 0 },
> >>> +	{},
> >>> +};
> >>> +MODULE_DEVICE_TABLE(i2c, ov4689_id);
> >>> +
> >>> +static const struct of_device_id ov4689_of_match[] = {
> >>> +	{ .compatible = "ovti,ov4689" },
> >>> +	{},
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, ov4689_of_match);
> >>> +
> >>> +static struct i2c_driver ov4689_i2c_driver = {
> >>> +	.driver = {
> >>> +		.name = "ov4689",
> >>> +		.pm = &ov4689_pm_ops,
> >>> +		.of_match_table = of_match_ptr(ov4689_of_match),
> >>
> >> of_match_ptr is usually paired with maybe_unused, otherwise you will
> >> have compile test warnings.
> > 
> > I see. I think we could also use `#if IS_ENABLED(CONFIG_OF)` around
> > `ov4689_of_match` and the corresponding `MODULE_DEVICE_TABLE`. Is it
> > appropriate here?
> 
> Would work, but ifdefs are not nice. Just use maybe_unused. Warnings
> should disappear.

Neither is needed. Just drop of_match_ptr().

Apart from clock handling, it would probably work on ACPI, too. No need to
address that though.

-- 
Regards,

Sakari Ailus