All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Trent Piepho <tpiepho@impinj.com>
Cc: "m.felsch@pengutronix.de" <m.felsch@pengutronix.de>,
	"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 13:13:09 -0700	[thread overview]
Message-ID: <20181030201309.GC28185@roeck-us.net> (raw)
In-Reply-To: <1540925347.30311.65.camel@impinj.com>

On Tue, Oct 30, 2018 at 06:49:07PM +0000, Trent Piepho wrote:
> 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?
> 
I think we are beyond that.

Guenter

> > > 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?

      reply	other threads:[~2018-10-31  5:08 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
2018-10-30 20:13           ` Guenter Roeck [this message]

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=20181030201309.GC28185@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jdelvare@suse.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=m.felsch@pengutronix.de \
    --cc=tpiepho@impinj.com \
    /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.