[PATCH] Asus T300CHI.

[PATCH] Asus T300CHI.

[Mike Mestnik]

Hello,
  I'm looking to know the result of adding ACPI support for a new
tablet, the existing support shouldn't work because of a misplaced
__init that causes the function to be removed prior to being called.
After applying this patch I feel that I've moved support for the
T300CHI backwards.  What should be the intended effect of using the
ACPI interface?

Index: linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
===================================================================
--- linux-4.1.3.orig/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -23,27 +23,44 @@
 enum inv_mpu_product_name {
  INV_MPU_NOT_MATCHED,
  INV_MPU_ASUS_T100TA,
+ INV_MPU_ASUS_T300CHI,
 };

 static enum inv_mpu_product_name matched_product_name;

-static int __init asus_t100_matched(const struct dmi_system_id *d)
+static int asus_t100ta_matched(const struct dmi_system_id *d)
 {
  matched_product_name = INV_MPU_ASUS_T100TA;

  return 0;
 }

+static int asus_t300chi_matched(const struct dmi_system_id *d)
+{
+ matched_product_name = INV_MPU_ASUS_T300CHI;
+
+ return 0;
+}
+
 static const struct dmi_system_id inv_mpu_dev_list[] = {
  {
- .callback = asus_t100_matched,
- .ident = "Asus Transformer Book T100",
+ .callback = asus_t100ta_matched,
+ .ident = "Asus Transformer Book T100TA",
  .matches = {
  DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
  DMI_MATCH(DMI_PRODUCT_NAME, "T100TA"),
  DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
  },
  },
+ {
+ .callback = asus_t300chi_matched,
+ .ident = "Asus Transformer Book T300CHI",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "T300CHI"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
+ },
+ },
  /* Add more matching tables here..*/
  {}
 };
