On Sat, Sep 21, 2013 at 03:57:42PM +0100, Jonathan Cameron wrote: > On 09/18/13 16:11, Grant Likely wrote: > > On Wed, 18 Sep 2013 14:58:04 +0100, Mark Rutland <mark.rutland@xxxxxxx> wrote: > >> On Tue, Sep 17, 2013 at 02:48:41PM +0100, Lee Jones wrote: > >>> On Tue, 17 Sep 2013, Mark Rutland wrote: > >>> > >>>> On Tue, Sep 17, 2013 at 01:07:30PM +0100, Lee Jones wrote: > >>>>> On Tue, 17 Sep 2013, Mark Rutland wrote: > >>>>> > >>>>>> On Tue, Sep 17, 2013 at 09:10:03AM +0100, Lee Jones wrote: > >>>>>>>>> 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" > >>> > >>>> 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. > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
