linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
@ 2019-04-02 16:04 Luís Ferreira
  2019-04-03  9:10 ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Luís Ferreira @ 2019-04-02 16:04 UTC (permalink / raw)
  To: jic23
  Cc: Luís Ferreira, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Hans de Goede, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
and KIOX020A sensor on keyboard base, to detect tablet mode or screen
orientation.

Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
---
 drivers/iio/accel/kxcjk-1013.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 7096e577b23f..9a5e445facc1 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
 	{"KIOX0009", KXTJ21009},
 	{"KIOX000A", KXCJ91008},
 	{"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
+	{"KIOX020A", KXCJ91008},
 	{"KXTJ1009", KXTJ21009},
 	{"KXJ2109",  KXTJ21009},
 	{"SMO8500",  KXCJ91008},
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-02 16:04 [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks Luís Ferreira
@ 2019-04-03  9:10 ` Hans de Goede
  2019-04-05 23:01   ` Luís Ferreira
  0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2019-04-03  9:10 UTC (permalink / raw)
  To: Luís Ferreira, jic23
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Stephan Gerhold, open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

On 02-04-19 18:04, Luís Ferreira wrote:
> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
> orientation.

I deliberately left out the KIOX020A id for now, because currently
userspace cannot really deal with having 2 sensors.

See:
https://github.com/systemd/systemd/issues/6557
https://github.com/hadess/iio-sensor-proxy/issues/166

Basically we need to come up with a convention to (optionally) indicate
the sensors location with a udev attribute set by:
/lib/udev/hwdb.d/60-sensor.hwdb

And then patch iio-sensor-proxy to consume that attribute and ignore
the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties

Ignoring would be a first step, maybe later it can do something useful
with it, see e.g. :  https://github.com/alesguzik/linux_detect_tablet_mode

IMHO we really should minimally get code in place for iio-sensor-proxy
to ignore the keyboard accelerometer before merging this patch.

I realize that having the code in place will not magically get it on
all users machines, but I believe this is the minimum which needs to
happen before we push this out and potentially breaks people screen
rotation.

I've had working on this on my TODO list for a long long time now,
but -ENOTIME. If you have some time to work on this then that would
be great.

Regards,

Hans




> 
> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
> ---
>   drivers/iio/accel/kxcjk-1013.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 7096e577b23f..9a5e445facc1 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>   	{"KIOX0009", KXTJ21009},
>   	{"KIOX000A", KXCJ91008},
>   	{"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
> +	{"KIOX020A", KXCJ91008},
>   	{"KXTJ1009", KXTJ21009},
>   	{"KXJ2109",  KXTJ21009},
>   	{"SMO8500",  KXCJ91008},
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-03  9:10 ` Hans de Goede
@ 2019-04-05 23:01   ` Luís Ferreira
  2019-04-06  9:36     ` Hans de Goede
  2019-04-15 15:40     ` luis
  0 siblings, 2 replies; 11+ messages in thread
From: Luís Ferreira @ 2019-04-05 23:01 UTC (permalink / raw)
  To: Hans de Goede
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

> Basically we need to come up with a convention to (optionally) indicate
> the sensors location with a udev attribute set by:
> /lib/udev/hwdb.d/60-sensor.hwdb

So should we start adding `ACCEL_LOCATION=display` and
`ACCEL_LOCATION=keyboard` attributes to that file and patch
iio-sensor-proxy to ignore the keyboard ones as a first step ?

Best Regards,
Luís

Sincerely,
Luís Ferreira
E2AB 2BF8 F3EC ABE2 E149 F2FD BC43 0831 9CE4 0B64 (plain text)



Aurora Free Open Source Software is a non-profit organization that
promote free and open source software. All applications, source code
and services are complete open and free to use according to a valid
Open Source license. Check out more at https://www.aurorafoss.org/ .

---
PRIVILEGED & CONFIDENTIAL COMMUNICATIONS
This email and all rights associated with it are the property of Luís Ferreira.
It contains material for which legal professional privilege is
claimed. The contents of this email may not be examined, removed or
reproduced without my express written permission by Luís Ferreira. The
information contained in this e-mail message and any accompanying
files is or may be confidential. It is intended only for the
addressee. If you are not the intended recipient, any use,
dissemination, reliance, forwarding, printing or copying of this
e-mail or any attached files is unauthorised. This e-mail is subject
to copyright. No part of it should be reproduced, adapted or
communicated without the written consent of the copyright owner. If
you have received this e-mail in error please advise the sender
immediately by return e-mail or telephone and delete all copies. Luís
Ferreira does not guarantee the accuracy or completeness of any
information contained in this e-mail or attached files. Internet
communications are not secure, therefore Luís Ferreira does not accept
legal responsibility for the contents of this message or attached
files.


On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 02-04-19 18:04, Luís Ferreira wrote:
> > Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
> > and KIOX020A sensor on keyboard base, to detect tablet mode or screen
> > orientation.
>
> I deliberately left out the KIOX020A id for now, because currently
> userspace cannot really deal with having 2 sensors.
>
> See:
> https://github.com/systemd/systemd/issues/6557
> https://github.com/hadess/iio-sensor-proxy/issues/166
>
> Basically we need to come up with a convention to (optionally) indicate
> the sensors location with a udev attribute set by:
> /lib/udev/hwdb.d/60-sensor.hwdb
>
> And then patch iio-sensor-proxy to consume that attribute and ignore
> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
>
> Ignoring would be a first step, maybe later it can do something useful
> with it, see e.g. :  https://github.com/alesguzik/linux_detect_tablet_mode
>
> IMHO we really should minimally get code in place for iio-sensor-proxy
> to ignore the keyboard accelerometer before merging this patch.
>
> I realize that having the code in place will not magically get it on
> all users machines, but I believe this is the minimum which needs to
> happen before we push this out and potentially breaks people screen
> rotation.
>
> I've had working on this on my TODO list for a long long time now,
> but -ENOTIME. If you have some time to work on this then that would
> be great.
>
> Regards,
>
> Hans
>
>
>
>
> >
> > Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
> > ---
> >   drivers/iio/accel/kxcjk-1013.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > index 7096e577b23f..9a5e445facc1 100644
> > --- a/drivers/iio/accel/kxcjk-1013.c
> > +++ b/drivers/iio/accel/kxcjk-1013.c
> > @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
> >       {"KIOX0009", KXTJ21009},
> >       {"KIOX000A", KXCJ91008},
> >       {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
> > +     {"KIOX020A", KXCJ91008},
> >       {"KXTJ1009", KXTJ21009},
> >       {"KXJ2109",  KXTJ21009},
> >       {"SMO8500",  KXCJ91008},
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-05 23:01   ` Luís Ferreira
@ 2019-04-06  9:36     ` Hans de Goede
  2019-04-15 15:40     ` luis
  1 sibling, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2019-04-06  9:36 UTC (permalink / raw)
  To: Luís Ferreira
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

On 06-04-19 01:01, Luís Ferreira wrote:
> Hi,
> 
>> Basically we need to come up with a convention to (optionally) indicate
>> the sensors location with a udev attribute set by:
>> /lib/udev/hwdb.d/60-sensor.hwdb
> 
> So should we start adding `ACCEL_LOCATION=display` and
> `ACCEL_LOCATION=keyboard` attributes to that file and patch
> iio-sensor-proxy to ignore the keyboard ones as a first step ?

Yes that seems the best way forward with this.

Note I think "base" is better then "keyboard" for the sensor which
is in the base/keyboard. But neither is perfect, so go which whatever
you prefer.

Thanks & Regards,

Hans



> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 02-04-19 18:04, Luís Ferreira wrote:
>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
>>> orientation.
>>
>> I deliberately left out the KIOX020A id for now, because currently
>> userspace cannot really deal with having 2 sensors.
>>
>> See:
>> https://github.com/systemd/systemd/issues/6557
>> https://github.com/hadess/iio-sensor-proxy/issues/166
>>
>> Basically we need to come up with a convention to (optionally) indicate
>> the sensors location with a udev attribute set by:
>> /lib/udev/hwdb.d/60-sensor.hwdb
>>
>> And then patch iio-sensor-proxy to consume that attribute and ignore
>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
>>
>> Ignoring would be a first step, maybe later it can do something useful
>> with it, see e.g. :  https://github.com/alesguzik/linux_detect_tablet_mode
>>
>> IMHO we really should minimally get code in place for iio-sensor-proxy
>> to ignore the keyboard accelerometer before merging this patch.
>>
>> I realize that having the code in place will not magically get it on
>> all users machines, but I believe this is the minimum which needs to
>> happen before we push this out and potentially breaks people screen
>> rotation.
>>
>> I've had working on this on my TODO list for a long long time now,
>> but -ENOTIME. If you have some time to work on this then that would
>> be great.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>>
>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
>>> ---
>>>    drivers/iio/accel/kxcjk-1013.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>>> index 7096e577b23f..9a5e445facc1 100644
>>> --- a/drivers/iio/accel/kxcjk-1013.c
>>> +++ b/drivers/iio/accel/kxcjk-1013.c
>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>>>        {"KIOX0009", KXTJ21009},
>>>        {"KIOX000A", KXCJ91008},
>>>        {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
>>> +     {"KIOX020A", KXCJ91008},
>>>        {"KXTJ1009", KXTJ21009},
>>>        {"KXJ2109",  KXTJ21009},
>>>        {"SMO8500",  KXCJ91008},
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-05 23:01   ` Luís Ferreira
  2019-04-06  9:36     ` Hans de Goede
@ 2019-04-15 15:40     ` luis
  2019-04-17 15:08       ` Hans de Goede
  1 sibling, 1 reply; 11+ messages in thread
From: luis @ 2019-04-15 15:40 UTC (permalink / raw)
  To: Hans de Goede
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:

Hi,

> Yes that seems the best way forward with this.
> 
> Note I think "base" is better then "keyboard" for the sensor which
> is in the base/keyboard. But neither is perfect, so go which whatever
> you prefer.

Reference to:
- https://github.com/hadess/iio-sensor-proxy/pull/262
- https://github.com/systemd/systemd/pull/12322

Regards,
Luís

> Hi,
> 
> On 06-04-19 01:01, Luís Ferreira wrote:
> 
>> Hi,
>> Basically we need to come up with a convention to (optionally) indicate
> 
> the sensors location with a udev attribute set by:
> /lib/udev/hwdb.d/60-sensor.hwdb
>> So should we start adding `ACCEL_LOCATION=display` and
>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
> 
> Yes that seems the best way forward with this.
> 
> Note I think "base" is better then "keyboard" for the sensor which
> is in the base/keyboard. But neither is perfect, so go which whatever
> you prefer.
> 
> Thanks & Regards,
> 
> Hans
> 
>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> Hi,
> 
> On 02-04-19 18:04, Luís Ferreira wrote:
>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
>> orientation.
> 
> I deliberately left out the KIOX020A id for now, because currently
> userspace cannot really deal with having 2 sensors.
> 
> See:
> https://github.com/systemd/systemd/issues/6557
> https://github.com/hadess/iio-sensor-proxy/issues/166
> 
> Basically we need to come up with a convention to (optionally) indicate
> the sensors location with a udev attribute set by:
> /lib/udev/hwdb.d/60-sensor.hwdb
> 
> And then patch iio-sensor-proxy to consume that attribute and ignore
> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
> 
> Ignoring would be a first step, maybe later it can do something useful
> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
> 
> IMHO we really should minimally get code in place for iio-sensor-proxy
> to ignore the keyboard accelerometer before merging this patch.
> 
> I realize that having the code in place will not magically get it on
> all users machines, but I believe this is the minimum which needs to
> happen before we push this out and potentially breaks people screen
> rotation.
> 
> I've had working on this on my TODO list for a long long time now,
> but -ENOTIME. If you have some time to work on this then that would
> be great.
> 
> Regards,
> 
> Hans
>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
>> ---
>> drivers/iio/accel/kxcjk-1013.c | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>> index 7096e577b23f..9a5e445facc1 100644
>> --- a/drivers/iio/accel/kxcjk-1013.c
>> +++ b/drivers/iio/accel/kxcjk-1013.c
>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>> {"KIOX0009", KXTJ21009},
>> {"KIOX000A", KXCJ91008},
>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
>> + {"KIOX020A", KXCJ91008},
>> {"KXTJ1009", KXTJ21009},
>> {"KXJ2109", KXTJ21009},
>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-15 15:40     ` luis
@ 2019-04-17 15:08       ` Hans de Goede
  2019-05-23 12:03         ` Luís Ferreira
  0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2019-04-17 15:08 UTC (permalink / raw)
  To: luis
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

On 15-04-19 17:40, luis@aurorafoss.org wrote:
> April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
> 
> Hi,
> 
>> Yes that seems the best way forward with this.
>>
>> Note I think "base" is better then "keyboard" for the sensor which
>> is in the base/keyboard. But neither is perfect, so go which whatever
>> you prefer.
> 
> Reference to:
> - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322

Thank you for your work on this, I see that Bastien has
already reviewed the iio-sensor-proxy changes.

I've just added one small remark to the systemd changes,
except for that small remark the systemd changes look good to me.

Regards,

Hams




>> On 06-04-19 01:01, Luís Ferreira wrote:
>>
>>> Hi,
>>> Basically we need to come up with a convention to (optionally) indicate
>>
>> the sensors location with a udev attribute set by:
>> /lib/udev/hwdb.d/60-sensor.hwdb
>>> So should we start adding `ACCEL_LOCATION=display` and
>>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
>>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
>>
>> Yes that seems the best way forward with this.
>>
>> Note I think "base" is better then "keyboard" for the sensor which
>> is in the base/keyboard. But neither is perfect, so go which whatever
>> you prefer.
>>
>> Thanks & Regards,
>>
>> Hans
>>
>>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 02-04-19 18:04, Luís Ferreira wrote:
>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
>>> orientation.
>>
>> I deliberately left out the KIOX020A id for now, because currently
>> userspace cannot really deal with having 2 sensors.
>>
>> See:
>> https://github.com/systemd/systemd/issues/6557
>> https://github.com/hadess/iio-sensor-proxy/issues/166
>>
>> Basically we need to come up with a convention to (optionally) indicate
>> the sensors location with a udev attribute set by:
>> /lib/udev/hwdb.d/60-sensor.hwdb
>>
>> And then patch iio-sensor-proxy to consume that attribute and ignore
>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
>>
>> Ignoring would be a first step, maybe later it can do something useful
>> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
>>
>> IMHO we really should minimally get code in place for iio-sensor-proxy
>> to ignore the keyboard accelerometer before merging this patch.
>>
>> I realize that having the code in place will not magically get it on
>> all users machines, but I believe this is the minimum which needs to
>> happen before we push this out and potentially breaks people screen
>> rotation.
>>
>> I've had working on this on my TODO list for a long long time now,
>> but -ENOTIME. If you have some time to work on this then that would
>> be great.
>>
>> Regards,
>>
>> Hans
>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
>>> ---
>>> drivers/iio/accel/kxcjk-1013.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>>> index 7096e577b23f..9a5e445facc1 100644
>>> --- a/drivers/iio/accel/kxcjk-1013.c
>>> +++ b/drivers/iio/accel/kxcjk-1013.c
>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>>> {"KIOX0009", KXTJ21009},
>>> {"KIOX000A", KXCJ91008},
>>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
>>> + {"KIOX020A", KXCJ91008},
>>> {"KXTJ1009", KXTJ21009},
>>> {"KXJ2109", KXTJ21009},
>>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-04-17 15:08       ` Hans de Goede
@ 2019-05-23 12:03         ` Luís Ferreira
  2019-05-29  9:57           ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Luís Ferreira @ 2019-05-23 12:03 UTC (permalink / raw)
  To: Hans de Goede
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi Hams,

https://github.com/systemd/systemd/pull/12322
https://github.com/hadess/iio-sensor-proxy/pull/262

As all the pull requests to systemd and iio-sensor-proxy has already
been merged, I just need your review on this:
https://github.com/systemd/systemd/pull/12449

Then, I guess this patch is ready to go. Please let me know if theres any issue.

Regards,
Luis


On Wed, 17 Apr 2019 at 16:08, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 15-04-19 17:40, luis@aurorafoss.org wrote:
> > April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
> >
> > Hi,
> >
> >> Yes that seems the best way forward with this.
> >>
> >> Note I think "base" is better then "keyboard" for the sensor which
> >> is in the base/keyboard. But neither is perfect, so go which whatever
> >> you prefer.
> >
> > Reference to:
> > - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322
>
> Thank you for your work on this, I see that Bastien has
> already reviewed the iio-sensor-proxy changes.
>
> I've just added one small remark to the systemd changes,
> except for that small remark the systemd changes look good to me.
>
> Regards,
>
> Hams
>
>
>
>
> >> On 06-04-19 01:01, Luís Ferreira wrote:
> >>
> >>> Hi,
> >>> Basically we need to come up with a convention to (optionally) indicate
> >>
> >> the sensors location with a udev attribute set by:
> >> /lib/udev/hwdb.d/60-sensor.hwdb
> >>> So should we start adding `ACCEL_LOCATION=display` and
> >>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
> >>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
> >>
> >> Yes that seems the best way forward with this.
> >>
> >> Note I think "base" is better then "keyboard" for the sensor which
> >> is in the base/keyboard. But neither is perfect, so go which whatever
> >> you prefer.
> >>
> >> Thanks & Regards,
> >>
> >> Hans
> >>
> >>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 02-04-19 18:04, Luís Ferreira wrote:
> >>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
> >>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
> >>> orientation.
> >>
> >> I deliberately left out the KIOX020A id for now, because currently
> >> userspace cannot really deal with having 2 sensors.
> >>
> >> See:
> >> https://github.com/systemd/systemd/issues/6557
> >> https://github.com/hadess/iio-sensor-proxy/issues/166
> >>
> >> Basically we need to come up with a convention to (optionally) indicate
> >> the sensors location with a udev attribute set by:
> >> /lib/udev/hwdb.d/60-sensor.hwdb
> >>
> >> And then patch iio-sensor-proxy to consume that attribute and ignore
> >> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
> >>
> >> Ignoring would be a first step, maybe later it can do something useful
> >> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
> >>
> >> IMHO we really should minimally get code in place for iio-sensor-proxy
> >> to ignore the keyboard accelerometer before merging this patch.
> >>
> >> I realize that having the code in place will not magically get it on
> >> all users machines, but I believe this is the minimum which needs to
> >> happen before we push this out and potentially breaks people screen
> >> rotation.
> >>
> >> I've had working on this on my TODO list for a long long time now,
> >> but -ENOTIME. If you have some time to work on this then that would
> >> be great.
> >>
> >> Regards,
> >>
> >> Hans
> >>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
> >>> ---
> >>> drivers/iio/accel/kxcjk-1013.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> >>> index 7096e577b23f..9a5e445facc1 100644
> >>> --- a/drivers/iio/accel/kxcjk-1013.c
> >>> +++ b/drivers/iio/accel/kxcjk-1013.c
> >>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
> >>> {"KIOX0009", KXTJ21009},
> >>> {"KIOX000A", KXCJ91008},
> >>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
> >>> + {"KIOX020A", KXCJ91008},
> >>> {"KXTJ1009", KXTJ21009},
> >>> {"KXJ2109", KXTJ21009},
> >>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-05-23 12:03         ` Luís Ferreira
@ 2019-05-29  9:57           ` Hans de Goede
  2019-05-30 11:53             ` Luís Ferreira
  0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2019-05-29  9:57 UTC (permalink / raw)
  To: Luís Ferreira
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

On 5/23/19 2:03 PM, Luís Ferreira wrote:
> Hi Hams,
> 
> https://github.com/systemd/systemd/pull/12322
> https://github.com/hadess/iio-sensor-proxy/pull/262
> 
> As all the pull requests to systemd and iio-sensor-proxy has already
> been merged, I just need your review on this:
> https://github.com/systemd/systemd/pull/12449
> 
> Then, I guess this patch is ready to go. Please let me know if theres any issue.

Thank you for your work on this. I've given a favorable review to the

last systemd  pull-req.

Regards,

Hans


> On Wed, 17 Apr 2019 at 16:08, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 15-04-19 17:40, luis@aurorafoss.org wrote:
>>> April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>>> Yes that seems the best way forward with this.
>>>>
>>>> Note I think "base" is better then "keyboard" for the sensor which
>>>> is in the base/keyboard. But neither is perfect, so go which whatever
>>>> you prefer.
>>>
>>> Reference to:
>>> - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322
>>
>> Thank you for your work on this, I see that Bastien has
>> already reviewed the iio-sensor-proxy changes.
>>
>> I've just added one small remark to the systemd changes,
>> except for that small remark the systemd changes look good to me.
>>
>> Regards,
>>
>> Hams
>>
>>
>>
>>
>>>> On 06-04-19 01:01, Luís Ferreira wrote:
>>>>
>>>>> Hi,
>>>>> Basically we need to come up with a convention to (optionally) indicate
>>>>
>>>> the sensors location with a udev attribute set by:
>>>> /lib/udev/hwdb.d/60-sensor.hwdb
>>>>> So should we start adding `ACCEL_LOCATION=display` and
>>>>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
>>>>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
>>>>
>>>> Yes that seems the best way forward with this.
>>>>
>>>> Note I think "base" is better then "keyboard" for the sensor which
>>>> is in the base/keyboard. But neither is perfect, so go which whatever
>>>> you prefer.
>>>>
>>>> Thanks & Regards,
>>>>
>>>> Hans
>>>>
>>>>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 02-04-19 18:04, Luís Ferreira wrote:
>>>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
>>>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
>>>>> orientation.
>>>>
>>>> I deliberately left out the KIOX020A id for now, because currently
>>>> userspace cannot really deal with having 2 sensors.
>>>>
>>>> See:
>>>> https://github.com/systemd/systemd/issues/6557
>>>> https://github.com/hadess/iio-sensor-proxy/issues/166
>>>>
>>>> Basically we need to come up with a convention to (optionally) indicate
>>>> the sensors location with a udev attribute set by:
>>>> /lib/udev/hwdb.d/60-sensor.hwdb
>>>>
>>>> And then patch iio-sensor-proxy to consume that attribute and ignore
>>>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
>>>>
>>>> Ignoring would be a first step, maybe later it can do something useful
>>>> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
>>>>
>>>> IMHO we really should minimally get code in place for iio-sensor-proxy
>>>> to ignore the keyboard accelerometer before merging this patch.
>>>>
>>>> I realize that having the code in place will not magically get it on
>>>> all users machines, but I believe this is the minimum which needs to
>>>> happen before we push this out and potentially breaks people screen
>>>> rotation.
>>>>
>>>> I've had working on this on my TODO list for a long long time now,
>>>> but -ENOTIME. If you have some time to work on this then that would
>>>> be great.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
>>>>> ---
>>>>> drivers/iio/accel/kxcjk-1013.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>>>>> index 7096e577b23f..9a5e445facc1 100644
>>>>> --- a/drivers/iio/accel/kxcjk-1013.c
>>>>> +++ b/drivers/iio/accel/kxcjk-1013.c
>>>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>>>>> {"KIOX0009", KXTJ21009},
>>>>> {"KIOX000A", KXCJ91008},
>>>>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
>>>>> + {"KIOX020A", KXCJ91008},
>>>>> {"KXTJ1009", KXTJ21009},
>>>>> {"KXJ2109", KXTJ21009},
>>>>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-05-29  9:57           ` Hans de Goede
@ 2019-05-30 11:53             ` Luís Ferreira
  2019-05-30 12:35               ` Hans de Goede
  0 siblings, 1 reply; 11+ messages in thread
From: Luís Ferreira @ 2019-05-30 11:53 UTC (permalink / raw)
  To: Hans de Goede
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

@poettering just merged the pull request. Is it ready to go now?

Sincerely,
Luís Ferreira
E2AB 2BF8 F3EC ABE2 E149 F2FD BC43 0831 9CE4 0B64

On Wed, 29 May 2019 at 10:57, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 5/23/19 2:03 PM, Luís Ferreira wrote:
> > Hi Hams,
> >
> > https://github.com/systemd/systemd/pull/12322
> > https://github.com/hadess/iio-sensor-proxy/pull/262
> >
> > As all the pull requests to systemd and iio-sensor-proxy has already
> > been merged, I just need your review on this:
> > https://github.com/systemd/systemd/pull/12449
> >
> > Then, I guess this patch is ready to go. Please let me know if theres any issue.
>
> Thank you for your work on this. I've given a favorable review to the
>
> last systemd  pull-req.
>
> Regards,
>
> Hans
>
>
> > On Wed, 17 Apr 2019 at 16:08, Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 15-04-19 17:40, luis@aurorafoss.org wrote:
> >>> April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>>> Yes that seems the best way forward with this.
> >>>>
> >>>> Note I think "base" is better then "keyboard" for the sensor which
> >>>> is in the base/keyboard. But neither is perfect, so go which whatever
> >>>> you prefer.
> >>>
> >>> Reference to:
> >>> - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322
> >>
> >> Thank you for your work on this, I see that Bastien has
> >> already reviewed the iio-sensor-proxy changes.
> >>
> >> I've just added one small remark to the systemd changes,
> >> except for that small remark the systemd changes look good to me.
> >>
> >> Regards,
> >>
> >> Hams
> >>
> >>
> >>
> >>
> >>>> On 06-04-19 01:01, Luís Ferreira wrote:
> >>>>
> >>>>> Hi,
> >>>>> Basically we need to come up with a convention to (optionally) indicate
> >>>>
> >>>> the sensors location with a udev attribute set by:
> >>>> /lib/udev/hwdb.d/60-sensor.hwdb
> >>>>> So should we start adding `ACCEL_LOCATION=display` and
> >>>>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
> >>>>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
> >>>>
> >>>> Yes that seems the best way forward with this.
> >>>>
> >>>> Note I think "base" is better then "keyboard" for the sensor which
> >>>> is in the base/keyboard. But neither is perfect, so go which whatever
> >>>> you prefer.
> >>>>
> >>>> Thanks & Regards,
> >>>>
> >>>> Hans
> >>>>
> >>>>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 02-04-19 18:04, Luís Ferreira wrote:
> >>>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
> >>>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
> >>>>> orientation.
> >>>>
> >>>> I deliberately left out the KIOX020A id for now, because currently
> >>>> userspace cannot really deal with having 2 sensors.
> >>>>
> >>>> See:
> >>>> https://github.com/systemd/systemd/issues/6557
> >>>> https://github.com/hadess/iio-sensor-proxy/issues/166
> >>>>
> >>>> Basically we need to come up with a convention to (optionally) indicate
> >>>> the sensors location with a udev attribute set by:
> >>>> /lib/udev/hwdb.d/60-sensor.hwdb
> >>>>
> >>>> And then patch iio-sensor-proxy to consume that attribute and ignore
> >>>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
> >>>>
> >>>> Ignoring would be a first step, maybe later it can do something useful
> >>>> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
> >>>>
> >>>> IMHO we really should minimally get code in place for iio-sensor-proxy
> >>>> to ignore the keyboard accelerometer before merging this patch.
> >>>>
> >>>> I realize that having the code in place will not magically get it on
> >>>> all users machines, but I believe this is the minimum which needs to
> >>>> happen before we push this out and potentially breaks people screen
> >>>> rotation.
> >>>>
> >>>> I've had working on this on my TODO list for a long long time now,
> >>>> but -ENOTIME. If you have some time to work on this then that would
> >>>> be great.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hans
> >>>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
> >>>>> ---
> >>>>> drivers/iio/accel/kxcjk-1013.c | 1 +
> >>>>> 1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> >>>>> index 7096e577b23f..9a5e445facc1 100644
> >>>>> --- a/drivers/iio/accel/kxcjk-1013.c
> >>>>> +++ b/drivers/iio/accel/kxcjk-1013.c
> >>>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
> >>>>> {"KIOX0009", KXTJ21009},
> >>>>> {"KIOX000A", KXCJ91008},
> >>>>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
> >>>>> + {"KIOX020A", KXCJ91008},
> >>>>> {"KXTJ1009", KXTJ21009},
> >>>>> {"KXJ2109", KXTJ21009},
> >>>>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-05-30 11:53             ` Luís Ferreira
@ 2019-05-30 12:35               ` Hans de Goede
  2019-06-08 10:53                 ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2019-05-30 12:35 UTC (permalink / raw)
  To: Luís Ferreira
  Cc: jic23, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

Hi,

On 30-05-19 13:53, Luís Ferreira wrote:
> Hi,
> 
> @poettering just merged the pull request. Is it ready to go now?

You mean the kernel patch, yes merging that now is fine with me.

Regards,

Hans



> 
> Sincerely,
> Luís Ferreira
> E2AB 2BF8 F3EC ABE2 E149 F2FD BC43 0831 9CE4 0B64
> 
> On Wed, 29 May 2019 at 10:57, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 5/23/19 2:03 PM, Luís Ferreira wrote:
>>> Hi Hams,
>>>
>>> https://github.com/systemd/systemd/pull/12322
>>> https://github.com/hadess/iio-sensor-proxy/pull/262
>>>
>>> As all the pull requests to systemd and iio-sensor-proxy has already
>>> been merged, I just need your review on this:
>>> https://github.com/systemd/systemd/pull/12449
>>>
>>> Then, I guess this patch is ready to go. Please let me know if theres any issue.
>>
>> Thank you for your work on this. I've given a favorable review to the
>>
>> last systemd  pull-req.
>>
>> Regards,
>>
>> Hans
>>
>>
>>> On Wed, 17 Apr 2019 at 16:08, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 15-04-19 17:40, luis@aurorafoss.org wrote:
>>>>> April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> Yes that seems the best way forward with this.
>>>>>>
>>>>>> Note I think "base" is better then "keyboard" for the sensor which
>>>>>> is in the base/keyboard. But neither is perfect, so go which whatever
>>>>>> you prefer.
>>>>>
>>>>> Reference to:
>>>>> - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322
>>>>
>>>> Thank you for your work on this, I see that Bastien has
>>>> already reviewed the iio-sensor-proxy changes.
>>>>
>>>> I've just added one small remark to the systemd changes,
>>>> except for that small remark the systemd changes look good to me.
>>>>
>>>> Regards,
>>>>
>>>> Hams
>>>>
>>>>
>>>>
>>>>
>>>>>> On 06-04-19 01:01, Luís Ferreira wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>> Basically we need to come up with a convention to (optionally) indicate
>>>>>>
>>>>>> the sensors location with a udev attribute set by:
>>>>>> /lib/udev/hwdb.d/60-sensor.hwdb
>>>>>>> So should we start adding `ACCEL_LOCATION=display` and
>>>>>>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
>>>>>>> iio-sensor-proxy to ignore the keyboard ones as a first step ?
>>>>>>
>>>>>> Yes that seems the best way forward with this.
>>>>>>
>>>>>> Note I think "base" is better then "keyboard" for the sensor which
>>>>>> is in the base/keyboard. But neither is perfect, so go which whatever
>>>>>> you prefer.
>>>>>>
>>>>>> Thanks & Regards,
>>>>>>
>>>>>> Hans
>>>>>>
>>>>>>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 02-04-19 18:04, Luís Ferreira wrote:
>>>>>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
>>>>>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
>>>>>>> orientation.
>>>>>>
>>>>>> I deliberately left out the KIOX020A id for now, because currently
>>>>>> userspace cannot really deal with having 2 sensors.
>>>>>>
>>>>>> See:
>>>>>> https://github.com/systemd/systemd/issues/6557
>>>>>> https://github.com/hadess/iio-sensor-proxy/issues/166
>>>>>>
>>>>>> Basically we need to come up with a convention to (optionally) indicate
>>>>>> the sensors location with a udev attribute set by:
>>>>>> /lib/udev/hwdb.d/60-sensor.hwdb
>>>>>>
>>>>>> And then patch iio-sensor-proxy to consume that attribute and ignore
>>>>>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
>>>>>>
>>>>>> Ignoring would be a first step, maybe later it can do something useful
>>>>>> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
>>>>>>
>>>>>> IMHO we really should minimally get code in place for iio-sensor-proxy
>>>>>> to ignore the keyboard accelerometer before merging this patch.
>>>>>>
>>>>>> I realize that having the code in place will not magically get it on
>>>>>> all users machines, but I believe this is the minimum which needs to
>>>>>> happen before we push this out and potentially breaks people screen
>>>>>> rotation.
>>>>>>
>>>>>> I've had working on this on my TODO list for a long long time now,
>>>>>> but -ENOTIME. If you have some time to work on this then that would
>>>>>> be great.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Hans
>>>>>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
>>>>>>> ---
>>>>>>> drivers/iio/accel/kxcjk-1013.c | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>>>>>>> index 7096e577b23f..9a5e445facc1 100644
>>>>>>> --- a/drivers/iio/accel/kxcjk-1013.c
>>>>>>> +++ b/drivers/iio/accel/kxcjk-1013.c
>>>>>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
>>>>>>> {"KIOX0009", KXTJ21009},
>>>>>>> {"KIOX000A", KXCJ91008},
>>>>>>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
>>>>>>> + {"KIOX020A", KXCJ91008},
>>>>>>> {"KXTJ1009", KXTJ21009},
>>>>>>> {"KXJ2109", KXTJ21009},
>>>>>>> {"SMO8500", KXCJ91008},

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
  2019-05-30 12:35               ` Hans de Goede
@ 2019-06-08 10:53                 ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2019-06-08 10:53 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Luís Ferreira, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Stephan Gerhold,
	open list:IIO SUBSYSTEM AND DRIVERS, open list

On Thu, 30 May 2019 14:35:10 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 30-05-19 13:53, Luís Ferreira wrote:
> > Hi,
> > 
> > @poettering just merged the pull request. Is it ready to go now?  
> 
> You mean the kernel patch, yes merging that now is fine with me.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> 
> Regards,
> 
> Hans
> 
> 
> 
> > 
> > Sincerely,
> > Luís Ferreira
> > E2AB 2BF8 F3EC ABE2 E149 F2FD BC43 0831 9CE4 0B64
> > 
> > On Wed, 29 May 2019 at 10:57, Hans de Goede <hdegoede@redhat.com> wrote:  
> >>
> >> Hi,
> >>
> >> On 5/23/19 2:03 PM, Luís Ferreira wrote:  
> >>> Hi Hams,
> >>>
> >>> https://github.com/systemd/systemd/pull/12322
> >>> https://github.com/hadess/iio-sensor-proxy/pull/262
> >>>
> >>> As all the pull requests to systemd and iio-sensor-proxy has already
> >>> been merged, I just need your review on this:
> >>> https://github.com/systemd/systemd/pull/12449
> >>>
> >>> Then, I guess this patch is ready to go. Please let me know if theres any issue.  
> >>
> >> Thank you for your work on this. I've given a favorable review to the
> >>
> >> last systemd  pull-req.
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>  
> >>> On Wed, 17 Apr 2019 at 16:08, Hans de Goede <hdegoede@redhat.com> wrote:  
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 15-04-19 17:40, luis@aurorafoss.org wrote:  
> >>>>> April 6, 2019 10:36 AM, "Hans de Goede" <hdegoede@redhat.com> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>  
> >>>>>> Yes that seems the best way forward with this.
> >>>>>>
> >>>>>> Note I think "base" is better then "keyboard" for the sensor which
> >>>>>> is in the base/keyboard. But neither is perfect, so go which whatever
> >>>>>> you prefer.  
> >>>>>
> >>>>> Reference to:
> >>>>> - https://github.com/hadess/iio-sensor-proxy/pull/262 > - https://github.com/systemd/systemd/pull/12322  
> >>>>
> >>>> Thank you for your work on this, I see that Bastien has
> >>>> already reviewed the iio-sensor-proxy changes.
> >>>>
> >>>> I've just added one small remark to the systemd changes,
> >>>> except for that small remark the systemd changes look good to me.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Hams
> >>>>
> >>>>
> >>>>
> >>>>  
> >>>>>> On 06-04-19 01:01, Luís Ferreira wrote:
> >>>>>>  
> >>>>>>> Hi,
> >>>>>>> Basically we need to come up with a convention to (optionally) indicate  
> >>>>>>
> >>>>>> the sensors location with a udev attribute set by:
> >>>>>> /lib/udev/hwdb.d/60-sensor.hwdb  
> >>>>>>> So should we start adding `ACCEL_LOCATION=display` and
> >>>>>>> `ACCEL_LOCATION=keyboard` attributes to that file and patch
> >>>>>>> iio-sensor-proxy to ignore the keyboard ones as a first step ?  
> >>>>>>
> >>>>>> Yes that seems the best way forward with this.
> >>>>>>
> >>>>>> Note I think "base" is better then "keyboard" for the sensor which
> >>>>>> is in the base/keyboard. But neither is perfect, so go which whatever
> >>>>>> you prefer.
> >>>>>>
> >>>>>> Thanks & Regards,
> >>>>>>
> >>>>>> Hans
> >>>>>>  
> >>>>>>> On Wed, 3 Apr 2019 at 10:10, Hans de Goede <hdegoede@redhat.com> wrote:  
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 02-04-19 18:04, Luís Ferreira wrote:  
> >>>>>>> Some ultrabooks, like Teclast F6 Pro, use KIOX010A sensor on display
> >>>>>>> and KIOX020A sensor on keyboard base, to detect tablet mode or screen
> >>>>>>> orientation.  
> >>>>>>
> >>>>>> I deliberately left out the KIOX020A id for now, because currently
> >>>>>> userspace cannot really deal with having 2 sensors.
> >>>>>>
> >>>>>> See:
> >>>>>> https://github.com/systemd/systemd/issues/6557
> >>>>>> https://github.com/hadess/iio-sensor-proxy/issues/166
> >>>>>>
> >>>>>> Basically we need to come up with a convention to (optionally) indicate
> >>>>>> the sensors location with a udev attribute set by:
> >>>>>> /lib/udev/hwdb.d/60-sensor.hwdb
> >>>>>>
> >>>>>> And then patch iio-sensor-proxy to consume that attribute and ignore
> >>>>>> the one which has e.g. ACCEL_LOCATION=keyboard in its udev properties
> >>>>>>
> >>>>>> Ignoring would be a first step, maybe later it can do something useful
> >>>>>> with it, see e.g. : https://github.com/alesguzik/linux_detect_tablet_mode
> >>>>>>
> >>>>>> IMHO we really should minimally get code in place for iio-sensor-proxy
> >>>>>> to ignore the keyboard accelerometer before merging this patch.
> >>>>>>
> >>>>>> I realize that having the code in place will not magically get it on
> >>>>>> all users machines, but I believe this is the minimum which needs to
> >>>>>> happen before we push this out and potentially breaks people screen
> >>>>>> rotation.
> >>>>>>
> >>>>>> I've had working on this on my TODO list for a long long time now,
> >>>>>> but -ENOTIME. If you have some time to work on this then that would
> >>>>>> be great.
> >>>>>>
> >>>>>> Regards,
> >>>>>>
> >>>>>> Hans  
> >>>>>>> Signed-off-by: Luís Ferreira <luis@aurorafoss.org>
> >>>>>>> ---
> >>>>>>> drivers/iio/accel/kxcjk-1013.c | 1 +
> >>>>>>> 1 file changed, 1 insertion(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> >>>>>>> index 7096e577b23f..9a5e445facc1 100644
> >>>>>>> --- a/drivers/iio/accel/kxcjk-1013.c
> >>>>>>> +++ b/drivers/iio/accel/kxcjk-1013.c
> >>>>>>> @@ -1492,6 +1492,7 @@ static const struct acpi_device_id kx_acpi_match[] = {
> >>>>>>> {"KIOX0009", KXTJ21009},
> >>>>>>> {"KIOX000A", KXCJ91008},
> >>>>>>> {"KIOX010A", KXCJ91008}, /* KXCJ91008 inside the display of a 2-in-1 */
> >>>>>>> + {"KIOX020A", KXCJ91008},
> >>>>>>> {"KXTJ1009", KXTJ21009},
> >>>>>>> {"KXJ2109", KXTJ21009},
> >>>>>>> {"SMO8500", KXCJ91008},  


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-06-08 10:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-02 16:04 [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks Luís Ferreira
2019-04-03  9:10 ` Hans de Goede
2019-04-05 23:01   ` Luís Ferreira
2019-04-06  9:36     ` Hans de Goede
2019-04-15 15:40     ` luis
2019-04-17 15:08       ` Hans de Goede
2019-05-23 12:03         ` Luís Ferreira
2019-05-29  9:57           ` Hans de Goede
2019-05-30 11:53             ` Luís Ferreira
2019-05-30 12:35               ` Hans de Goede
2019-06-08 10:53                 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).