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

DISCUSSION Jonathan Cameron 7 participants 12 messages Sep 14, 2013 – Oct 8, 2013 7 messages not in the archive

lore

Share & tools

lore

Jonathan Cameron

author

2013-09-14 12:07 UTC #0 lore raw

Hi All,

I'm getting a lot of IIO related device tree bindings at the moment. Most are pretty trivial e.g.

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

I know you are probably still working out how the device tree maintainership is going to work in the long run, but in the meantime do you want me to hold these on an Ack from one of the maintainers and/or to ensure that they are all posted to [email protected]?

Really the question is do you want to be completely treated like any other subsystem, where, if a driver touches it, in how ever trivial a fashion, the assumption is it would be unwise to not attempt to get an Ack from the relevant maintainer.

There are plenty of more 'interesting' IIO binding questions out there and obviously those need the input from the list and maintainers!

In the meantime, we'll send everything to devicetree-u79uwXL29TaiAVqoAR/[email protected]

Jonathan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Rob Herring

2013-09-16 16:17 UTC #2 lore raw

Quoted 6 lines · Jonathan Cameron

Hi All,
I'm getting a lot of IIO related device tree bindings at the moment. Most are pretty trivial e.g.
http://marc.info/?l=linux-iio&m=137881790217809&w=2

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

Quoted 4 lines

I know you are probably still working out how the device tree maintainership is going to work in the long run, but in the meantime do you want me to hold these on an Ack from one of the maintainers and/or to ensure that they are all posted to [email protected]?

I think if it is something self-contained to a node using existing binding definitions and typically just a new compatible string like the above case, then they don't need an ack. The main requirement in this case is that compatible strings and other properties are documented. For new or changes to common bindings, then they should be acked. If in doubt, you can always require an ack on a case by case basis.

Quoted 8 lines

Really the question is do you want to be completely treated like any other subsystem, where, if a driver touches it, in how ever trivial a fashion, the assumption is it would be unwise to not attempt to get an Ack from the relevant maintainer.
There are plenty of more 'interesting' IIO binding questions out there and obviously those need the input from the list and maintainers!
In the meantime, we'll send everything to devicetree-u79uwXL29TaiAVqoAR/[email protected]

That should always be the case whether we require an ack or not.

Rob

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Lee Jones

2013-09-17 08:10 UTC #4 lore raw

Quoted 3 lines

Quoted 1 line
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.

Signature · 8 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Rutland

2013-09-17 11:24 UTC #5 lore raw

Quoted 9 lines · Lee Jones

Quoted 3 lines
Quoted 1 line
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.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20130917112433.GB26737-NuALmloUBlrZROr8t4l/[email protected]

Lee Jones

2013-09-17 12:07 UTC #7 lore raw

Quoted 5 lines · Mark Rutland

Quoted 4 lines · Lee Jones
Quoted 3 lines
Quoted 1 line
http://marc.info/?l=linux-iio&m=137881790217809&w=2
Looks reasonable. My only comment is '-' is preferred over '_'.

Quoted 8 lines

Quoted 5 lines
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.

Signature · 8 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Rutland

2013-09-17 13:30 UTC #8 lore raw

Quoted 13 lines · Lee Jones

Quoted 11 lines · Mark Rutland
Quoted 9 lines · Lee Jones
Quoted 3 lines
Quoted 1 line
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.

Quoted 5 lines

Quoted 2 lines
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, 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.

I don't see how the "st,lsm303dlh_magn" string could be getting to userspace already such that it could be relying on it.

Am I missing something?

Thanks,
Mark.

Quoted 7 lines

Signature · 5 lines

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20130917133025.GD26737-NuALmloUBlrZROr8t4l/[email protected]

Lee Jones

2013-09-17 13:48 UTC #10 lore raw

Quoted 17 lines · Mark Rutland

Quoted 16 lines · Lee Jones
Quoted 13 lines · Mark Rutland
Quoted 9 lines · Lee Jones
Quoted 3 lines
Quoted 1 line
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.

Quoted 8 lines

Quoted 2 lines
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 3 lines

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();

Quoted 9 lines

I don't see how the "st,lsm303dlh_magn" string could be getting to userspace already such that it could be relying on it.
Am I missing something?
Thanks,
Mark.
Quoted 1 line

Signature · 8 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Rutland

2013-09-18 13:58 UTC #11 lore raw

Quoted 35 lines · Lee Jones

Quoted 17 lines · Mark Rutland
Quoted 16 lines · Lee Jones
Quoted 13 lines · Mark Rutland
Quoted 9 lines · Lee Jones
Quoted 3 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 2 lines
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 8 lines

Quoted 3 lines
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...

We don't appear to have any clashes yet (with "vendor-a,device" and "vendor-b,device"), so maybe that's not an issue -- we can add of_match_table entries as required to disambiguate, though we have to be careful to capture all the prefixes already in use.

