Re: All device tree bindings need maintainer ack or just the 'interesting' ones.

On Wed, 18 Sep 2013 14:58:04 +0100, Mark Rutland [off-list ref] wrote:

On Tue, Sep 17, 2013 at 02:48:41PM +0100, Lee Jones wrote:

quoted

On Tue, 17 Sep 2013, Mark Rutland wrote:

quoted

On Tue, Sep 17, 2013 at 01:07:30PM +0100, Lee Jones wrote:

quoted

On Tue, 17 Sep 2013, Mark Rutland wrote:

quoted

On Tue, Sep 17, 2013 at 09:10:03AM +0100, Lee Jones wrote:

quoted

quoted

quoted

http://marc.info/?l=linux-iio&m=137881790217809&w=2

Looks reasonable. My only comment is '-' is preferred over '_'.

Ah, in the node name you mean. Yes, I can change that.

The compatible string has to stay the same however, as it has to match
the device name which can't be changed due to possible userspace
breakage.

I'm not sure I follow why the compatible string must be identical to the
name appearing to userspace.

It doesn't. It has to be identical to the device name, which I'm
guessing is used when populating sysfs.

I still don't understand why the device name must be identical to the
compatible string.

The string "st,lsm303dlh_magn" (with the "st," prefix) appears nowhere
in v3.12-rc1

You're looking at an old patchset. We found out that the Snowball
schematic was actually incorrect and we have a LSM303DLHC.

git grep -i lsm303dlhc_magn v3.12-rc1
  drivers/iio/magnetometer/st_magn.h:
    #define LSM303DLHC_MAGN_DEV_NAME   "lsm303dlhc_magn"

quoted

and the compatible string in the dt doesn't seem to get
assigned to the device in the i2c core or in
drivers/iio/magnetometer/st_magn_i2c.c.

It does:
  drivers/i2c/busses/*.c: of_i2c_register_devices();
  drivers/of/of_i2c.c:    |-> of_modalias_node();
                            |-> i2c_new_device();

Ah, I see.

I thought the return of of_modalias_node was only used to *guess* the
module which might need to be loaded, rather than also being exposed to
userspace as the device name.

We've got a leak of Linux implementation details into the dt here, and I
don't like it. Thanks to the combination of of_i2c_register device's use
of of_modalias_node, and i2c_device_match using the id table after
failing to use dt, any driver with a i2c_device_id name implicitly
defines a dt binding for "${ARBITRARY_PREFIX},name". The same is true
for SPI drivers using spi_device_id. This turns what looks like a Linux
internal value into an ABI facing bootloader software.

Judging by a quick and dirty grep [1], as of v3.12-rc1 there are ~930
suffixes which *may* be accepted with an arbitrary prefix as an I2C
compatible string, of which ~750 do not appear in
Documentation/devicetree. For SPI there are just over 400, of which just
under 400 aren't documented. Both these counts include drivers that may
never be built for DT platforms, as I've not filtered the results.

Some drivers have an of_match_table (e.g. i2c-hid has a single
"hid-over-i2c" entry), but if the I2C core fails to match in the
of_match_table it'll still carry on and match the stripped compatible
against the i2c_device_id table (where it'll happily match
"${VENDOR},hid"). I think if a driver has an of_match_table and we fail
to match with of_match_device we should fail rather than falling back to
the id table.

In the i2c-hid case, I can't see how the "hid-over-i2c" variant can ever
work -- of_i2c_register_devices will skip over devices where the
compatible string doesn't contain a ','. So our documented compatible
string isn't usable, while an undocumented one is...

I do appologies for the i2c of_modalias_node mess. At the time is was a
quick hack to make it each to bind nodes to i2c devices without changing
the drivers. At the time there wasn't much support for device tree. Now
it is just painfull.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html