Hi Sakari
On 29.03.2018 11:29, Sakari Ailus wrote:
> 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.
Ok.
> There's another change needed, too, which is not using of_match_ptr
> macro, but instead assigning the of_match_table unconditionally.
In that case the MODULE_DEVICE_TABLE(i2c, ...) is again not needed?
And matching will be again via of_match_table?
>
> Up to you.
>
Thank you for your help!
Best regards,
Todor Tomov
