diff mbox

[v2] HID: i2c-hid: add ACPI support

Message ID 1357742588-11366-1-git-send-email-mika.westerberg@linux.intel.com (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

Mika Westerberg Jan. 9, 2013, 2:43 p.m. UTC
The HID over I2C protocol specification states that when the device is
enumerated from ACPI the HID descriptor address can be obtained by
executing "_DSM" for the device with function 1. Enable this.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes to previous version:

	* platform data is embedded into struct i2c_hid
	* i2c_hid_acpi_pdata() returns error codes or 0 on success
	* i2c_hid_acpi_pdata() prints out errors if _DSM execution fails or
	  returns something we don't understand
	* constify i2c_hid_acpi_match
	* set the hid device ACPI handle

We still don't handle the GpioInt in case client->irq is not set but I
didn't want to implement it now as I'm unable to test it. So this only
works if the ACPI device has an Interrupt or IRQ resource set.

 drivers/hid/i2c-hid/i2c-hid.c |   87 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 81 insertions(+), 6 deletions(-)

Comments

Benjamin Tissoires Jan. 9, 2013, 2:49 p.m. UTC | #1
Hi Mika,

On Wed, Jan 9, 2013 at 3:43 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> The HID over I2C protocol specification states that when the device is
> enumerated from ACPI the HID descriptor address can be obtained by
> executing "_DSM" for the device with function 1. Enable this.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I'm eager to test it (hopefully next week).

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks,
Benjamin

> ---
> Changes to previous version:
>
>         * platform data is embedded into struct i2c_hid
>         * i2c_hid_acpi_pdata() returns error codes or 0 on success
>         * i2c_hid_acpi_pdata() prints out errors if _DSM execution fails or
>           returns something we don't understand
>         * constify i2c_hid_acpi_match
>         * set the hid device ACPI handle
>
> We still don't handle the GpioInt in case client->irq is not set but I
> didn't want to implement it now as I'm unable to test it. So this only
> works if the ACPI device has an Interrupt or IRQ resource set.
>
>  drivers/hid/i2c-hid/i2c-hid.c |   87 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 81 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 9ef22244..9e46e42 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -34,6 +34,7 @@
>  #include <linux/kernel.h>
>  #include <linux/hid.h>
>  #include <linux/mutex.h>
> +#include <linux/acpi.h>
>
>  #include <linux/i2c/i2c-hid.h>
>
> @@ -139,6 +140,8 @@ struct i2c_hid {
>         unsigned long           flags;          /* device flags */
>
>         wait_queue_head_t       wait;           /* For waiting the interrupt */
> +
> +       struct i2c_hid_platform_data pdata;
>  };
>
>  static int __i2c_hid_command(struct i2c_client *client,
> @@ -810,6 +813,70 @@ static int __devinit i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid)
>         return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +static int i2c_hid_acpi_pdata(struct i2c_client *client,
> +               struct i2c_hid_platform_data *pdata)
> +{
> +       static u8 i2c_hid_guid[] = {
> +               0xF7, 0xF6, 0xDF, 0x3C, 0x67, 0x42, 0x55, 0x45,
> +               0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE,
> +       };
> +       struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> +       union acpi_object params[4], *obj;
> +       struct acpi_object_list input;
> +       struct acpi_device *adev;
> +       acpi_handle handle;
> +
> +       handle = ACPI_HANDLE(&client->dev);
> +       if (!handle || acpi_bus_get_device(handle, &adev))
> +               return -ENODEV;
> +
> +       input.count = ARRAY_SIZE(params);
> +       input.pointer = params;
> +
> +       params[0].type = ACPI_TYPE_BUFFER;
> +       params[0].buffer.length = sizeof(i2c_hid_guid);
> +       params[0].buffer.pointer = i2c_hid_guid;
> +       params[1].type = ACPI_TYPE_INTEGER;
> +       params[1].integer.value = 1;
> +       params[2].type = ACPI_TYPE_INTEGER;
> +       params[2].integer.value = 1; /* HID function */
> +       params[3].type = ACPI_TYPE_INTEGER;
> +       params[3].integer.value = 0;
> +
> +       if (ACPI_FAILURE(acpi_evaluate_object(handle, "_DSM", &input, &buf))) {
> +               dev_err(&client->dev, "device _DSM execution failed\n");
> +               return -ENODEV;
> +       }
> +
> +       obj = (union acpi_object *)buf.pointer;
> +       if (obj->type != ACPI_TYPE_INTEGER) {
> +               dev_err(&client->dev, "device _DSM returned invalid type: %d\n",
> +                       obj->type);
> +               kfree(buf.pointer);
> +               return -EINVAL;
> +       }
> +
> +       pdata->hid_descriptor_address = obj->integer.value;
> +
> +       kfree(buf.pointer);
> +       return 0;
> +}
> +
> +static const struct acpi_device_id i2c_hid_acpi_match[] = {
> +       {"ACPI0C50", 0 },
> +       {"PNP0C50", 0 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(acpi, i2c_hid_acpi_match);
> +#else
> +static inline int i2c_hid_acpi_pdata(struct i2c_client *client,
> +               struct i2c_hid_platform_data *pdata)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +
>  static int __devinit i2c_hid_probe(struct i2c_client *client,
>                 const struct i2c_device_id *dev_id)
>  {
> @@ -821,11 +888,6 @@ static int __devinit i2c_hid_probe(struct i2c_client *client,
>
>         dbg_hid("HID probe called for i2c 0x%02x\n", client->addr);
>
> -       if (!platform_data) {
> -               dev_err(&client->dev, "HID register address not provided\n");
> -               return -EINVAL;
> -       }
> -
>         if (!client->irq) {
>                 dev_err(&client->dev,
>                         "HID over i2c has not been provided an Int IRQ\n");
> @@ -836,11 +898,22 @@ static int __devinit i2c_hid_probe(struct i2c_client *client,
>         if (!ihid)
>                 return -ENOMEM;
>
> +       if (!platform_data) {
> +               ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
> +               if (ret) {
> +                       dev_err(&client->dev,
> +                               "HID register address not provided\n");
> +                       goto err;
> +               }
> +       } else {
> +               ihid->pdata = *platform_data;
> +       }
> +
>         i2c_set_clientdata(client, ihid);
>
>         ihid->client = client;
>
> -       hidRegister = platform_data->hid_descriptor_address;
> +       hidRegister = ihid->pdata.hid_descriptor_address;
>         ihid->wHIDDescRegister = cpu_to_le16(hidRegister);
>
>         init_waitqueue_head(&ihid->wait);
> @@ -873,6 +946,7 @@ static int __devinit i2c_hid_probe(struct i2c_client *client,
>         hid->hid_get_raw_report = i2c_hid_get_raw_report;
>         hid->hid_output_raw_report = i2c_hid_output_raw_report;
>         hid->dev.parent = &client->dev;
> +       ACPI_HANDLE_SET(&hid->dev, ACPI_HANDLE(&client->dev));
>         hid->bus = BUS_I2C;
>         hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
>         hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
> @@ -964,6 +1038,7 @@ static struct i2c_driver i2c_hid_driver = {
>                 .name   = "i2c_hid",
>                 .owner  = THIS_MODULE,
>                 .pm     = &i2c_hid_pm,
> +               .acpi_match_table = ACPI_PTR(i2c_hid_acpi_match),
>         },
>
>         .probe          = i2c_hid_probe,
> --
> 1.7.10.4
>
--
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
Jiri Kosina Jan. 9, 2013, 2:52 p.m. UTC | #2
On Wed, 9 Jan 2013, Benjamin Tissoires wrote:

> <mika.westerberg@linux.intel.com> wrote:
> > The HID over I2C protocol specification states that when the device is
> > enumerated from ACPI the HID descriptor address can be obtained by
> > executing "_DSM" for the device with function 1. Enable this.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> I'm eager to test it (hopefully next week).
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Perfect. I will wait for your Tested-by: as well before applying to public 
git tree.

Thanks to both of you,
Benjamin Tissoires Jan. 17, 2013, 9:52 p.m. UTC | #3
On Wed, Jan 9, 2013 at 9:52 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 9 Jan 2013, Benjamin Tissoires wrote:
>
>> <mika.westerberg@linux.intel.com> wrote:
>> > The HID over I2C protocol specification states that when the device is
>> > enumerated from ACPI the HID descriptor address can be obtained by
>> > executing "_DSM" for the device with function 1. Enable this.
>> >
>> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>
>> I'm eager to test it (hopefully next week).
>>
>> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Perfect. I will wait for your Tested-by: as well before applying to public
> git tree.

Hi Jiri,

well, we tried yesterday to make all those things working, but the
Haswell platform I was able to have access was not very cooperative.
So I wasn't able to test the whole autodetection mechanism through
ACPI (I even not managed to communicate with the various HID over i2c
devices that were available to test).

However, Mika managed to test this ACPI stuff, and he is able to
retrieve the HID descriptor address. Given that, I think it's safe to
include it right now. There will still be the last point with the
gpios, but it will come when someone get access to a device presenting
this feature.

Cheers,
Benjamin

>
> Thanks to both of you,
>
> --
> Jiri Kosina
> SUSE Labs
--
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
Jiri Kosina Jan. 18, 2013, 10:42 a.m. UTC | #4
On Thu, 17 Jan 2013, Benjamin Tissoires wrote:

> >> > The HID over I2C protocol specification states that when the device is
> >> > enumerated from ACPI the HID descriptor address can be obtained by
> >> > executing "_DSM" for the device with function 1. Enable this.
> >> >
> >> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>
> >> I'm eager to test it (hopefully next week).
> >>
> >> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > Perfect. I will wait for your Tested-by: as well before applying to public
> > git tree.
> 
> Hi Jiri,
> 
> well, we tried yesterday to make all those things working, but the
> Haswell platform I was able to have access was not very cooperative.
> So I wasn't able to test the whole autodetection mechanism through
> ACPI (I even not managed to communicate with the various HID over i2c
> devices that were available to test).
> 
> However, Mika managed to test this ACPI stuff, and he is able to
> retrieve the HID descriptor address. Given that, I think it's safe to
> include it right now. There will still be the last point with the
> gpios, but it will come when someone get access to a device presenting
> this feature.

Thanks a lot for the update. I have now applied Mika's patch.
diff mbox

Patch

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 9ef22244..9e46e42 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -34,6 +34,7 @@ 
 #include <linux/kernel.h>
 #include <linux/hid.h>
 #include <linux/mutex.h>
+#include <linux/acpi.h>
 
 #include <linux/i2c/i2c-hid.h>
 
@@ -139,6 +140,8 @@  struct i2c_hid {
 	unsigned long		flags;		/* device flags */
 
 	wait_queue_head_t	wait;		/* For waiting the interrupt */
+
+	struct i2c_hid_platform_data pdata;
 };
 
 static int __i2c_hid_command(struct i2c_client *client,
@@ -810,6 +813,70 @@  static int __devinit i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+static int i2c_hid_acpi_pdata(struct i2c_client *client,
+		struct i2c_hid_platform_data *pdata)
+{
+	static u8 i2c_hid_guid[] = {
+		0xF7, 0xF6, 0xDF, 0x3C, 0x67, 0x42, 0x55, 0x45,
+		0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE,
+	};
+	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object params[4], *obj;
+	struct acpi_object_list input;
+	struct acpi_device *adev;
+	acpi_handle handle;
+
+	handle = ACPI_HANDLE(&client->dev);
+	if (!handle || acpi_bus_get_device(handle, &adev))
+		return -ENODEV;
+
+	input.count = ARRAY_SIZE(params);
+	input.pointer = params;
+
+	params[0].type = ACPI_TYPE_BUFFER;
+	params[0].buffer.length = sizeof(i2c_hid_guid);
+	params[0].buffer.pointer = i2c_hid_guid;
+	params[1].type = ACPI_TYPE_INTEGER;
+	params[1].integer.value = 1;
+	params[2].type = ACPI_TYPE_INTEGER;
+	params[2].integer.value = 1; /* HID function */
+	params[3].type = ACPI_TYPE_INTEGER;
+	params[3].integer.value = 0;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_DSM", &input, &buf))) {
+		dev_err(&client->dev, "device _DSM execution failed\n");
+		return -ENODEV;
+	}
+
+	obj = (union acpi_object *)buf.pointer;
+	if (obj->type != ACPI_TYPE_INTEGER) {
+		dev_err(&client->dev, "device _DSM returned invalid type: %d\n",
+			obj->type);
+		kfree(buf.pointer);
+		return -EINVAL;
+	}
+
+	pdata->hid_descriptor_address = obj->integer.value;
+
+	kfree(buf.pointer);
+	return 0;
+}
+
+static const struct acpi_device_id i2c_hid_acpi_match[] = {
+	{"ACPI0C50", 0 },
+	{"PNP0C50", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, i2c_hid_acpi_match);
+#else
+static inline int i2c_hid_acpi_pdata(struct i2c_client *client,
+		struct i2c_hid_platform_data *pdata)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __devinit i2c_hid_probe(struct i2c_client *client,
 		const struct i2c_device_id *dev_id)
 {
@@ -821,11 +888,6 @@  static int __devinit i2c_hid_probe(struct i2c_client *client,
 
 	dbg_hid("HID probe called for i2c 0x%02x\n", client->addr);
 
-	if (!platform_data) {
-		dev_err(&client->dev, "HID register address not provided\n");
-		return -EINVAL;
-	}
-
 	if (!client->irq) {
 		dev_err(&client->dev,
 			"HID over i2c has not been provided an Int IRQ\n");
@@ -836,11 +898,22 @@  static int __devinit i2c_hid_probe(struct i2c_client *client,
 	if (!ihid)
 		return -ENOMEM;
 
+	if (!platform_data) {
+		ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
+		if (ret) {
+			dev_err(&client->dev,
+				"HID register address not provided\n");
+			goto err;
+		}
+	} else {
+		ihid->pdata = *platform_data;
+	}
+
 	i2c_set_clientdata(client, ihid);
 
 	ihid->client = client;
 
-	hidRegister = platform_data->hid_descriptor_address;
+	hidRegister = ihid->pdata.hid_descriptor_address;
 	ihid->wHIDDescRegister = cpu_to_le16(hidRegister);
 
 	init_waitqueue_head(&ihid->wait);
@@ -873,6 +946,7 @@  static int __devinit i2c_hid_probe(struct i2c_client *client,
 	hid->hid_get_raw_report = i2c_hid_get_raw_report;
 	hid->hid_output_raw_report = i2c_hid_output_raw_report;
 	hid->dev.parent = &client->dev;
+	ACPI_HANDLE_SET(&hid->dev, ACPI_HANDLE(&client->dev));
 	hid->bus = BUS_I2C;
 	hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
 	hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
@@ -964,6 +1038,7 @@  static struct i2c_driver i2c_hid_driver = {
 		.name	= "i2c_hid",
 		.owner	= THIS_MODULE,
 		.pm	= &i2c_hid_pm,
+		.acpi_match_table = ACPI_PTR(i2c_hid_acpi_match),
 	},
 
 	.probe		= i2c_hid_probe,