On Mon, Dec 12, 2016 at 4:01 AM, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote: > On Dec 12 2016 or thereabouts, Jiri Kosina wrote: >> Given the timing (merge window being open) and given then NACK given by >> Rob, I've now unapplied the patches (the for-4.10/i2c-hid branch is now >> obsolete, and has been superseded by for-4.10/i2c-hid-nopower). >> >> However, this is mostly done in order to provide more time for discussion; >> I still disagree with the reasoning behind the NACK. >> > > To hopefully make things going forward a little bit, I was wondering > over the week-end if we should not solve this particular issue by adding > an intermediate platform DT node: > > instead of having: > --- > i2c-hid-dev@2c { > compatible = "hid-over-i2c"; > reg = <0x2c>; > hid-descr-addr = <0x0020>; > interrupt-parent = <&gpx3>; > interrupts = <3 2>; > vdd-supply = <sth>; > init-delay-ms = <100>; > }; > --- > > we would have: > --- > platform-i2c-hid@01 { > compatible = "very-special-board-that-needs-firmware-quirks-and-delay-of-100ms"; > vdd-supply = <sth>; > i2c-hid-dev@2c { > compatible = "hid-over-i2c"; > reg = <0x2c>; > hid-descr-addr = <0x0020>; > interrupt-parent = <&gpx3>; > interrupts = <3 2>; > }; > }; > --- > > If I am not wrong, the platform device should be initialized before > i2c-hid get called, which allows to setup properly the vdd supply. > On resume/suspend, the tree should be respected and we should be able to > enable/disable power in the same fashion this patch provides. > > We could then extend this platform device at will without tinkering in > i2c-hid and we could also handle the GPIOs, reset or whatever is > required in the future through compatibles. > > Thoughts? yes? no? bullshit? That is structuring the DT match your driver structure, not what the h/w looks like. This would make sense if the device was multi-function of which HID-over-I2C was one function. You could do what you describe with a single node and the platform driver creates the hid-over-i2c device. DT is not the only way to create devices. Anyway, we're only debating this: i2c-hid-dev@2c { compatible = "wacom,w9013", "hid-over-i2c"; reg = <0x2c>; hid-descr-addr = <0x0020>; interrupt-parent = <&gpx3>; interrupts = <3 2>; vdd-supply = <sth>; init-delay-ms = <100>; }; vs. i2c-hid-dev@2c { compatible = "hid-over-i2c"; reg = <0x2c>; hid-descr-addr = <0x0020>; interrupt-parent = <&gpx3>; interrupts = <3 2>; vdd-supply = <sth>; init-delay-ms = <100>; }; My only other nit is use "post-power-on-delay-ms" which is already a defined property name rather than "init-delay-ms". Rob -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html