public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
From: Trent Piepho <tpiepho@impinj.com>
To: "linux@roeck-us.net" <linux@roeck-us.net>,
	"m.felsch@pengutronix.de" <m.felsch@pengutronix.de>
Cc: "dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"jdelvare@suse.com" <jdelvare@suse.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>
Subject: Re: [PATCH v2 2/2] hwmon: add generic GPIO brownout support
Date: Tue, 30 Oct 2018 18:49:07 +0000	[thread overview]
Message-ID: <1540925347.30311.65.camel@impinj.com> (raw)
In-Reply-To: <8841d944-4fab-7d43-4edc-20f9a95ac009@roeck-us.net>

On Mon, 2018-10-29 at 18:12 -0700, Guenter Roeck wrote:
> On 10/29/18 2:16 PM, Trent Piepho wrote:
> > 
> > If we ignore the ability to stop other devices, how is this not a hwmon
> > device with just alarm features?
> > 
> 
> Possibly, but the ability to stop other devices is at the core of the driver
> as submitted.

I was thinking along the lines of a driver for gpio based hardware
alarms, that did not include the device stop feature.  Would that also
be quickly NACKed?

> > I2C/LPC/SPI interface was not connected as an appropriate master was
> > not available, and the default configuration of the chip was
> > acceptable.  The chip's alarm outputs are connected to GPIOs, as it
> > normal for a hwmon chip with alarm outputs.
> > 
> 
> "If we had" is theory. Do we ? We don't usually add code to the kernel
> just in case the hardware it supports might be out there.

What I was trying to do was reach the conclusion that a gpio hardware
alarm as a hwmon driver is appropriate via clear steps.

A classic hwmon chip should have a hwmon driver.  We all accept that.

Disconnect i2c interface, keep alarms, does the kernel interface need
to change?  Seems clear to me the answer is no, should still be hwmon.

Replace chip with discrete logic, e.g. an op amp and a few resistors
serving as a voltage comparator, which has the same behavior as the 
hwmon chip as far as the rest of the system is concerned.  Does the
kernel interface need to change now?  Again, seems like it shouldn't
change.

> 
> For voltage monitoring, that is not normally the case. It is much more likely
> that there is in fact a hardware monitoring or power control chip, the
> (or an) alarm output of that chip is connected to a gpio line, and its control
> interface is connected. If so, the driver for that chip should be enhanced
> to support interrupts, and to report the status with its own sysfs attributes.

I agree that writing a specialized driver that pretends a hwmon chip
with a control interface is just a gpio wouldn't be appropriate as an
upstreamable driver for the kernel.  It's more of a one off hack of
expediency.

But it's pretty easy to make a voltage alarm circuit with an op amp. 
Even a differential temperature sensor with hysteresis is just a few
components.

Would a hwmon driver for this be unacceptable?

  parent reply	other threads:[~2018-10-30 18:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-29 14:35 [PATCH v2 0/2] Add GPIO brownout detection support Marco Felsch
2018-10-29 14:35 ` [PATCH v2 1/2] dt-binding: hwmon: add gpio-brownout bindings Marco Felsch
2018-10-29 14:35 ` [PATCH v2 2/2] hwmon: add generic GPIO brownout support Marco Felsch
2018-10-29 19:52   ` Guenter Roeck
2018-10-29 21:16     ` Trent Piepho
2018-10-30  1:12       ` Guenter Roeck
2018-10-30 10:47         ` Marco Felsch
2018-10-30 13:13           ` Guenter Roeck
2018-10-30 17:00             ` Marco Felsch
2018-10-30 19:34               ` Trent Piepho
2018-10-30 20:11                 ` Guenter Roeck
2018-11-01 10:40                   ` Marco Felsch
2018-11-01 13:01                     ` Guenter Roeck
2018-11-01 14:53                       ` Marco Felsch
2018-11-01 15:14                         ` Guenter Roeck
2018-11-01 18:21                           ` Trent Piepho
2018-11-02  6:38                             ` Marco Felsch
2018-11-02 23:05                               ` Trent Piepho
2018-11-05  8:19                                 ` Marco Felsch
2018-11-06 20:50                                   ` Trent Piepho
2018-11-07  9:35                                     ` Marco Felsch
2018-11-07 18:07                                       ` Trent Piepho
2018-11-01 13:02                     ` Guenter Roeck
2018-11-01 14:58                       ` Marco Felsch
2018-11-01 15:08                         ` Guenter Roeck
2018-11-01 17:41                     ` Trent Piepho
2018-11-02  6:48                       ` Marco Felsch
2018-10-30 19:56               ` Guenter Roeck
2018-11-01  9:44                 ` Marco Felsch
2018-10-30 18:54           ` Trent Piepho
2018-10-30 18:49         ` Trent Piepho [this message]
2018-10-30 20:13           ` 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=1540925347.30311.65.camel@impinj.com \
    --to=tpiepho@impinj.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jdelvare@suse.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=m.felsch@pengutronix.de \
    /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