From: Nicolin Chen <nicoleotsuka@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: jdelvare@suse.com, linux-hwmon@vger.kernel.org,
linux-kernel@vger.kernel.org, corbet@lwn.net,
linux-doc@vger.kernel.org
Subject: Re: [PATCH] hwmon (ina3221) Add single-shot mode support
Date: Fri, 16 Nov 2018 17:51:34 -0800 [thread overview]
Message-ID: <20181117015131.GA10407@Asurada-Nvidia.nvidia.com> (raw)
In-Reply-To: <20181114172330.GA25592@roeck-us.net>
Hello Guenter,
On Wed, Nov 14, 2018 at 09:23:30AM -0800, Guenter Roeck wrote:
> > An alternative way (without the sysfs node), after looking at
> > other hwmon code, could be to have a timed polling thread and
> > read data using an update_interval value from ABI. This might
> > turn out to be more complicated as it'll also involve settings
> > of hardware averaging and conversion time. Above all, I cannot
> > figure out a good threshold of update_interval to switch modes.
> >
> update_interval should only be used if it can be configured
> into hardware, not to trigger a polling thread. It should only
> be used in the driver to determine caching intervals.
I see..so it's more like the conversion time settings instead.
> > If you can give some advice of a better implementation, that'd
> > be great.
> >
> From your description, the only real feasible solution would be a
> generic one, with a well defined interface either to userspace or
> to the kernel. The best I can think of would be an in-kernel API
> setting the power operational mode via callbacks. Alternative would
> be a generic ability to set power operational mode from userspace,
> using an ABI which applies to all drivers, not just one.
Hmm. That would make the situation really complicated, I could
understand your concern though.
I searched a little and found that, while the initial ina3221
hwmon driver was under review, Laxman submitted an IIO version
where Jonathan had a similar comments against its "mode" node:
https://www.spinics.net/lists/linux-hwmon/msg00219.html
Quote from his comments {
* There is a lot of ABI in here concerned with oneshot vs continuous.
This seems to me to be more than it should be. We wouldn't expect to
see stuff changing as a result of switching between these modes other
than wrt to when the data shows up. So I'd expect to not see this
directly exposed at all - but rather sit in oneshot unless either:
1) Buffered mode is running (not currently supported)
2) Alerts are on - which I think requires it to be in continuous mode.
}
Since hwmon driver doesn't support buffered mode, what do you
think about having the chip run in single-shot mode by default
but changing it if alerts are on? Though the driver also needs
to support to enable warning/critical alert pins.
In short, other than exposing it via a generic ABI to the user
space, how about defining some policy to maintaining it within
the driver?
Thanks
Nicolin
next prev parent reply other threads:[~2018-11-17 1:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-13 4:23 [PATCH] hwmon (ina3221) Add single-shot mode support Nicolin Chen
2018-11-13 4:32 ` Guenter Roeck
2018-11-13 4:58 ` Nicolin Chen
2018-11-13 17:21 ` Guenter Roeck
2018-11-14 0:11 ` Nicolin Chen
2018-11-14 17:23 ` Guenter Roeck
2018-11-17 1:51 ` Nicolin Chen [this message]
2018-11-19 17:45 ` Guenter Roeck
2018-11-19 22:18 ` Nicolin Chen
2018-11-23 16:38 ` Guenter Roeck
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=20181117015131.GA10407@Asurada-Nvidia.nvidia.com \
--to=nicoleotsuka@gmail.com \
--cc=corbet@lwn.net \
--cc=jdelvare@suse.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
/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 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).