Re: [V2, 2/2] media: i2c: Add DW9768 VCM driver

Javier Martinez Canillas
2019-09-05T10:57:34Z

On 9/5/19 12:40 PM, Sakari Ailus wrote:
> On Thu, Sep 05, 2019 at 01:19:08PM +0300, Andy Shevchenko wrote:
>> On Thu, Sep 05, 2019 at 11:21:34AM +0300, Sakari Ailus wrote:
>>> On Thu, Sep 05, 2019 at 03:21:42PM +0800, dongchun.zhu@mediatek.com wrote:
>>>> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
>>
>>>> +static const struct i2c_device_id dw9768_id_table[] = {
>>>> + { DW9768_NAME, 0 },
>>>> + { },
>>>
>>> Could you drop the I²C ID table?
>>
>> But why?
>> It will allow you to instanciate the device from user space.

Yes, the I2C device table is still needed if the device can be instantiated
from user-space using the sysfs interface, or otherwise the module won't be
automatically loaded.

Kieran posted a "[PATCH RFC] modpost: Support I2C Aliases from OF tables"
patch that adds a MODULE_DEVICE_TABLE(i2c_of, ..) macro so modpost could
add legacy I2C modalias using the information in the OF device ID tables:

https://patchwork.kernel.org/patch/11038861/

If that lands, then we could get rid of the I2C device tables altogether
for non-legacy I2C drivers.

>
> The device is supposed to be present in DT (or ACPI tables) already.
>

Agreed. Also by looking at the driver's probe function I see that the
device lookups a 'vin' and 'vdd' regulators supplies and it fails if
aren't defined, so it can't be instantiated from user-space anyways.

BTW, these two regulators supplies should be listed as 'vin-supply'
and 'vdd-supply' as required properties in the DT binding document.

Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org