All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javierm@redhat.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	drinkcat@chromium.org, srv_heupstream@mediatek.com,
	sam.hung@mediatek.com, shengnan.wang@mediatek.com,
	tfiga@chromium.org, sj.huang@mediatek.com, robh+dt@kernel.org,
	linux-mediatek@lists.infradead.org, dongchun.zhu@mediatek.com,
	matthias.bgg@gmail.com, bingbu.cao@intel.com, mchehab@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [V2, 2/2] media: i2c: Add DW9768 VCM driver
Date: Thu, 5 Sep 2019 12:57:34 +0200	[thread overview]
Message-ID: <ad357e27-3e51-6922-1924-5d2c2daf1934@redhat.com> (raw)
In-Reply-To: <20190905104001.GZ5475@paasikivi.fi.intel.com>

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
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Javier Martinez Canillas <javierm@redhat.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: dongchun.zhu@mediatek.com, mchehab@kernel.org,
	robh+dt@kernel.org, mark.rutland@arm.com, drinkcat@chromium.org,
	tfiga@chromium.org, matthias.bgg@gmail.com, bingbu.cao@intel.com,
	srv_heupstream@mediatek.com, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, sj.huang@mediatek.com,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	sam.hung@mediatek.com, shengnan.wang@mediatek.com
Subject: Re: [V2, 2/2] media: i2c: Add DW9768 VCM driver
Date: Thu, 5 Sep 2019 12:57:34 +0200	[thread overview]
Message-ID: <ad357e27-3e51-6922-1924-5d2c2daf1934@redhat.com> (raw)
In-Reply-To: <20190905104001.GZ5475@paasikivi.fi.intel.com>

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

  reply	other threads:[~2019-09-05 10:57 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05  7:21 [V2, 0/2] media: i2c: add support for DW9768 VCM driver dongchun.zhu-NuS5LvNUpcJWk0Htik3J/w
2019-09-05  7:21 ` dongchun.zhu
2019-09-05  7:21 ` dongchun.zhu
2019-09-05  7:21 ` [V2, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry dongchun.zhu
2019-09-05  7:21   ` dongchun.zhu
2019-09-05  7:21   ` dongchun.zhu
2019-09-05 10:14   ` Andy Shevchenko
2019-09-05 10:14     ` Andy Shevchenko
2019-09-05 10:14     ` Andy Shevchenko
2019-09-05 10:48     ` Sakari Ailus
2019-09-05 10:48       ` Sakari Ailus
2019-09-05 10:48       ` Sakari Ailus
2019-09-05 11:35       ` Andy Shevchenko
2019-09-05 11:35         ` Andy Shevchenko
2019-09-05 11:35         ` Andy Shevchenko
2019-09-05 11:49         ` Javier Martinez Canillas
2019-09-05 11:49           ` Javier Martinez Canillas
2019-09-05 11:49           ` Javier Martinez Canillas
2019-09-05 12:00           ` Sakari Ailus
2019-09-05 12:00             ` Sakari Ailus
2019-09-05 12:00             ` Sakari Ailus
2019-09-05 12:24             ` Andy Shevchenko
2019-09-05 12:24               ` Andy Shevchenko
2019-09-05 12:24               ` Andy Shevchenko
2019-09-17 20:47               ` Rob Herring
2019-09-17 20:47                 ` Rob Herring
2019-09-17 20:47                 ` Rob Herring
2019-09-05  7:21 ` [V2, 2/2] media: i2c: Add DW9768 VCM driver dongchun.zhu
2019-09-05  7:21   ` dongchun.zhu
2019-09-05  7:21   ` dongchun.zhu
2019-09-05  8:21   ` Sakari Ailus
2019-09-05  8:21     ` Sakari Ailus
2019-09-05  8:21     ` Sakari Ailus
2019-09-05 10:19     ` Andy Shevchenko
2019-09-05 10:19       ` Andy Shevchenko
2019-09-05 10:19       ` Andy Shevchenko
2019-09-05 10:40       ` Sakari Ailus
2019-09-05 10:40         ` Sakari Ailus
2019-09-05 10:40         ` Sakari Ailus
2019-09-05 10:57         ` Javier Martinez Canillas [this message]
2019-09-05 10:57           ` Javier Martinez Canillas
2019-09-05 11:36           ` Andy Shevchenko
2019-09-05 11:36             ` Andy Shevchenko
2019-09-05 11:36             ` Andy Shevchenko
2019-09-05  8:28   ` Tomasz Figa
2019-09-05  8:28     ` Tomasz Figa
2019-09-05  8:28     ` Tomasz Figa
2019-09-05 10:26   ` Andy Shevchenko
2019-09-05 10:26     ` Andy Shevchenko
2019-09-05 10:26     ` Andy Shevchenko
     [not found]     ` <e8b59857e39744a6acfe5d862f3ac8d5@mtkmbs05n2.mediatek.inc>
2020-01-20  8:34       ` Dongchun Zhu
2020-01-20  8:34         ` Dongchun Zhu
2020-01-20  8:34         ` Dongchun Zhu
2019-09-07 22:12   ` kbuild test robot
2019-09-07 22:12     ` kbuild test robot
2019-09-07 22:12     ` kbuild test robot
     [not found]   ` <20190905072142.14606-3-dongchun.zhu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2019-10-09  4:40     ` Tomasz Figa
2019-10-09  4:40       ` Tomasz Figa
2019-10-09  4:40       ` Tomasz Figa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad357e27-3e51-6922-1924-5d2c2daf1934@redhat.com \
    --to=javierm@redhat.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bingbu.cao@intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dongchun.zhu@mediatek.com \
    --cc=drinkcat@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=sam.hung@mediatek.com \
    --cc=shengnan.wang@mediatek.com \
    --cc=sj.huang@mediatek.com \
    --cc=srv_heupstream@mediatek.com \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.