| Message ID | 20171002213215.32201-1-rajatja@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
+ Cc: Jarkko, he spent a lot of nice hours to debug i2c HID touchscreen devices on x86 ACPI enabled platforms, so, he might have a better idea or some comments. On Mon, 2017-10-02 at 14:32 -0700, Rajat Jain wrote: > The property "post-power-on-delay-ms"" allows a platform to specify > the delay needed after power-on, but only via device trees. Thus > allow ACPI systems to also provide the same information. This one is even less acceptable (in given form), see below why. > + if (!device_property_read_u32(&client->dev, "post-power-on- > delay-ms", > + &val)) The main idea behind unified device properties API is to provide a way which will be resource provider agnostic, i.e. callers will get data in some kind of generic way independently on the source of it. Since I2C HID protocol is well defined by Microsoft and it doesn't involve _DSD, you make here even more gnostic solution. Besides the fact, each property must be registered in Device Tree bindings (yes, even if it's going to be used for ACPI enabled platforms initially). Thus, _if_ (and only if) we have no other solution, you need to clean up your first version and send it as v3. Don't forget to add a version to the patch (git-format-patch has a command line option to make this simpler).
On Tue, Oct 3, 2017 at 2:28 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > + Cc: Jarkko, he spent a lot of nice hours to debug i2c HID touchscreen > devices on x86 ACPI enabled platforms, so, he might have a better idea > or some comments. Thanks Andy, I'll look out for any suggestions on inputs from Jarkko. > > On Mon, 2017-10-02 at 14:32 -0700, Rajat Jain wrote: >> The property "post-power-on-delay-ms"" allows a platform to specify >> the delay needed after power-on, but only via device trees. Thus >> allow ACPI systems to also provide the same information. > > This one is even less acceptable (in given form), see below why. > >> + if (!device_property_read_u32(&client->dev, "post-power-on- >> delay-ms", >> + &val)) > > The main idea behind unified device properties API is to provide a way > which will be resource provider agnostic, i.e. callers will get data in > some kind of generic way independently on the source of it. > > Since I2C HID protocol is well defined by Microsoft and it doesn't > involve _DSD, you make here even more gnostic solution. Agree. Since I don't understand the HID protocol (or _DSD for that matter) that well, and hence I want to solve my problem without disturbing any other code. I've sent yet another iteration (v3) that introduces a new function to parse the common properties, please let me know if this is any good. > > Besides the fact, each property must be registered in Device Tree > bindings (yes, even if it's going to be used for ACPI enabled platforms > initially). The property was already documented, however, thanks for reminding, I cleaned up the doc to indicate that regulator property is not a requirement for the delay property. Thanks & Best Regards, Rajat > > Thus, _if_ (and only if) we have no other solution, you need to clean up > your first version and send it as v3. > > Don't forget to add a version to the patch (git-format-patch has a > command line option to make this simpler). > > -- > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 77396145d2d0..97405156315a 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -865,6 +865,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, union acpi_object *obj; struct acpi_device *adev; acpi_handle handle; + u32 val; handle = ACPI_HANDLE(&client->dev); if (!handle || acpi_bus_get_device(handle, &adev)) @@ -880,6 +881,10 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, pdata->hid_descriptor_address = obj->integer.value; ACPI_FREE(obj); + if (!device_property_read_u32(&client->dev, "post-power-on-delay-ms", + &val)) + pdata->post_power_delay_ms = val; + return 0; }
The property "post-power-on-delay-ms"" allows a platform to specify the delay needed after power-on, but only via device trees. Thus allow ACPI systems to also provide the same information. Signed-off-by: Rajat Jain <rajatja@google.com> --- drivers/hid/i2c-hid/i2c-hid.c | 5 +++++ 1 file changed, 5 insertions(+)