All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Hart <dvhart@infradead.org>
To: Azael Avalos <coproscefalo@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jonathan Cameron <jic23@kernel.org>
Cc: Matthew Garrett <matthew.garrett@nebula.com>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH 3/5] toshiba_acpi: Add accelerometer input polled device
Date: Mon, 8 Sep 2014 17:04:30 -0700	[thread overview]
Message-ID: <20140909000429.GB5835@vmdeb7> (raw)
In-Reply-To: <CAGdLNWGhdQU3Fpud9Zgvx3AQ5Lb=WdEkJb_wWTb1hJoHxXPXpQ@mail.gmail.com>

On Fri, Sep 05, 2014 at 11:04:18PM -0600, Azael Avalos wrote:
> Hi there,
> 
> 2014-09-05 20:42 GMT-06:00 Darren Hart <dvhart@infradead.org>:
> > On Fri, Sep 05, 2014 at 11:14:05AM -0600, Azael Avalos wrote:
> >> The accelerometer sensor is very sensitive, and having userspace
> >> poll the sysfs position entry is not very battery friendly.
> >>
> >> This patch removes the sysfs entry and instead, it creates an
> >> input polled device (joystick) for the built-in accelerometer.
> >
> > Hrm, while sysfs details can change across kernel versions, usually due to
> > driver core changes, we try to keep them as consistent as possible so as not to
> > break userspace.
> >
> > That said, if we are going to try and come up with a better model for
> > representing an accelerometer, wouldn't treating it as an IIO device be the more
> > logical approach?
> 
> Yes of course, but the actual accelerometer device (sensor?) is not
> really exposed,
> only certain "functions" it provides, and they are divided across two
> different ACPI devices,
> TOS620A exposes the protection, and the TOS1900 (and et. al.) only
> exposes the axes.

As I understand it, IIO defines an interface to a device, a standard sysfs set
of properties. I should think we could provide the appropriate callbacks even
for a partially implemented (or a pair of) accelerometer.

Jonathan, what are your thoughts here. Is such a "device" (ACPI accessors to
axis and threshold) a candidate for IIO, or is this input polled device more
appropriate?

> 
> I see your point in breaking userspace, but given the fact that it was
> recently introduced,
> I didn't thought it was already "adopted", that's why I decided to
> remove the sysfs entry.

Looks like since 3.15 if I read the log correctly. That is fairly recent and
this is not one of the "defined interfaces" in the sysfs documentation.

Greg, can you weigh in here - does this change count as "breaking userspace", or
is this more inline with the scheduler knobs in /proc/sched_debug which can
change from version to version.

> 
> Then we might as well keep the sysfs entry and have the input polled
> device as well.

Let's see what Greg has to say. If he isn't bothered by the change, I won't push
the issue.

-- 
Darren Hart
Intel Open Source Technology Center

WARNING: multiple messages have this Message-ID (diff)
From: Darren Hart <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
To: Azael Avalos
	<coproscefalo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Matthew Garrett
	<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>,
	"platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 3/5] toshiba_acpi: Add accelerometer input polled device
Date: Mon, 8 Sep 2014 17:04:30 -0700	[thread overview]
Message-ID: <20140909000429.GB5835@vmdeb7> (raw)
In-Reply-To: <CAGdLNWGhdQU3Fpud9Zgvx3AQ5Lb=WdEkJb_wWTb1hJoHxXPXpQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Sep 05, 2014 at 11:04:18PM -0600, Azael Avalos wrote:
> Hi there,
> 
> 2014-09-05 20:42 GMT-06:00 Darren Hart <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>:
> > On Fri, Sep 05, 2014 at 11:14:05AM -0600, Azael Avalos wrote:
> >> The accelerometer sensor is very sensitive, and having userspace
> >> poll the sysfs position entry is not very battery friendly.
> >>
> >> This patch removes the sysfs entry and instead, it creates an
> >> input polled device (joystick) for the built-in accelerometer.
> >
> > Hrm, while sysfs details can change across kernel versions, usually due to
> > driver core changes, we try to keep them as consistent as possible so as not to
> > break userspace.
> >
> > That said, if we are going to try and come up with a better model for
> > representing an accelerometer, wouldn't treating it as an IIO device be the more
> > logical approach?
> 
> Yes of course, but the actual accelerometer device (sensor?) is not
> really exposed,
> only certain "functions" it provides, and they are divided across two
> different ACPI devices,
> TOS620A exposes the protection, and the TOS1900 (and et. al.) only
> exposes the axes.

As I understand it, IIO defines an interface to a device, a standard sysfs set
of properties. I should think we could provide the appropriate callbacks even
for a partially implemented (or a pair of) accelerometer.

Jonathan, what are your thoughts here. Is such a "device" (ACPI accessors to
axis and threshold) a candidate for IIO, or is this input polled device more
appropriate?

> 
> I see your point in breaking userspace, but given the fact that it was
> recently introduced,
> I didn't thought it was already "adopted", that's why I decided to
> remove the sysfs entry.

Looks like since 3.15 if I read the log correctly. That is fairly recent and
this is not one of the "defined interfaces" in the sysfs documentation.

Greg, can you weigh in here - does this change count as "breaking userspace", or
is this more inline with the scheduler knobs in /proc/sched_debug which can
change from version to version.

> 
> Then we might as well keep the sysfs entry and have the input polled
> device as well.

Let's see what Greg has to say. If he isn't bothered by the change, I won't push
the issue.

-- 
Darren Hart
Intel Open Source Technology Center

  reply	other threads:[~2014-09-09  0:04 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-05 17:14 [PATCH 0/5] toshiba_acpi: Various changes plus fixes Azael Avalos
2014-09-05 17:14 ` [PATCH 1/5] toshiba_acpi: Additional hotkey scancodes Azael Avalos
2014-09-09  0:12   ` Darren Hart
2014-09-05 17:14 ` [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models Azael Avalos
2014-09-06  2:35   ` Darren Hart
2014-09-06  4:49     ` Azael Avalos
2014-09-09  0:09       ` Darren Hart
2014-09-05 17:14 ` [PATCH 3/5] toshiba_acpi: Add accelerometer input polled device Azael Avalos
2014-09-06  2:42   ` Darren Hart
2014-09-06  5:04     ` Azael Avalos
2014-09-09  0:04       ` Darren Hart [this message]
2014-09-09  0:04         ` Darren Hart
2014-09-09  1:35         ` Greg Kroah-Hartman
2014-09-10  3:35           ` Darren Hart
2014-09-10 15:28             ` Azael Avalos
2014-09-10 16:08               ` Greg Kroah-Hartman
2014-09-10 16:08                 ` Greg Kroah-Hartman
2014-09-17 16:36         ` Jonathan Cameron
2014-09-17 18:38           ` Darren Hart
2014-09-05 17:14 ` [PATCH 4/5] toshiba_acpi: Support new keyboard backlight type Azael Avalos
2014-09-10  4:11   ` Darren Hart
2014-09-10 16:52     ` Azael Avalos
2014-09-10 18:34       ` Darren Hart
2014-09-05 17:14 ` [PATCH 5/5] toshiba_acpi: Change touchpad store to check for invalid values Azael Avalos
2014-09-10  4:17   ` Darren Hart

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=20140909000429.GB5835@vmdeb7 \
    --to=dvhart@infradead.org \
    --cc=coproscefalo@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew.garrett@nebula.com \
    --cc=platform-driver-x86@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.