On Tue, Sep 21, 2021 at 2:35 PM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > Hi Ping, > > On Fri, Sep 17, 2021 at 01:43:18PM -0700, Ping Cheng wrote: > > Hi Dmitry, > > > > On Tue, Sep 7, 2021 at 10:55 PM Dmitry Torokhov > > <dmitry.torokhov@xxxxxxxxx> wrote: > > > > > > > > > > > > > > > Yes, our firmware supports HID over I2C. However, some of our > > > > > > customers often do not want to use HID to handle our hardware; even > > > > > > they don't install the generic HID driver neither. In such case, we > > > > > > need to distinguish what generation of our device customer's has. And > > > > > > to do so, we check I2C HID descriptor even though the driver is not > > > > > > working with HID driver components, but this one. That is why I2C HID > > > > > > descriptor is used there. It is called, but the situation with this > > > > > > driver is not supposed to work as a HID device. > > > > > > > > > > I would like to understand better why the customers do not want to use > > > > > HID. > > > > > > > > > > > > Those customers normally run embedded Linux. Their hardwares have very > > > > specific use cases. They don't need to support any other HID devices except > > > > the Wacom i2c device. > > > > > > > > > > > > > There needs to be a _very_ strong reason to essentially duplicate > > > > > HID layer in a vendor driver and I inclined to say that such customers > > > > > > > > would need to patch their kernels themselves. > > > > > > > > They most likely don't want to duplicate HID layer. They just don't need > > > > most of the HID layer code. > > > > > > They just need touchscreen support. Plus stylus support. And maybe > > > battery support. And maybe something else down the road... And they need > > > to introduce DT and ACPI descriptors to be able to mould the behavior to > > > platform needs. Which is pretty much the purpose of HID layer. > > > > I see your point. > > > > > > wacom_i2c simplifies their deployment and > > > > testing process. Most of those customers are very small companies... > > > > > > And now please continue this train of thoughts and consider every touch > > > vendor. Wacom is not unique. We have Elan, Cypress, Weida, Goodix, etc. > > > etc. Vendor drivers were acceptable before we had I2C standard, but now > > > it is much better for everyone to share the efforts and use HID instead > > > of replicating it for every vendor. > > > > And I agree with you that we should share our efforts on the main tasks. > > > > However, with the same token of sharing efforts, I see the benefit of > > merging this set of patches upstream. From the version number we can > > tell the patchset has gone through at least 10 rounds of review and > > update. Alistair has put a lot of effort to get this far (Thank you > > Alistair for your time and effort!). > > I am sorry, but the fact that a developer spent a lot of time writing > code can not be used as a justification for merging said code. > > > A few community developers have > > also reviewed the patches. This set of patches thoroughly touched all > > parts of the components that related to an input i2c driver, which is > > much better than the original version. This patchset would be a great > > starting point for vendors to create their out of tree drivers, when > > necessary. It would also offer vendors a clear picture of what > > components they need to change/update to make their i2c input device > > work under kernel input subsystem. > > And they can do that by looking at the mailing list archive and they can > also follow this discussion and see why what they are doing is quite > wrong. I mean, why would they stop at dropping HID only? Why not bypass > I2C system as well and poke at the I2C controller directly. Skip PCI as > well? > > > > > So, merging the patchset will benefit more people and preserve the > > effort that went into the patchset so far. If you like, you can add a > > comment in the patch mentioning that future effort should be directed > > to the i2c-hid subsystem, etc... > > No, we should not merge the patch set that we agreed is wrong in its > approach. Maybe if I (and other community reviewers) realized that the > device was HID compliant we could stop this effort earlier, but > unfortunately it did not happen, so effort was wasted, but this happens > sometimes. Should I prepare a patch to remove the wacom I2C driver entirely then? Alistair > > Thanks. > > -- > Dmitry