The current use of of_modalias_node also only allows for the first entry in the compatible list to be used to request a compatible module. I think it would be better for the DT core to request modules somehow when iterating over compatible lists, but I'm not familiar with MODULE_DEVICE_TABLE() and friends, so maybe that's not as easy as it sounds. That doesn't solve exporting a sensible modalias to userspace, but it would somewhat decouple the compatible list and modaliases, and allow for fallback entries in the compatible list to load modules if required.

I see that some macintosh drivers have i2c_device_id values in device tree style (e.g. "MAC,fcu"), without an of_match_table. Assuming that "MAC,fcu" is the full string rather than "AAPL,MAC,fcu", I don't see how those get matched to drivers. The I2C core will drop the leading "MAC,", leaving only "fcu" in the i2c_board_info, and then that won't match any driver modaliases. I'm probably missing something here.

Thanks,
Mark.

[1] $ git grep -hW i2c_device_id.\*= -- drivers | grep {.\*} |

      grep -o "\"[^\"]\+\"" | grep -o "[^\"]\+" | sort -u |
      xargs -I '{}' sh -c 'git grep -q "{}" -- Documentation/devicetree ||
      echo Undocumented binding \"*,{}\"'

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20130918135803.GG26737-NuALmloUBlrZROr8t4l/[email protected]

Mark Brown

2013-09-18 15:05 UTC #13 lore raw

On Wed, Sep 18, 2013 at 02:58:04PM +0100, Mark Rutland wrote:

[Devices matching the Linux internal devices name as a compatible value]

Quoted 6 lines

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.

Personally I've always viewed this as being an attempt to be liberal in what we accept with regard to buggy device trees rather than anything else (and I do push back on DT bindings that don't include setting a proper match table up). For all practical purposes it should generally come up with the right answer.

Quoted 4 lines

We don't appear to have any clashes yet (with "vendor-a,device" and "vendor-b,device"), so maybe that's not an issue -- we can add of_match_table entries as required to disambiguate, though we have to be careful to capture all the prefixes already in use.

There are some chip naming clashes out there - both Wondermedia and Wolfson use WMxxxx and there are reused numbers for example - but it's not common and generally bus types will disambiguate).

Grant Likely

2013-09-18 15:11 UTC #14 lore raw

In reply to a message not in the archive

Quoted 74 lines · Mark Rutland

Quoted 35 lines · Lee Jones
Quoted 17 lines · Mark Rutland
Quoted 16 lines · Lee Jones
Quoted 13 lines · Mark Rutland
Quoted 9 lines · Lee Jones
Quoted 3 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 2 lines
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 8 lines
Quoted 3 lines
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.

Quoted 4 lines

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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20130918151119.59DB1C42CDF-WNowdnHR2B42iJbIjFUEsiwD8/[email protected]

Jonathan Cameron

2013-09-21 14:57 UTC #16 lore raw

Quoted 67 lines · Grant Likely

Quoted 66 lines · Mark Rutland
Quoted 29 lines · Lee Jones
Quoted 13 lines · Mark Rutland
Quoted 12 lines · Lee Jones
Quoted 10 lines · Mark Rutland
Quoted 7 lines · Lee Jones
Quoted 2 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 2 lines
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 7 lines
Quoted 3 lines
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.

Quoted 8 lines

Quoted 4 lines
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.

My understanding from this discussion is that we don't really have a way to back out the existing naming (with _ instead of the preferred -) from these bindings without breaking existing ABI.

Hence, shall I apply the patches that at least document that the ABI we are stuck with? (e.g. http://www.spinics.net/lists/devicetree/msg04808.html
[PATCH 07/20] Documentation: dt: iio: Add binding for LPS001WP )

Jonathan

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 523DB3E6.3040804-tko9wxEg+fIOOJlXag/[email protected]

Mark Rutland

2013-10-08 08:47 UTC #18 lore raw

Quoted 76 lines · Jonathan Cameron

Quoted 67 lines · Grant Likely
Quoted 66 lines · Mark Rutland
Quoted 29 lines · Lee Jones
Quoted 13 lines · Mark Rutland
Quoted 12 lines · Lee Jones
Quoted 10 lines · Mark Rutland
Quoted 7 lines · Lee Jones
Quoted 2 lines
Quoted 1 line
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.
Quoted 7 lines
Quoted 2 lines
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 7 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 4 lines
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.

Quoted 7 lines

My understanding from this discussion is that we don't really have a way to back out the existing naming (with _ instead of the preferred -) from these bindings without breaking existing ABI.
Hence, shall I apply the patches that at least document that the ABI we are stuck with? (e.g. http://www.spinics.net/lists/devicetree/msg04808.html
[PATCH 07/20] Documentation: dt: iio: Add binding for LPS001WP )

It does seem like that's the only thing we can do here. Yes please.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html