LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Luís Ferreira" <luis@aurorafoss.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: jic23@kernel.org, Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	Stephan Gerhold <stephan@gerhold.net>,
	"open list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks
Date: Thu, 30 May 2019 12:53:29 +0100	[thread overview]
Message-ID: <CAPyOxm-dA3BF-6oRJt-rcNMQSucfOHNtGgXj4=aZ7a7khfvhVw@mail.gmail.com> (raw)
In-Reply-To: <96658776-bd09-4b2d-2049-f2b213d60f8a@redhat.com>

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},

  reply	other threads:[~2019-05-30 11:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-02 16:04 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 [this message]
2019-05-30 12:35               ` Hans de Goede
2019-06-08 10:53                 ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPyOxm-dA3BF-6oRJt-rcNMQSucfOHNtGgXj4=aZ7a7khfvhVw@mail.gmail.com' \
    --to=luis@aurorafoss.org \
    --cc=hdegoede@redhat.com \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --cc=stephan@gerhold.net \
    --subject='Re: [PATCH] iio: accel: add missing sensor for some 2-in-1 based ultrabooks' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).