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

Andy Shevchenko
2019-09-05T11:36:38Z

On Thu, Sep 05, 2019 at 12:57:34PM +0200, Javier Martinez Canillas wrote:
> 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.

Thank you for clarifications!

So, it can use ->probe_new() in that case.

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

--
With Best Regards,
Andy Shevchenko