@@ -154,6 +171,7 @@ int inv_mpu_acpi_create_mux_client(struc
  dmi_check_system(inv_mpu_dev_list);
  switch (matched_product_name) {
  case INV_MPU_ASUS_T100TA:
+ case INV_MPU_ASUS_T300CHI:
  ret = asus_acpi_get_sensor_info(adev, st->client,
  &info);
  break;

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

Screen rotation for this device?  Howto wanted.

Re: [PATCH] Asus T300CHI.

[Daniel Baluta]

On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <cheako@mikemestnik.net> wrote:
> Hello,
>   I'm looking to know the result of adding ACPI support for a new
> tablet, the existing support shouldn't work because of a misplaced
> __init that causes the function to be removed prior to being called.

Are you sure about this? It seems that the existing support doesn't work
because you have different product ids.


> After applying this patch I feel that I've moved support for the
> T300CHI backwards.  What should be the intended effect of using the
> ACPI interface?
>
> Index: linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> ===================================================================
> --- linux-4.1.3.orig/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> +++ linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> @@ -23,27 +23,44 @@
>  enum inv_mpu_product_name {
>   INV_MPU_NOT_MATCHED,
>   INV_MPU_ASUS_T100TA,
> + INV_MPU_ASUS_T300CHI,
>  };
>
>  static enum inv_mpu_product_name matched_product_name;
>
> -static int __init asus_t100_matched(const struct dmi_system_id *d)
> +static int asus_t100ta_matched(const struct dmi_system_id *d)
>  {
>   matched_product_name = INV_MPU_ASUS_T100TA;
>
>   return 0;
>  }
>
> +static int asus_t300chi_matched(const struct dmi_system_id *d)
> +{
> + matched_product_name = INV_MPU_ASUS_T300CHI;
> +
> + return 0;
> +}
> +
>  static const struct dmi_system_id inv_mpu_dev_list[] = {
>   {
> - .callback = asus_t100_matched,
> - .ident = "Asus Transformer Book T100",
> + .callback = asus_t100ta_matched,
> + .ident = "Asus Transformer Book T100TA",
>   .matches = {
>   DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
>   DMI_MATCH(DMI_PRODUCT_NAME, "T100TA"),
>   DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
>   },
>   },
> + {
> + .callback = asus_t300chi_matched,
> + .ident = "Asus Transformer Book T300CHI",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "T300CHI"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
> + },
> + },
>   /* Add more matching tables here..*/
>   {}
>  };
> @@ -154,6 +171,7 @@ int inv_mpu_acpi_create_mux_client(struc
>   dmi_check_system(inv_mpu_dev_list);
>   switch (matched_product_name) {
>   case INV_MPU_ASUS_T100TA:
> + case INV_MPU_ASUS_T300CHI:
>   ret = asus_acpi_get_sensor_info(adev, st->client,
>   &info);
>   break;

This looks good to me. Care to send a formal patch?

thanks,
Daniel.

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <daniel.baluta@gmail.com> wrote:
> On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <cheako@mikemestnik.net> wrote:
>> Hello,
>>   I'm looking to know the result of adding ACPI support for a new
>> tablet, the existing support shouldn't work because of a misplaced
>> __init that causes the function to be removed prior to being called.
>
> Are you sure about this? It seems that the existing support doesn't work
> because you have different product ids.
>
The driver worked much better prior to me adding the product ids.  The
sensors were exposed to sysfs and all the data they collected seemed
correct to me.  The big issue is that there is no software, even
iio-sensor-proxy didn't know how to access the data.

>
>> After applying this patch I feel that I've moved support for the
>> T300CHI backwards.  What should be the intended effect of using the
>> ACPI interface?
>>
>> Index: linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> ===================================================================
>> --- linux-4.1.3.orig/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> +++ linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>> @@ -23,27 +23,44 @@
>>  enum inv_mpu_product_name {
>>   INV_MPU_NOT_MATCHED,
>>   INV_MPU_ASUS_T100TA,
>> + INV_MPU_ASUS_T300CHI,
>>  };
>>
>>  static enum inv_mpu_product_name matched_product_name;
>>
>> -static int __init asus_t100_matched(const struct dmi_system_id *d)
>> +static int asus_t100ta_matched(const struct dmi_system_id *d)
>>  {
>>   matched_product_name = INV_MPU_ASUS_T100TA;
>>
>>   return 0;
>>  }
>>
>> +static int asus_t300chi_matched(const struct dmi_system_id *d)
>> +{
>> + matched_product_name = INV_MPU_ASUS_T300CHI;
>> +
>> + return 0;
>> +}
>> +
>>  static const struct dmi_system_id inv_mpu_dev_list[] = {
>>   {
>> - .callback = asus_t100_matched,
>> - .ident = "Asus Transformer Book T100",
>> + .callback = asus_t100ta_matched,
>> + .ident = "Asus Transformer Book T100TA",
>>   .matches = {
>>   DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
>>   DMI_MATCH(DMI_PRODUCT_NAME, "T100TA"),
>>   DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
>>   },
>>   },
>> + {
>> + .callback = asus_t300chi_matched,
>> + .ident = "Asus Transformer Book T300CHI",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "T300CHI"),
>> + DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
>> + },
>> + },
>>   /* Add more matching tables here..*/
>>   {}
>>  };
>> @@ -154,6 +171,7 @@ int inv_mpu_acpi_create_mux_client(struc
>>   dmi_check_system(inv_mpu_dev_list);
>>   switch (matched_product_name) {
>>   case INV_MPU_ASUS_T100TA:
>> + case INV_MPU_ASUS_T300CHI:
>>   ret = asus_acpi_get_sensor_info(adev, st->client,
>>   &info);
>>   break;
>
> This looks good to me. Care to send a formal patch?
>
I would if the changes do what they are intended to do.  With this
patch applied there is no longer entries for the device in sysfs and
iio-sensor-proxy is still unable to access the device.  I'm wondering
how I'd be able to know if this change worked?

> thanks,
> Daniel.

Re: [PATCH] Asus T300CHI.

[Jonathan Cameron]

On 19/09/15 20:03, Mike Mestnik wrote:
> On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <daniel.baluta@gmail.com> wrote:
>> On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <cheako@mikemestnik.net> wrote:
>>> Hello,
>>>   I'm looking to know the result of adding ACPI support for a new
>>> tablet, the existing support shouldn't work because of a misplaced
>>> __init that causes the function to be removed prior to being called.
>>
>> Are you sure about this? It seems that the existing support doesn't work
>> because you have different product ids.
>>
> The driver worked much better prior to me adding the product ids.  The
> sensors were exposed to sysfs and all the data they collected seemed
> correct to me.  The big issue is that there is no software, even
> iio-sensor-proxy didn't know how to access the data.
Cc'd Bastien Nocera.
> 
>>
>>> After applying this patch I feel that I've moved support for the
>>> T300CHI backwards.  What should be the intended effect of using the
>>> ACPI interface?
>>>
>>> Index: linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> ===================================================================
>>> --- linux-4.1.3.orig/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> +++ linux-4.1.3/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
>>> @@ -23,27 +23,44 @@
>>>  enum inv_mpu_product_name {
>>>   INV_MPU_NOT_MATCHED,
>>>   INV_MPU_ASUS_T100TA,
>>> + INV_MPU_ASUS_T300CHI,
>>>  };
>>>
>>>  static enum inv_mpu_product_name matched_product_name;
>>>
>>> -static int __init asus_t100_matched(const struct dmi_system_id *d)
>>> +static int asus_t100ta_matched(const struct dmi_system_id *d)
>>>  {
>>>   matched_product_name = INV_MPU_ASUS_T100TA;
>>>
>>>   return 0;
>>>  }
>>>
>>> +static int asus_t300chi_matched(const struct dmi_system_id *d)
>>> +{
>>> + matched_product_name = INV_MPU_ASUS_T300CHI;
>>> +
>>> + return 0;
>>> +}
>>> +
>>>  static const struct dmi_system_id inv_mpu_dev_list[] = {
>>>   {
>>> - .callback = asus_t100_matched,
>>> - .ident = "Asus Transformer Book T100",
>>> + .callback = asus_t100ta_matched,
>>> + .ident = "Asus Transformer Book T100TA",
>>>   .matches = {
>>>   DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
>>>   DMI_MATCH(DMI_PRODUCT_NAME, "T100TA"),
>>>   DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
>>>   },
>>>   },
>>> + {
>>> + .callback = asus_t300chi_matched,
>>> + .ident = "Asus Transformer Book T300CHI",
>>> + .matches = {
>>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC"),
>>> + DMI_MATCH(DMI_PRODUCT_NAME, "T300CHI"),
>>> + DMI_MATCH(DMI_PRODUCT_VERSION, "1.0"),
>>> + },
>>> + },
>>>   /* Add more matching tables here..*/
>>>   {}
>>>  };
>>> @@ -154,6 +171,7 @@ int inv_mpu_acpi_create_mux_client(struc
>>>   dmi_check_system(inv_mpu_dev_list);
>>>   switch (matched_product_name) {
>>>   case INV_MPU_ASUS_T100TA:
>>> + case INV_MPU_ASUS_T300CHI:
>>>   ret = asus_acpi_get_sensor_info(adev, st->client,
>>>   &info);
>>>   break;
>>
>> This looks good to me. Care to send a formal patch?
>>
> I would if the changes do what they are intended to do.  With this
> patch applied there is no longer entries for the device in sysfs and
> iio-sensor-proxy is still unable to access the device.  I'm wondering
> how I'd be able to know if this change worked?
> 
>> thanks,
>> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Asus T300CHI.

[Bastien Nocera]

On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
> On 19/09/15 20:03, Mike Mestnik wrote:
> > On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
> > daniel.baluta@gmail.com> wrote:
> > > On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
> > > cheako@mikemestnik.net> wrote:
> > > > Hello,
> > > >   I'm looking to know the result of adding ACPI support for a
> > > > new
> > > > tablet, the existing support shouldn't work because of a
> > > > misplaced
> > > > __init that causes the function to be removed prior to being
> > > > called.
> > > 
> > > Are you sure about this? It seems that the existing support
> > > doesn't work
> > > because you have different product ids.
> > > 
> > The driver worked much better prior to me adding the product ids. 
> >  The
> > sensors were exposed to sysfs and all the data they collected
> > seemed
> > correct to me.  The big issue is that there is no software, even
> > iio-sensor-proxy didn't know how to access the data.
> Cc'd Bastien Nocera.

iio-sensor-proxy not finding the sensor, and with it working otherwise,
would be an iio-sensor-proxy bug. I have one of those already for the
accelerometer in the WinBook TW100 that I haven't had time to root down
though. See:
https://github.com/hadess/iio-sensor-proxy/issues/39

The main problem being that sensor types are already hard to detect,
and the iio subsystem doesn't make it any easier to check whether
there's buffered output available, or the application needs to poll.

If anyone wants to fix that in the kernel, that would certainly make my
life easier.

Cheers

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net> wrote:
>
> On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
> > On 19/09/15 20:03, Mike Mestnik wrote:
> > > On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
> > > daniel.baluta@gmail.com> wrote:
> > > > On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
> > > > cheako@mikemestnik.net> wrote:
> > > > > Hello,
> > > > >   I'm looking to know the result of adding ACPI support for a
> > > > > new
> > > > > tablet, the existing support shouldn't work because of a
> > > > > misplaced
> > > > > __init that causes the function to be removed prior to being
> > > > > called.
> > > >
> > > > Are you sure about this? It seems that the existing support
> > > > doesn't work
> > > > because you have different product ids.
> > > >
> > > The driver worked much better prior to me adding the product ids.
> > >  The
> > > sensors were exposed to sysfs and all the data they collected
> > > seemed
> > > correct to me.  The big issue is that there is no software, even
> > > iio-sensor-proxy didn't know how to access the data.
> > Cc'd Bastien Nocera.
>
> iio-sensor-proxy not finding the sensor, and with it working otherwise,
> would be an iio-sensor-proxy bug. I have one of those already for the
> accelerometer in the WinBook TW100 that I haven't had time to root down
> though. See:
> https://github.com/hadess/iio-sensor-proxy/issues/39
>
> The main problem being that sensor types are already hard to detect,
> and the iio subsystem doesn't make it any easier to check whether
> there's buffered output available, or the application needs to poll.
>
> If anyone wants to fix that in the kernel, that would certainly make my
> life easier.
>
I only kind of understand the reasoning for iio, but for the case of
devices used for instructing application behaviour I can't help but think
this would be better served using/extending input events.

That would seam to cover the above issues, in exchange for having to add
support for a few new tricks.

1. An axis where max and min are actually one 'step' apart.
2. Double axis for xyz, 6 instead of 3.  Perhaps this can be handled with
extra event nodes, but then there may be sync issues.
3. Integrating iio data into input events.  I don't even understand what
this would mean.

> Cheers

[-- Attachment #2: Type: text/html, Size: 3203 bytes --]

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net> wrote:
>
> On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
> > On 19/09/15 20:03, Mike Mestnik wrote:
> > > On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
> > > daniel.baluta@gmail.com> wrote:
> > > > On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
> > > > cheako@mikemestnik.net> wrote:
> > > > > Hello,
> > > > >   I'm looking to know the result of adding ACPI support for a
> > > > > new
> > > > > tablet, the existing support shouldn't work because of a
> > > > > misplaced
> > > > > __init that causes the function to be removed prior to being
> > > > > called.
> > > >
> > > > Are you sure about this? It seems that the existing support
> > > > doesn't work
> > > > because you have different product ids.
> > > >
> > > The driver worked much better prior to me adding the product ids.
> > >  The
> > > sensors were exposed to sysfs and all the data they collected
> > > seemed
> > > correct to me.  The big issue is that there is no software, even
> > > iio-sensor-proxy didn't know how to access the data.
> > Cc'd Bastien Nocera.
>
> iio-sensor-proxy not finding the sensor, and with it working otherwise,
> would be an iio-sensor-proxy bug. I have one of those already for the
> accelerometer in the WinBook TW100 that I haven't had time to root down
> though. See:
> https://github.com/hadess/iio-sensor-proxy/issues/39
>
> The main problem being that sensor types are already hard to detect,
> and the iio subsystem doesn't make it any easier to check whether
> there's buffered output available, or the application needs to poll.
>
> If anyone wants to fix that in the kernel, that would certainly make my
> life easier.
>
I only kind of understand the reasoning for iio, but for the case of
devices used for instructing application behaviour I can't help but think
this would be better served using/extending input events.

That would seam to cover the above issues, in exchange for having to add
support for a few new tricks.

1. An axis where max and min are actually one 'step' apart.
2. Double axis for xyz, 6 instead of 3.  Perhaps this can be handled with
extra event nodes, but then there may be sync issues.
3. Integrating iio data into input events.  I don't even understand what
this would mean.

> Cheers

Re: [PATCH] Asus T300CHI.

[Jonathan Cameron]

On 21/09/15 15:26, Mike Mestnik wrote:
> On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net> wrote:
>>
>> On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
>>> On 19/09/15 20:03, Mike Mestnik wrote:
>>>> On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
>>>> daniel.baluta@gmail.com> wrote:
>>>>> On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
>>>>> cheako@mikemestnik.net> wrote:
>>>>>> Hello,
>>>>>>   I'm looking to know the result of adding ACPI support for a
>>>>>> new
>>>>>> tablet, the existing support shouldn't work because of a
>>>>>> misplaced
>>>>>> __init that causes the function to be removed prior to being
>>>>>> called.
>>>>>
>>>>> Are you sure about this? It seems that the existing support
>>>>> doesn't work
>>>>> because you have different product ids.
>>>>>
>>>> The driver worked much better prior to me adding the product ids.
>>>>  The
>>>> sensors were exposed to sysfs and all the data they collected
>>>> seemed
>>>> correct to me.  The big issue is that there is no software, even
>>>> iio-sensor-proxy didn't know how to access the data.
>>> Cc'd Bastien Nocera.
>>
>> iio-sensor-proxy not finding the sensor, and with it working otherwise,
>> would be an iio-sensor-proxy bug. I have one of those already for the
>> accelerometer in the WinBook TW100 that I haven't had time to root down
>> though. See:
>> https://github.com/hadess/iio-sensor-proxy/issues/39
>>
>> The main problem being that sensor types are already hard to detect,
>> and the iio subsystem doesn't make it any easier to check whether
>> there's buffered output available, or the application needs to poll.
I dispute this one.  It's not exactly hard to check for the buffer
directory in sysfs and to evaluate if there is a trigger provided by
the device (again a simple directory presence check).
It gets harder if there is more than one trigger provided, but inherently
there isn't much we can do to suggest the best one.  If there are two
then there are two usecases that demand different choices.

>>
>> If anyone wants to fix that in the kernel, that would certainly make my
>> life easier.
>>
> I only kind of understand the reasoning for iio, but for the case of
> devices used for instructing application behaviour I can't help but think
> this would be better served using/extending input events.
Indeed, the IIO side of things is about providing generic support
for lots of use cases whereas in your case you are looking at something
which is naturally quite specific. 

> 
> That would seam to cover the above issues, in exchange for having to add
> support for a few new tricks.
> 
> 1. An axis where max and min are actually one 'step' apart.
> 2. Double axis for xyz, 6 instead of 3.  Perhaps this can be handled with
> extra event nodes, but then there may be sync issues.
> 3. Integrating iio data into input events.  I don't even understand what
> this would mean.

This last one exists, but is out of tree at the moment waiting for a better
way to instantiate iio-input bridge instances.
Historically we got a lot of grief over the iio-hwmon bridge device tree
bindings - they seemed fine at the time when none of us knew much about
device tree ;)
The plan for bridging to input is to do it via configfs once Daniel Baluta's
had a chance to revise his patches to add support for configfs to IIO. 
We'll probably also support this approach for the hwmon bridge as well
and deprecate the current approach. The bridge is imaginatively called iio-input
if you want to google it (quite a few years old as this binding issue wasn't
an easy one to solve and when written it was more of a hyperthetical use case
than one anyone was actually using).

Jonathan
> 
>> Cheers
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

On Mon, Sep 21, 2015 at 2:46 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 21/09/15 15:26, Mike Mestnik wrote:
>> On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net> wrote:
>>>
>>> On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
>>>> On 19/09/15 20:03, Mike Mestnik wrote:
>>>>> On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
>>>>> daniel.baluta@gmail.com> wrote:
>>>>>> On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
>>>>>> cheako@mikemestnik.net> wrote:
>>>>>>> Hello,
>>>>>>>   I'm looking to know the result of adding ACPI support for a
>>>>>>> new
>>>>>>> tablet, the existing support shouldn't work because of a
>>>>>>> misplaced
>>>>>>> __init that causes the function to be removed prior to being
>>>>>>> called.
>>>>>>
>>>>>> Are you sure about this? It seems that the existing support
>>>>>> doesn't work
>>>>>> because you have different product ids.
>>>>>>
>>>>> The driver worked much better prior to me adding the product ids.
>>>>>  The
>>>>> sensors were exposed to sysfs and all the data they collected
>>>>> seemed
>>>>> correct to me.  The big issue is that there is no software, even
>>>>> iio-sensor-proxy didn't know how to access the data.
>>>> Cc'd Bastien Nocera.
>>>
>>> iio-sensor-proxy not finding the sensor, and with it working otherwise,
>>> would be an iio-sensor-proxy bug. I have one of those already for the
>>> accelerometer in the WinBook TW100 that I haven't had time to root down
>>> though. See:
>>> https://github.com/hadess/iio-sensor-proxy/issues/39
>>>
>>> The main problem being that sensor types are already hard to detect,
>>> and the iio subsystem doesn't make it any easier to check whether
>>> there's buffered output available, or the application needs to poll.
> I dispute this one.  It's not exactly hard to check for the buffer
> directory in sysfs and to evaluate if there is a trigger provided by
> the device (again a simple directory presence check).
> It gets harder if there is more than one trigger provided, but inherently
> there isn't much we can do to suggest the best one.  If there are two
> then there are two usecases that demand different choices.
>
>>>
>>> If anyone wants to fix that in the kernel, that would certainly make my
>>> life easier.
>>>
>> I only kind of understand the reasoning for iio, but for the case of
>> devices used for instructing application behaviour I can't help but think
>> this would be better served using/extending input events.
> Indeed, the IIO side of things is about providing generic support
> for lots of use cases whereas in your case you are looking at something
> which is naturally quite specific.
>
I just found a copy of this driver in the input tree:
drivers/input/misc/mpu3050.c

Perhaps "indexing" drivers by userspace API is the wrong approach as
it's logical that a device my be accessible from userspace via
multiple APIs and applicable for lots of reasons.  Splinting drivers
into directories because of how userspace communicates with the driver
is just asking for duplication or problems with bringing in the
appropriate code.

>>
>> That would seam to cover the above issues, in exchange for having to add
>> support for a few new tricks.
>>
>> 1. An axis where max and min are actually one 'step' apart.
>> 2. Double axis for xyz, 6 instead of 3.  Perhaps this can be handled with
>> extra event nodes, but then there may be sync issues.

At first glance it seems that these are still outstanding, needing a
proposed userspace API.

Re: [PATCH] Asus T300CHI.

[Srinivas Pandruvada]

On Thu, 2015-09-24 at 10:57 -0500, Mike Mestnik wrote:
> On Mon, Sep 21, 2015 at 2:46 PM, Jonathan Cameron <jic23@kernel.org>
> wrote:
> > On 21/09/15 15:26, Mike Mestnik wrote:
> > > On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net>
> > > wrote:
> > > > 
> > > > On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
> > > > > On 19/09/15 20:03, Mike Mestnik wrote:
> > > > > > On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
> > > > > > daniel.baluta@gmail.com> wrote:
> > > > > > > On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
> > > > > > > cheako@mikemestnik.net> wrote:
> > > > > > > > Hello,
> > > > > > > >   I'm looking to know the result of adding ACPI support
> > > > > > > > for a
> > > > > > > > new
> > > > > > > > tablet, the existing support shouldn't work because of
> > > > > > > > a
> > > > > > > > misplaced
> > > > > > > > __init that causes the function to be removed prior to
> > > > > > > > being
> > > > > > > > called.
> > > > > > > 
> > > > > > > Are you sure about this? It seems that the existing
> > > > > > > support
> > > > > > > doesn't work
> > > > > > > because you have different product ids.
> > > > > > > 
> > > > > > The driver worked much better prior to me adding the
> > > > > > product ids.
> > > > > >  The
> > > > > > sensors were exposed to sysfs and all the data they
> > > > > > collected
> > > > > > seemed
> > > > > > correct to me.  The big issue is that there is no software,
> > > > > > even
> > > > > > iio-sensor-proxy didn't know how to access the data.
> > > > > Cc'd Bastien Nocera.
> > > > 
> > > > iio-sensor-proxy not finding the sensor, and with it working
> > > > otherwise,
> > > > would be an iio-sensor-proxy bug. I have one of those already
> > > > for the
> > > > accelerometer in the WinBook TW100 that I haven't had time to
> > > > root down
> > > > though. See:
> > > > https://github.com/hadess/iio-sensor-proxy/issues/39
> > > > 
> > > > The main problem being that sensor types are already hard to
> > > > detect,
> > > > and the iio subsystem doesn't make it any easier to check
> > > > whether
> > > > there's buffered output available, or the application needs to
> > > > poll.
> > I dispute this one.  It's not exactly hard to check for the buffer
> > directory in sysfs and to evaluate if there is a trigger provided
> > by
> > the device (again a simple directory presence check).
> > It gets harder if there is more than one trigger provided, but
> > inherently
> > there isn't much we can do to suggest the best one.  If there are
> > two
> > then there are two usecases that demand different choices.
> > 
> > > > 
> > > > If anyone wants to fix that in the kernel, that would certainly
> > > > make my
> > > > life easier.
> > > > 
> > > I only kind of understand the reasoning for iio, but for the case
> > > of
> > > devices used for instructing application behaviour I can't help
> > > but think
> > > this would be better served using/extending input events.
> > Indeed, the IIO side of things is about providing generic support
> > for lots of use cases whereas in your case you are looking at
> > something
> > which is naturally quite specific.
> > 
> I just found a copy of this driver in the input tree:
> drivers/input/misc/mpu3050.c
This was done in 2011 time frame, when IIO was still in staging, I
guess.
Using input driver is not good idea as we need some special sysfs
entries for control.
There was some proposal for IIO input driver bridge.

Thanks,
Srinivas

> Perhaps "indexing" drivers by userspace API is the wrong approach as
> it's logical that a device my be accessible from userspace via
> multiple APIs and applicable for lots of reasons.  Splinting drivers
> into directories because of how userspace communicates with the
> driver
> is just asking for duplication or problems with bringing in the
> appropriate code.
> 
> > > 
> > > That would seam to cover the above issues, in exchange for having
> > > to add
> > > support for a few new tricks.
> > > 
> > > 1. An axis where max and min are actually one 'step' apart.
> > > 2. Double axis for xyz, 6 instead of 3.  Perhaps this can be
> > > handled with
> > > extra event nodes, but then there may be sync issues.
> 
> At first glance it seems that these are still outstanding, needing a
> proposed userspace API.

Re: [PATCH] Asus T300CHI.

[Mike Mestnik]

On Fri, Sep 25, 2015 at 9:19 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Thu, 2015-09-24 at 10:57 -0500, Mike Mestnik wrote:
>> On Mon, Sep 21, 2015 at 2:46 PM, Jonathan Cameron <jic23@kernel.org>
>> wrote:
>> > On 21/09/15 15:26, Mike Mestnik wrote:
>> > > On Sep 20, 2015 5:42 PM, "Bastien Nocera" <hadess@hadess.net>
>> > > wrote:
>> > > >
>> > > > On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
>> > > > > On 19/09/15 20:03, Mike Mestnik wrote:
>> > > > > > On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
>> > > > > > daniel.baluta@gmail.com> wrote:
>> > > > > > > On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
>> > > > > > > cheako@mikemestnik.net> wrote:
>> > > > > > > > Hello,
>> > > > > > > >   I'm looking to know the result of adding ACPI support
>> > > > > > > > for a
>> > > > > > > > new
>> > > > > > > > tablet, the existing support shouldn't work because of
>> > > > > > > > a
>> > > > > > > > misplaced
>> > > > > > > > __init that causes the function to be removed prior to
>> > > > > > > > being
>> > > > > > > > called.
>> > > > > > >
>> > > > > > > Are you sure about this? It seems that the existing
>> > > > > > > support
>> > > > > > > doesn't work
>> > > > > > > because you have different product ids.
>> > > > > > >
>> > > > > > The driver worked much better prior to me adding the
>> > > > > > product ids.
>> > > > > >  The
>> > > > > > sensors were exposed to sysfs and all the data they
>> > > > > > collected
>> > > > > > seemed
>> > > > > > correct to me.  The big issue is that there is no software,
>> > > > > > even
>> > > > > > iio-sensor-proxy didn't know how to access the data.
>> > > > > Cc'd Bastien Nocera.
>> > > >
>> > > > iio-sensor-proxy not finding the sensor, and with it working
>> > > > otherwise,
>> > > > would be an iio-sensor-proxy bug. I have one of those already
>> > > > for the
>> > > > accelerometer in the WinBook TW100 that I haven't had time to
>> > > > root down
>> > > > though. See:
>> > > > https://github.com/hadess/iio-sensor-proxy/issues/39
>> > > >
>> > > > The main problem being that sensor types are already hard to
>> > > > detect,
>> > > > and the iio subsystem doesn't make it any easier to check
>> > > > whether
>> > > > there's buffered output available, or the application needs to
>> > > > poll.
>> > I dispute this one.  It's not exactly hard to check for the buffer
>> > directory in sysfs and to evaluate if there is a trigger provided
>> > by
>> > the device (again a simple directory presence check).
>> > It gets harder if there is more than one trigger provided, but
>> > inherently
>> > there isn't much we can do to suggest the best one.  If there are
>> > two
>> > then there are two usecases that demand different choices.
>> >
>> > > >
>> > > > If anyone wants to fix that in the kernel, that would certainly
>> > > > make my
>> > > > life easier.
>> > > >
>> > > I only kind of understand the reasoning for iio, but for the case
>> > > of
>> > > devices used for instructing application behaviour I can't help
>> > > but think
>> > > this would be better served using/extending input events.
>> > Indeed, the IIO side of things is about providing generic support
>> > for lots of use cases whereas in your case you are looking at
>> > something
>> > which is naturally quite specific.
>> >
>> I just found a copy of this driver in the input tree:
>> drivers/input/misc/mpu3050.c
> This was done in 2011 time frame, when IIO was still in staging, I
> guess.
> Using input driver is not good idea as we need some special sysfs
> entries for control.
It has a configurable trigger and several power levels depending on
the events needing to be generated.

> There was some proposal for IIO input driver bridge.
>
Testing for this driver isn't as promising as the iio.  I'm getting a
better feel for talking to this device and implementing the hardware
side of a driver.  Though I've no direction for implementing the
userspace api, see below.

> Thanks,
> Srinivas
>
>> Perhaps "indexing" drivers by userspace API is the wrong approach as
>> it's logical that a device my be accessible from userspace via
>> multiple APIs and applicable for lots of reasons.  Splinting drivers
>> into directories because of how userspace communicates with the
>> driver
>> is just asking for duplication or problems with bringing in the
>> appropriate code.
>>
>> > >
>> > > That would seam to cover the above issues, in exchange for having
>> > > to add
>> > > support for a few new tricks.
>> > >
>> > > 1. An axis where max and min are actually one 'step' apart.
>> > > 2. Double axis for xyz, 6 instead of 3.  Perhaps this can be
>> > > handled with
>> > > extra event nodes, but then there may be sync issues.
>>
>> At first glance it seems that these are still outstanding, needing a
>> proposed userspace API.

Re: [PATCH] Asus T300CHI.

[Jonathan Cameron]

On 20/09/15 23:42, Bastien Nocera wrote:
> On Sun, 2015-09-20 at 20:08 +0100, Jonathan Cameron wrote:
>> On 19/09/15 20:03, Mike Mestnik wrote:
>>> On Sat, Sep 19, 2015 at 1:48 PM, Daniel Baluta <
>>> daniel.baluta@gmail.com> wrote:
>>>> On Fri, Sep 18, 2015 at 12:07 AM, Mike Mestnik <
>>>> cheako@mikemestnik.net> wrote:
>>>>> Hello,
>>>>>   I'm looking to know the result of adding ACPI support for a
>>>>> new
>>>>> tablet, the existing support shouldn't work because of a
>>>>> misplaced
>>>>> __init that causes the function to be removed prior to being
>>>>> called.
>>>>
>>>> Are you sure about this? It seems that the existing support
>>>> doesn't work
>>>> because you have different product ids.
>>>>
>>> The driver worked much better prior to me adding the product ids. 
>>>  The
>>> sensors were exposed to sysfs and all the data they collected
>>> seemed
>>> correct to me.  The big issue is that there is no software, even
>>> iio-sensor-proxy didn't know how to access the data.
>> Cc'd Bastien Nocera.
> 
> iio-sensor-proxy not finding the sensor, and with it working otherwise,
> would be an iio-sensor-proxy bug. I have one of those already for the
> accelerometer in the WinBook TW100 that I haven't had time to root down
> though. See:
> https://github.com/hadess/iio-sensor-proxy/issues/39
> 
> The main problem being that sensor types are already hard to detect,
> and the iio subsystem doesn't make it any easier to check whether
> there's buffered output available, or the application needs to poll.
> 
> If anyone wants to fix that in the kernel, that would certainly make my
> life easier.
> 
Suggestions for improvements welcome, but take into account that I for
one am unconvinced that a bit of parsing for presence of files is 
bad enough to demand another ABI to read them same things.

Jonathan
> Cheers
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>