linux-kernel - Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180329082923.e55pclvlclamnsqz@paasikivi.fi.intel.com>
Date:   Thu, 29 Mar 2018 11:29:23 +0300
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Todor Tomov <todor.tomov@...aro.org>
Cc:     jacopo mondi <jacopo@...ndi.org>, mchehab@...nel.org,
        hverkuil@...all.nl, laurent.pinchart@...asonboard.com,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [v2,2/2] media: Add a driver for the ov7251 camera sensor

Hi Todor and Jacopo,

On Thu, Mar 29, 2018 at 10:50:10AM +0300, Todor Tomov wrote:
...
> >> +static const struct of_device_id ov7251_of_match[] = {
> >> +	{ .compatible = "ovti,ov7251" },
> >> +	{ /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, ov7251_of_match);
> >> +
> >> +static struct i2c_driver ov7251_i2c_driver = {
> >> +	.driver = {
> >> +		.of_match_table = of_match_ptr(ov7251_of_match),
> >> +		.name  = "ov7251",
> >> +	},
> >> +	.probe  = ov7251_probe,
> >> +	.remove = ov7251_remove,
> >> +	.id_table = ov7251_id,
> > 
> > As this driver depends on CONFIG_OF, I've been suggested to use probe_new and
> > get rid of i2c id_tables.
> 
> Yes, I'll do that.

The proposal sounds good to me but rather than adding CONFIG_OF dependency,
I'd instead suggest changing the of_property_read_u32 to
fwnode_property_read_u32; then the driver may work on ACPI based systems as
well. There's another change needed, too, which is not using of_match_ptr
macro, but instead assigning the of_match_table unconditionally.

Up to you.

-- 
Sakari Ailus
sakari.ailus@...ux.intel.com