lkml.org 
[lkml]   [2019]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH RFC] modpost: Support I2C Aliases from OF tables
    From
    Date
    Hello Kieran,

    On 7/10/19 9:39 PM, Kieran Bingham wrote:
    > I2C drivers match against an I2C ID table, an OF table, and an ACPI
    > table. It is now also possible to match against an OF table entry
    > without the vendor prefix to support backwards compatibility, and allow
    > simplification of the i2c probe functions.
    >
    > As part of this matching, the probe function is being converted to
    > remove the need to specify the i2c_device_id table, but to support
    > module aliasing, we still require to have the MODULE_DEVICE_TABLE entry.
    >

    My opinion on this is that I2C drivers should register the tables of the
    firmware interfaces that support. That is, if a driver is only used in a
    platform that supports OF then it should only require an OF device table.

    But if the driver supports devices that can also be present in platforms
    that use ACPI, then should also require to have an ACPI device ID table.

    So there should be consistency about what table is used for both matching
    a device with a driver and reporting a modalias to user-space for module
    auto-loading. If a I2C device was instantiated by OF, then the OF table
    should be used for both reporting a modalias uevent and matching a driver.

    Now, the i2c_of_match_device() function attempts to match by first calling
    of_match_device() and if fails fallbacks to i2c_of_match_device_sysfs().

    The latter attempts to match the I2C device by striping the vendor prefix
    of the compatible strings on the OF device ID table. That means that you
    could instantiate an I2C device ID through the sysfs interface and the OF
    table would be used to match the driver.

    But i2c_device_uevent() would had reported an I2C modalias and not an OF
    modalias (since the registered device won't have an associated of_node) so
    there's inconsistency in that case since a table is used to match but no
    to report modaliases.

    > Facilitate generating the I2C aliases directly from the of_device_id
    > tables, by stripping the vendor prefix prefix from the compatible string
    > and using that as an alias just as the i2c-core supports.
    >

    I see two ways to solve this issue. One is with $SUBJECT since we can argue
    that if a OF-only driver is able to match devices that were instantiated
    through the sysfs interface that only have a device name, then a modalias
    of the form i2c:<foo> is needed. Since the compatible strings without the
    vendor prefix is used to match, then I think that makes sense to also use
    them without the vendor prefix to populate I2C modaliases as $SUBJECT does.

    The other option is to remove i2c_of_match_device() and don't make OF match
    to fallback to i2c_of_match_device_sysfs(). This is what happens in the ACPI
    case, since i2c_device_match() just calls acpi_driver_match_device() directly
    and doesn't have a wrapper function that fallbacks to sysfs matching.

    In this case an I2C device ID table would be required if the devices have to
    be instantiated through sysfs. That way the I2C table would be used both for
    auto-loading and also to match the device when it doesn't have an of_node.

    If the former is the correct way to solve this then the patch looks good to me.

    Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

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

    \
     
     \ /
      Last update: 2019-07-22 15:04    [W:2.409 / U:0.816 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site