All of lore.kernel.org
 help / color / mirror / Atom feed
* GPIO Centralized Control Daemon
@ 2017-09-14 22:02 Patrick Venture
  2017-09-14 22:29 ` Rick Altherr
  2017-09-16 22:12 ` Brad Bishop
  0 siblings, 2 replies; 15+ messages in thread
From: Patrick Venture @ 2017-09-14 22:02 UTC (permalink / raw)
  To: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

I apologize if this already exists or is in the works.

I propose we create a daemon that centralizes userspace GPIO access.

From a high level, the daemon will implement interfaces to export or
unexport GPIOs.  Each GPIO exported will exist on the dbus:

/xyz/openbmc_project/gpio/53 and implement an interface with the following
properties:
value, direction, active_low, etc.

Some doubts, should the gpio name on the dbus be the relative name (53),
should it be the system specific name (G5) or the absolute name?  I'm
thinking the relative name and let the daemon handle internalizing the
adjustment from relative to absolute.

I'm working on adding GPIO support within phosphor-hwmon so that I can
access a voltage sensor that's gated by a GPIO, and I know there have been
conversations and implementations of this for IPMI OEM -- and I think it
could easily be centralized.

Thoughts?

Patrick

[-- Attachment #2: Type: text/html, Size: 1114 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-14 22:02 GPIO Centralized Control Daemon Patrick Venture
@ 2017-09-14 22:29 ` Rick Altherr
  2017-09-14 22:55   ` Patrick Venture
  2017-09-16 22:12 ` Brad Bishop
  1 sibling, 1 reply; 15+ messages in thread
From: Rick Altherr @ 2017-09-14 22:29 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

When you do this, please use the gpio chardev interface instead of sysfs.
Ask me for details if you aren't familiar.

On Thu, Sep 14, 2017 at 3:02 PM, Patrick Venture <venture@google.com> wrote:

> I apologize if this already exists or is in the works.
>
> I propose we create a daemon that centralizes userspace GPIO access.
>
> From a high level, the daemon will implement interfaces to export or
> unexport GPIOs.  Each GPIO exported will exist on the dbus:
>
> /xyz/openbmc_project/gpio/53 and implement an interface with the following
> properties:
> value, direction, active_low, etc.
>
> Some doubts, should the gpio name on the dbus be the relative name (53),
> should it be the system specific name (G5) or the absolute name?  I'm
> thinking the relative name and let the daemon handle internalizing the
> adjustment from relative to absolute.
>
> I'm working on adding GPIO support within phosphor-hwmon so that I can
> access a voltage sensor that's gated by a GPIO, and I know there have been
> conversations and implementations of this for IPMI OEM -- and I think it
> could easily be centralized.
>
> Thoughts?
>
> Patrick
>

[-- Attachment #2: Type: text/html, Size: 1666 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-14 22:29 ` Rick Altherr
@ 2017-09-14 22:55   ` Patrick Venture
  0 siblings, 0 replies; 15+ messages in thread
From: Patrick Venture @ 2017-09-14 22:55 UTC (permalink / raw)
  To: Rick Altherr; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Thu, Sep 14, 2017 at 3:29 PM, Rick Altherr <raltherr@google.com> wrote:

> When you do this, please use the gpio chardev interface instead of sysfs.
> Ask me for details if you aren't familiar.
>

I'm familiar with it.  I've heard they're phasing out sysfs gpio.


>
> On Thu, Sep 14, 2017 at 3:02 PM, Patrick Venture <venture@google.com>
> wrote:
>
>> I apologize if this already exists or is in the works.
>>
>> I propose we create a daemon that centralizes userspace GPIO access.
>>
>> From a high level, the daemon will implement interfaces to export or
>> unexport GPIOs.  Each GPIO exported will exist on the dbus:
>>
>> /xyz/openbmc_project/gpio/53 and implement an interface with the
>> following properties:
>> value, direction, active_low, etc.
>>
>> Some doubts, should the gpio name on the dbus be the relative name (53),
>> should it be the system specific name (G5) or the absolute name?  I'm
>> thinking the relative name and let the daemon handle internalizing the
>> adjustment from relative to absolute.
>>
>> I'm working on adding GPIO support within phosphor-hwmon so that I can
>> access a voltage sensor that's gated by a GPIO, and I know there have been
>> conversations and implementations of this for IPMI OEM -- and I think it
>> could easily be centralized.
>>
>> Thoughts?
>>
>> Patrick
>>
>
>

[-- Attachment #2: Type: text/html, Size: 2335 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-14 22:02 GPIO Centralized Control Daemon Patrick Venture
  2017-09-14 22:29 ` Rick Altherr
@ 2017-09-16 22:12 ` Brad Bishop
  2017-09-17  4:22   ` Patrick Venture
  1 sibling, 1 reply; 15+ messages in thread
From: Brad Bishop @ 2017-09-16 22:12 UTC (permalink / raw)
  To: Patrick Venture, OpenBMC Maillist

Hi Patrick

On Thu, 2017-09-14 at 15:02 -0700, Patrick Venture wrote:
> I apologize if this already exists or is in the works.
> 
> I propose we create a daemon that centralizes userspace GPIO access.
> 
> From a high level, the daemon will implement interfaces to export or
> unexport GPIOs.  Each GPIO exported will exist on the dbus:
> 
> /xyz/openbmc_project/gpio/53 and implement an interface with the
> following properties:
> value, direction, active_low, etc.

Just thinking out loud...are we sure a gpio is a useful abstraction? 
If applications all just use the chardev API what is the benefit of
having a dbus object, at the cost of API complexity.

> 
> Some doubts, should the gpio name on the dbus be the relative name
> (53), should it be the system specific name (G5) or the absolute
> name?  I'm thinking the relative name and let the daemon handle
> internalizing the adjustment from relative to absolute.
> 
> I'm working on adding GPIO support within phosphor-hwmon so that I
> can access a voltage sensor that's gated by a GPIO, and I know there 

Can you elaborate on what gated means?  Is it you have to wiggle a GPIO
before the hwmon driver for this voltage sensor can actually access the
sensor hardware?


> have been conversations and implementations of this for IPMI OEM --
> and I think it could easily be centralized.
> 
> Thoughts?
> 
> Patrick

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-16 22:12 ` Brad Bishop
@ 2017-09-17  4:22   ` Patrick Venture
  2017-09-17 16:26     ` Brad Bishop
  2017-09-18  7:07     ` Andrew Jeffery
  0 siblings, 2 replies; 15+ messages in thread
From: Patrick Venture @ 2017-09-17  4:22 UTC (permalink / raw)
  To: Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]

On Sat, Sep 16, 2017 at 3:12 PM, Brad Bishop <bradleyb@fuzziesquirrel.com>
wrote:

> Hi Patrick
>
> On Thu, 2017-09-14 at 15:02 -0700, Patrick Venture wrote:
> > I apologize if this already exists or is in the works.
> >
> > I propose we create a daemon that centralizes userspace GPIO access.
> >
> > From a high level, the daemon will implement interfaces to export or
> > unexport GPIOs.  Each GPIO exported will exist on the dbus:
> >
> > /xyz/openbmc_project/gpio/53 and implement an interface with the
> > following properties:
> > value, direction, active_low, etc.
>
> Just thinking out loud...are we sure a gpio is a useful abstraction?
> If applications all just use the chardev API what is the benefit of
> having a dbus object, at the cost of API complexity.
>

As an alternative, perhaps a library would suffice.  My goal is to avoid
multiple in-application implementations.  However, I felt it might be
helpful to have some centralized daemon serving this information -- a
simple dbus property access may be simpler than either implementing the
ioctl in each application.  However, a library may also provide the
necessary abstraction.


>
> >
> > Some doubts, should the gpio name on the dbus be the relative name
> > (53), should it be the system specific name (G5) or the absolute
> > name?  I'm thinking the relative name and let the daemon handle
> > internalizing the adjustment from relative to absolute.
> >
> > I'm working on adding GPIO support within phosphor-hwmon so that I
> > can access a voltage sensor that's gated by a GPIO, and I know there
>
> Can you elaborate on what gated means?  Is it you have to wiggle a GPIO
> before the hwmon driver for this voltage sensor can actually access the
> sensor hardware?
>
> There is a GPIO that controls whether the battery sensor on the quanta
board (and I've heard of similar configurations on others) that needs to be
set high for the sensor to work.  I implemented a bit of a hack in
phosphor-hwmon to allow one to specify a GPIO that needs to be flipped in
this type of configuration.  I'd consider submitting the patch, however it
uses the sysfs interface -- as this saved a little implementation time.

I'd be willing to fix it up to use the proper interface and submit it
upstream if that's something helpful -- or seek out a good library for us
to use -- I'm also open to suggestions on this.

>
> > have been conversations and implementations of this for IPMI OEM --
> > and I think it could easily be centralized.
> >
> > Thoughts?
> >
> > Patrick
>

[-- Attachment #2: Type: text/html, Size: 3498 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17  4:22   ` Patrick Venture
@ 2017-09-17 16:26     ` Brad Bishop
  2017-09-17 16:39       ` Patrick Venture
  2017-09-18  7:07     ` Andrew Jeffery
  1 sibling, 1 reply; 15+ messages in thread
From: Brad Bishop @ 2017-09-17 16:26 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist

On Sat, 2017-09-16 at 21:22 -0700, Patrick Venture wrote:
> On Sat, Sep 16, 2017 at 3:12 PM, Brad Bishop
> <bradleyb@fuzziesquirrel.com> wrote:
> > Hi Patrick
> > 
> > On Thu, 2017-09-14 at 15:02 -0700, Patrick Venture wrote:
> > > I apologize if this already exists or is in the works.
> > >
> > > I propose we create a daemon that centralizes userspace GPIO
> > access.
> > >
> > > From a high level, the daemon will implement interfaces to export
> > or
> > > unexport GPIOs.  Each GPIO exported will exist on the dbus:
> > >
> > > /xyz/openbmc_project/gpio/53 and implement an interface with the
> > > following properties:
> > > value, direction, active_low, etc.
> > 
> > Just thinking out loud...are we sure a gpio is a useful
> > abstraction?
> > If applications all just use the chardev API what is the benefit of
> > having a dbus object, at the cost of API complexity.
> 
> As an alternative, perhaps a library would suffice.  My goal is to
> avoid multiple in-application implementations.  However, I felt it 

A good goal.

> might be helpful to have some centralized daemon serving this
> information -- a simple dbus property access may be simpler than
> either implementing the ioctl in each application.  However, a
> library may also provide the necessary abstraction.

I think if you implement a library with API it becomes a wash.  There
is also polling to consider - we have a number of applications using
gpio-keys + libevdev.

>  
> > >
> > > Some doubts, should the gpio name on the dbus be the relative
> > name
> > > (53), should it be the system specific name (G5) or the absolute
> > > name?  I'm thinking the relative name and let the daemon handle
> > > internalizing the adjustment from relative to absolute.
> > >
> > > I'm working on adding GPIO support within phosphor-hwmon so that
> > I
> > > can access a voltage sensor that's gated by a GPIO, and I know
> > there
> > 
> > Can you elaborate on what gated means?  Is it you have to wiggle a
> > GPIO
> > before the hwmon driver for this voltage sensor can actually access
> > the
> > sensor hardware?
> > 
> > 
> 
> There is a GPIO that controls whether the battery sensor on the
> quanta board (and I've heard of similar configurations on others)
> that needs to be set high for the sensor to work.  I implemented a
> bit of a hack in phosphor-hwmon to allow one to specify a GPIO that
> needs to be flipped in this type of configuration.  I'd consider
> submitting the patch, however it uses the sysfs interface -- as this
> saved a little implementation time.

We have run into situations like this multiple times.  What we did was
bind/unbind the driver in question when the hardware behind it becomes
available.

What do you think of this?

1 - You simply wiggle the gpio from a script or small application at
      the appropriate time.
2 - An application other than phosphor-hwmon monitors this gpio and
      binds the driver to it (or unbinds).
3 - phosphor-hwmon works without modification.

In fact, #2 is already done - Matt Spinler recently added code to
phosphor-gpio-monitor to do exactly this.

I prefer something like this for two reasons:

1 - Consistency in how we handle things.  It becomes easier to write
howtos or cookbooks to provide guidance in the future.

2 - We avoid one-offs in phosphor-hwmon.


> 
> I'd be willing to fix it up to use the proper interface and submit it
> upstream if that's something helpful -- or seek out a good library
> for us to use -- I'm also open to suggestions on this.
> > > have been conversations and implementations of this for IPMI OEM
> > --
> > > and I think it could easily be centralized.
> > >
> > > Thoughts?
> > >
> > > Patrick
> > 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 16:26     ` Brad Bishop
@ 2017-09-17 16:39       ` Patrick Venture
  2017-09-17 17:33         ` Brad Bishop
  0 siblings, 1 reply; 15+ messages in thread
From: Patrick Venture @ 2017-09-17 16:39 UTC (permalink / raw)
  To: Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 4551 bytes --]

On Sun, Sep 17, 2017 at 9:26 AM, Brad Bishop <bradleyb@fuzziesquirrel.com>
wrote:

> On Sat, 2017-09-16 at 21:22 -0700, Patrick Venture wrote:
> > On Sat, Sep 16, 2017 at 3:12 PM, Brad Bishop
> > <bradleyb@fuzziesquirrel.com> wrote:
> > > Hi Patrick
> > >
> > > On Thu, 2017-09-14 at 15:02 -0700, Patrick Venture wrote:
> > > > I apologize if this already exists or is in the works.
> > > >
> > > > I propose we create a daemon that centralizes userspace GPIO
> > > access.
> > > >
> > > > From a high level, the daemon will implement interfaces to export
> > > or
> > > > unexport GPIOs.  Each GPIO exported will exist on the dbus:
> > > >
> > > > /xyz/openbmc_project/gpio/53 and implement an interface with the
> > > > following properties:
> > > > value, direction, active_low, etc.
> > >
> > > Just thinking out loud...are we sure a gpio is a useful
> > > abstraction?
> > > If applications all just use the chardev API what is the benefit of
> > > having a dbus object, at the cost of API complexity.
> >
> > As an alternative, perhaps a library would suffice.  My goal is to
> > avoid multiple in-application implementations.  However, I felt it
>
> A good goal.
>
> > might be helpful to have some centralized daemon serving this
> > information -- a simple dbus property access may be simpler than
> > either implementing the ioctl in each application.  However, a
> > library may also provide the necessary abstraction.
>
> I think if you implement a library with API it becomes a wash.  There
> is also polling to consider - we have a number of applications using
> gpio-keys + libevdev.
>
> >
> > > >
> > > > Some doubts, should the gpio name on the dbus be the relative
> > > name
> > > > (53), should it be the system specific name (G5) or the absolute
> > > > name?  I'm thinking the relative name and let the daemon handle
> > > > internalizing the adjustment from relative to absolute.
> > > >
> > > > I'm working on adding GPIO support within phosphor-hwmon so that
> > > I
> > > > can access a voltage sensor that's gated by a GPIO, and I know
> > > there
> > >
> > > Can you elaborate on what gated means?  Is it you have to wiggle a
> > > GPIO
> > > before the hwmon driver for this voltage sensor can actually access
> > > the
> > > sensor hardware?
> > >
> > >
> >
> > There is a GPIO that controls whether the battery sensor on the
> > quanta board (and I've heard of similar configurations on others)
> > that needs to be set high for the sensor to work.  I implemented a
> > bit of a hack in phosphor-hwmon to allow one to specify a GPIO that
> > needs to be flipped in this type of configuration.  I'd consider
> > submitting the patch, however it uses the sysfs interface -- as this
> > saved a little implementation time.
>
> We have run into situations like this multiple times.  What we did was
> bind/unbind the driver in question when the hardware behind it becomes
> available.
>
> What do you think of this?
>
> 1 - You simply wiggle the gpio from a script or small application at
>       the appropriate time.
>

My thought here is -- the appropriate time is, right before phosphor-hwmon
wants to read it.


> 2 - An application other than phosphor-hwmon monitors this gpio and
>       binds the driver to it (or unbinds).
>

In this case, won't the device be removed/added to the hwmon sysfs on
demand?  I don't believe phosphor-hwmon picks up new devices automatically.


> 3 - phosphor-hwmon works without modification.
>
> In fact, #2 is already done - Matt Spinler recently added code to
> phosphor-gpio-monitor to do exactly this.
>

I haven't looked at gpio-monitor.


>
> I prefer something like this for two reasons:
>
> 1 - Consistency in how we handle things.  It becomes easier to write
> howtos or cookbooks to provide guidance in the future.
>


> 2 - We avoid one-offs in phosphor-hwmon.
>

I wouldn't necessarily consider it a one-off, as I can immediately think of
at least one other motherboard that would benefit from such an
opportunity.  Also, in this case it would all fit into one nice package --
one thing to configure by adding the corresponding line to a configuration
file.


>
>
> >
> > I'd be willing to fix it up to use the proper interface and submit it
> > upstream if that's something helpful -- or seek out a good library
> > for us to use -- I'm also open to suggestions on this.
> > > > have been conversations and implementations of this for IPMI OEM
> > > --
> > > > and I think it could easily be centralized.
> > > >
> > > > Thoughts?
> > > >
> > > > Patrick
> > >
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 6553 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 16:39       ` Patrick Venture
@ 2017-09-17 17:33         ` Brad Bishop
  2017-09-17 21:04           ` Patrick Venture
  0 siblings, 1 reply; 15+ messages in thread
From: Brad Bishop @ 2017-09-17 17:33 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist


> > 
> > What do you think of this?
> > 
> > 1 - You simply wiggle the gpio from a script or small application
> > at
> >       the appropriate time.
> 
> My thought here is -- the appropriate time is, right before phosphor-
> hwmon wants to read it.

phosphor-hwmon shouldn't be providing an interface for something
that isn't accessible.  And it shouldn't concern itself with
making something accessible because there could be any number of
ways to do that, based on the wiring of the board.

What I meant was why is the sensor gated by a gpio?  For example, is it
because the sensor is only available when some power domain is actually
powered on?  In that example, the appropriate time would be when
the power domain is turned on and off.

Is it just stuck behind a gpio for no reason?  If that is the case, I
would think you just wiggle the gpio when the bmc boots.

>  
> > 2 - An application other than phosphor-hwmon monitors this gpio and
> >       binds the driver to it (or unbinds).
> 
> In this case, won't the device be removed/added to the hwmon sysfs on
> demand?  I don't believe phosphor-hwmon picks up new devices
> automatically.

Yes, on demand, but he phosphor-layer has a udev rule that launches
(and terminates) a phosphor-hwmon instance along with it.

>  
> > 3 - phosphor-hwmon works without modification.
> > 
> > In fact, #2 is already done - Matt Spinler recently added code to
> > phosphor-gpio-monitor to do exactly this.
> 
> I haven't looked at gpio-monitor.
>  
> > I prefer something like this for two reasons:
> > 
> > 1 - Consistency in how we handle things.  It becomes easier to
> > write
> > howtos or cookbooks to provide guidance in the future. 
> 
>  
> > 2 - We avoid one-offs in phosphor-hwmon.
> 
> I wouldn't necessarily consider it a one-off, as I can immediately
> think of at least one other motherboard that would benefit from such
> an opportunity.  Also, in this case it would all fit into one nice
> package -- one thing to configure by adding the corresponding line to
> a configuration file.

I'm not arguing that there aren't several board setups where this is
the case.  Rather, board designers are free to do whatever they want,
and if we start putting the board specific logic in phosphor-hwmon you
won't have "one nice package" for very long....

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 17:33         ` Brad Bishop
@ 2017-09-17 21:04           ` Patrick Venture
  2017-09-17 21:38             ` Brad Bishop
  0 siblings, 1 reply; 15+ messages in thread
From: Patrick Venture @ 2017-09-17 21:04 UTC (permalink / raw)
  To: Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 4110 bytes --]

On Sun, Sep 17, 2017 at 10:33 AM, Brad Bishop <bradleyb@fuzziesquirrel.com>
wrote:

>
> > >
> > > What do you think of this?
> > >
> > > 1 - You simply wiggle the gpio from a script or small application
> > > at
> > >       the appropriate time.
> >
> > My thought here is -- the appropriate time is, right before phosphor-
> > hwmon wants to read it.
>
> phosphor-hwmon shouldn't be providing an interface for something
> that isn't accessible.  And it shouldn't concern itself with
> making something accessible because there could be any number of
> ways to do that, based on the wiring of the board.
>
> What I meant was why is the sensor gated by a gpio?  For example, is it
> because the sensor is only available when some power domain is actually
> powered on?  In that example, the appropriate time would be when
> the power domain is turned on and off.
>
> Is it just stuck behind a gpio for no reason?  If that is the case, I
> would think you just wiggle the gpio when the bmc boots.
>

In this particular case, it's behind a GPIO so it doesn't drain the battery
-- or at least that's my understanding of the design.

The appropriate time would be -- when someone tries to read it from the
host.  What you're suggesting is that, the ipmi daemon, when it receives a
request for this sensor - that is then launches something that sets the
gpio, and then waits for the dbus objects to exist from the launched
phosphor-hwmon, before then requesting the sensor value in its normal
course of action.  Because it's my understanding it's a gated sensor so
that it doesn't drain the battery, which means leaving it open effectively
permanently isn't an option.  If I'm misunderstanding the goal of the
design, then obviously just unlocking the GPIO effectively permanently is a
solution.


>
> >
> > > 2 - An application other than phosphor-hwmon monitors this gpio and
> > >       binds the driver to it (or unbinds).
> >
> > In this case, won't the device be removed/added to the hwmon sysfs on
> > demand?  I don't believe phosphor-hwmon picks up new devices
> > automatically.
>
> Yes, on demand, but he phosphor-layer has a udev rule that launches
> (and terminates) a phosphor-hwmon instance along with it.
>
> >
> > > 3 - phosphor-hwmon works without modification.
> > >
> > > In fact, #2 is already done - Matt Spinler recently added code to
> > > phosphor-gpio-monitor to do exactly this.
> >
> > I haven't looked at gpio-monitor.
> >
> > > I prefer something like this for two reasons:
> > >
> > > 1 - Consistency in how we handle things.  It becomes easier to
> > > write
> > > howtos or cookbooks to provide guidance in the future.
> >
> >
> > > 2 - We avoid one-offs in phosphor-hwmon.
> >
> > I wouldn't necessarily consider it a one-off, as I can immediately
> > think of at least one other motherboard that would benefit from such
> > an opportunity.  Also, in this case it would all fit into one nice
> > package -- one thing to configure by adding the corresponding line to
> > a configuration file.
>
> I'm not arguing that there aren't several board setups where this is
> the case.  Rather, board designers are free to do whatever they want,
> and if we start putting the board specific logic in phosphor-hwmon you
> won't have "one nice package" for very long....
>

True-ish.  This is quite similar to dealing with GAIN and OFFSET.  Those
are particulars of a board that now get factored into the value.  I'm not
suggesting something beyond the norms of hwmon.  It would be convenient
however, if the ADC driver could be programmed to know that some sensor is
gated by a GPIO and handled flipping, waiting, reading, and flipping-back
for someone -- quasi-similarly to i2c muxes that use gpios.  Except,
obviously, the waiting and setting it back part are ... a bad plan to build
into a driver, in that it could delay returning a value.  Although -- one
could argue the fan speed controller driver does that -- takes time to
respond.  It's just, one would then also want to configure the wait time,
because some configurations will take longer for a sensor value to
stabilize.

Patrick

[-- Attachment #2: Type: text/html, Size: 5186 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 21:04           ` Patrick Venture
@ 2017-09-17 21:38             ` Brad Bishop
  2017-09-17 21:49               ` Patrick Venture
  0 siblings, 1 reply; 15+ messages in thread
From: Brad Bishop @ 2017-09-17 21:38 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist


> > Is it just stuck behind a gpio for no reason?  If that is the case,
> > I
> > would think you just wiggle the gpio when the bmc boots.
> 
> In this particular case, it's behind a GPIO so it doesn't drain the
> battery -- or at least that's my understanding of the design.
> 
> The appropriate time would be -- when someone tries to read it from
> the host.  What you're suggesting is that, the ipmi daemon, when it
> receives a request for this sensor - that is then launches something
> that sets the gpio, and then waits for the dbus objects to exist from
> the launched phosphor-hwmon, before then requesting the sensor value
> in its normal course of action.  Because it's my understanding it's a
> gated sensor so that it doesn't drain the battery, which means
> leaving it open effectively permanently isn't an option.  If I'm
> misunderstanding the goal of the design, then obviously just
> unlocking the GPIO effectively permanently is a solution.

Aha!  Now I understand the motivation.  And yes the approach I've
outlined is quite painful.  In my defense I would not have suggested
that had I known :-)

> > 
> > I'm not arguing that there aren't several board setups where this
> > is
> > the case.  Rather, board designers are free to do whatever they
> > want,
> > and if we start putting the board specific logic in phosphor-hwmon
> > you
> > won't have "one nice package" for very long....
> 
> True-ish.  This is quite similar to dealing with GAIN and OFFSET.  

Those feel different to me.  They are needed for thermistor type
sensors regardless of board wiring.

> Those are particulars of a board that now get factored into the
> value.  I'm not suggesting something beyond the norms of hwmon.  It
> would be convenient however, if the ADC driver could be programmed to
> know that some sensor is gated by a GPIO and handled flipping, 

Can you reach out to Guenter or the relevant maintainer (is this an IIO
device?) for ideas here?  I hadn't mentioned it but in the back of my
head this whole time I've been wondering about a completely in-kernel
solution.

> waiting, reading, and flipping-back for someone -- quasi-similarly to
> i2c muxes that use gpios.  Except, obviously, the waiting and setting
> it back part are ... a bad plan to build into a driver, in that it 

It has to be done in one place or the other, does it not?  You mention
i2c muxes - managing those isn't delegated to userspace (there are
drivers for i2c muxes) so maybe this situation need not be any
different?

> could delay returning a value.  Although -- one could argue the fan
> speed controller driver does that -- takes time to respond.  It's
> just, one would then also want to configure the wait time, because 

In the very near future I intend to make all the wait/delay/retry
intervals runtime configurable to handle exactly these kinds of nuances
between devices and their drivers.

> some configurations will take longer for a sensor value to stabilize.
> 
> Patrick
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 21:38             ` Brad Bishop
@ 2017-09-17 21:49               ` Patrick Venture
  2017-09-17 22:10                 ` Brad Bishop
  2017-09-18  7:32                 ` Andrew Jeffery
  0 siblings, 2 replies; 15+ messages in thread
From: Patrick Venture @ 2017-09-17 21:49 UTC (permalink / raw)
  To: Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 4423 bytes --]

On Sun, Sep 17, 2017 at 2:38 PM, Brad Bishop <bradleyb@fuzziesquirrel.com>
wrote:

>
> > > Is it just stuck behind a gpio for no reason?  If that is the case,
> > > I
> > > would think you just wiggle the gpio when the bmc boots.
> >
> > In this particular case, it's behind a GPIO so it doesn't drain the
> > battery -- or at least that's my understanding of the design.
> >
> > The appropriate time would be -- when someone tries to read it from
> > the host.  What you're suggesting is that, the ipmi daemon, when it
> > receives a request for this sensor - that is then launches something
> > that sets the gpio, and then waits for the dbus objects to exist from
> > the launched phosphor-hwmon, before then requesting the sensor value
> > in its normal course of action.  Because it's my understanding it's a
> > gated sensor so that it doesn't drain the battery, which means
> > leaving it open effectively permanently isn't an option.  If I'm
> > misunderstanding the goal of the design, then obviously just
> > unlocking the GPIO effectively permanently is a solution.
>
> Aha!  Now I understand the motivation.  And yes the approach I've
> outlined is quite painful.  In my defense I would not have suggested
> that had I known :-)
>

Ahh - yeah, sorry, I didn't explain the entire use-case path initially.  I
assumed gpio-gated sensors were just a normal thing.

>
> > >
> > > I'm not arguing that there aren't several board setups where this
> > > is
> > > the case.  Rather, board designers are free to do whatever they
> > > want,
> > > and if we start putting the board specific logic in phosphor-hwmon
> > > you
> > > won't have "one nice package" for very long....
> >
> > True-ish.  This is quite similar to dealing with GAIN and OFFSET.
>
> Those feel different to me.  They are needed for thermistor type
> sensors regardless of board wiring.
>
> > Those are particulars of a board that now get factored into the
> > value.  I'm not suggesting something beyond the norms of hwmon.  It
> > would be convenient however, if the ADC driver could be programmed to
> > know that some sensor is gated by a GPIO and handled flipping,
>
> Can you reach out to Guenter or the relevant maintainer (is this an IIO
> device?) for ideas here?  I hadn't mentioned it but in the back of my
> head this whole time I've been wondering about a completely in-kernel
> solution.
>

I'll reach out and see if this should go into the adc driver or the
iio-hwmon.  Since those are the paths that are involved.  The sensor itself
it plugged into the aspeed adc, and then controlled by a gpio.  My initial
thought is the adc should be configured to know if such things are required
for its sensors, so that iio-hwmon wouldn't need to know.

>
> > waiting, reading, and flipping-back for someone -- quasi-similarly to
> > i2c muxes that use gpios.  Except, obviously, the waiting and setting
> > it back part are ... a bad plan to build into a driver, in that it
>
> It has to be done in one place or the other, does it not?  You mention
> i2c muxes - managing those isn't delegated to userspace (there are
> drivers for i2c muxes) so maybe this situation need not be any
> different?
>

That's what I'm thinking, if I can put it in the driver as something that
can be configured - then perhaps that'll be a neater solution.  Although,
depending on how it gets added, it might be too platform specific.  I'm
using an Aspeed part, but I know there are other manufacturers, and if this
type of arrangement exists elsewhere, more drivers will need modification
in-lieu of a solution at the highest level.  I see the value in both
approaches.

Downstream I have a gpio addition to phosphor-hwmon -- that I'll hold off
on submitting.  I'm going to pursue the modifications to the ADC, I think
they might be appropriate at that level, and if I recall I known the author
and may be able to rapidly implement the necessary modification myself.

>
> > could delay returning a value.  Although -- one could argue the fan
> > speed controller driver does that -- takes time to respond.  It's
> > just, one would then also want to configure the wait time, because
>
> In the very near future I intend to make all the wait/delay/retry
> intervals runtime configurable to handle exactly these kinds of nuances
> between devices and their drivers.
>
> > some configurations will take longer for a sensor value to stabilize.
> >
> > Patrick
> >
>

[-- Attachment #2: Type: text/html, Size: 5829 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 21:49               ` Patrick Venture
@ 2017-09-17 22:10                 ` Brad Bishop
  2017-09-18  7:32                 ` Andrew Jeffery
  1 sibling, 0 replies; 15+ messages in thread
From: Brad Bishop @ 2017-09-17 22:10 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist

On Sun, 2017-09-17 at 14:49 -0700, Patrick Venture wrote:
> 
> Ahh - yeah, sorry, I didn't explain the entire use-case path
> initially.  I assumed gpio-gated sensors were just a normal thing. 

No problemo!

> I'll reach out and see if this should go into the adc driver or the
> iio-hwmon.  Since those are the paths that are involved.  The sensor
> itself it plugged into the aspeed adc, and then controlled by a
> gpio.  My initial thought is the adc should be configured to know if
> such things are required for its sensors, so that iio-hwmon wouldn't
> need to know.

Excellent.  Thx Patrick.

> That's what I'm thinking, if I can put it in the driver as something
> that can be configured - then perhaps that'll be a neater solution. 
> Although, depending on how it gets added, it might be too platform
> specific.  I'm using an Aspeed part, but I know there are other
> manufacturers, and if this type of arrangement exists elsewhere, more
> drivers will need modification in-lieu of a solution at the highest
> level.  I see the value in both approaches.

I can too.  Adding to that I'd like for our standard operating
procedure in general to be that we work with upstream Linux to set the
kernel direction and then we react to that to meet OpenBMCs needs.

-brad

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17  4:22   ` Patrick Venture
  2017-09-17 16:26     ` Brad Bishop
@ 2017-09-18  7:07     ` Andrew Jeffery
  1 sibling, 0 replies; 15+ messages in thread
From: Andrew Jeffery @ 2017-09-18  7:07 UTC (permalink / raw)
  To: Patrick Venture, Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

On Sat, 2017-09-16 at 21:22 -0700, Patrick Venture wrote:
> > > From a high level, the daemon will implement interfaces to export
> > or
> > > unexport GPIOs.  Each GPIO exported will exist on the dbus:
> > >
> > > /xyz/openbmc_project/gpio/53 and implement an interface with the
> > > following properties:
> > > value, direction, active_low, etc.
> > 
> > Just thinking out loud...are we sure a gpio is a useful
> > abstraction?
> > If applications all just use the chardev API what is the benefit of
> > having a dbus object, at the cost of API complexity.
> 
> As an alternative, perhaps a library would suffice.  My goal is to
> avoid multiple in-application implementations.  However, I felt it
> might be helpful to have some centralized daemon serving this
> information -- a simple dbus property access may be simpler than
> either implementing the ioctl in each application.  However, a
> library may also provide the necessary abstraction.
>  

If we do need a library, there's this:

    https://github.com/brgl/libgpiod

We should try to make use of that before doing something ourselves.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-17 21:49               ` Patrick Venture
  2017-09-17 22:10                 ` Brad Bishop
@ 2017-09-18  7:32                 ` Andrew Jeffery
  2017-09-18 14:52                   ` Patrick Venture
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Jeffery @ 2017-09-18  7:32 UTC (permalink / raw)
  To: Patrick Venture, Brad Bishop; +Cc: OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

On Sun, 2017-09-17 at 14:49 -0700, Patrick Venture wrote:
> > > Those are particulars of a board that now get factored into the
> > > value.  I'm not suggesting something beyond the norms of hwmon.  It
> > > would be convenient however, if the ADC driver could be programmed to
> > > know that some sensor is gated by a GPIO and handled flipping,
> > 
> > Can you reach out to Guenter or the relevant maintainer (is this an IIO
> > device?) for ideas here?  I hadn't mentioned it but in the back of my
> > head this whole time I've been wondering about a completely in-kernel
> > solution.
> 
> I'll reach out and see if this should go into the adc driver or the
> iio-hwmon.  Since those are the paths that are involved.  The sensor
> itself it plugged into the aspeed adc, and then controlled by a
> gpio.  My initial thought is the adc should be configured to know if
> such things are required for its sensors, so that iio-hwmon wouldn't
> need to know.

My gut feeling is the other way around. There should be some higher-
level driver (i.e. still in the kernel, not in userspace) coordinating
the GPIO and the ADC such that we sample values at the appropriate time
after the GPIO has been set. The ADC driver should be well defined in
terms of the function the ADC hardware provides - gluing in random
GPIOs makes the ADC driver do things the ADC hardware itself does not
do.

I'm not across how the iio-hwmon bridge works so I don't know how
trivial it would be to re-export the sampled ADC values from this
higher-level driver as another IIO channel, but that's probably the
approach I would look at to glue it into hwmon.

That's all just off-the-cuff opinion though, so take it with a grain of
salt. Certainly talking to the IIO and hwmon maintainers is the right
approach.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: GPIO Centralized Control Daemon
  2017-09-18  7:32                 ` Andrew Jeffery
@ 2017-09-18 14:52                   ` Patrick Venture
  0 siblings, 0 replies; 15+ messages in thread
From: Patrick Venture @ 2017-09-18 14:52 UTC (permalink / raw)
  To: Andrew Jeffery; +Cc: Brad Bishop, OpenBMC Maillist

[-- Attachment #1: Type: text/plain, Size: 2465 bytes --]

On Mon, Sep 18, 2017 at 12:32 AM, Andrew Jeffery <andrew@aj.id.au> wrote:

> On Sun, 2017-09-17 at 14:49 -0700, Patrick Venture wrote:
> > > > Those are particulars of a board that now get factored into the
> > > > value.  I'm not suggesting something beyond the norms of hwmon.  It
> > > > would be convenient however, if the ADC driver could be programmed to
> > > > know that some sensor is gated by a GPIO and handled flipping,
> > >
> > > Can you reach out to Guenter or the relevant maintainer (is this an IIO
> > > device?) for ideas here?  I hadn't mentioned it but in the back of my
> > > head this whole time I've been wondering about a completely in-kernel
> > > solution.
> >
> > I'll reach out and see if this should go into the adc driver or the
> > iio-hwmon.  Since those are the paths that are involved.  The sensor
> > itself it plugged into the aspeed adc, and then controlled by a
> > gpio.  My initial thought is the adc should be configured to know if
> > such things are required for its sensors, so that iio-hwmon wouldn't
> > need to know.
>
> My gut feeling is the other way around. There should be some higher-
> level driver (i.e. still in the kernel, not in userspace) coordinating
> the GPIO and the ADC such that we sample values at the appropriate time
> after the GPIO has been set. The ADC driver should be well defined in
> terms of the function the ADC hardware provides - gluing in random
> GPIOs makes the ADC driver do things the ADC hardware itself does not
> do.
>

Interesting.  That does make sense, however, one often sees -- some level
of "oh and this" configuration with drivers.  Which I'm in general against,
but it does have a certain cleanness to putting the value update at the
lowest point possible. -- Although, that could easily mean the earliest
place it is read, which is in the hwmon interface -- it can know do flip a
gpio and wait, whereas the ADC just keeps on doing its thing.


>
> I'm not across how the iio-hwmon bridge works so I don't know how
> trivial it would be to re-export the sampled ADC values from this
> higher-level driver as another IIO channel, but that's probably the
> approach I would look at to glue it into hwmon.
>
> That's all just off-the-cuff opinion though, so take it with a grain of
> salt. Certainly talking to the IIO and hwmon maintainers is the right
> approach.
>

I'll be broaching this subject in the next few days and will report back
here.


>
> Cheers,
>
> Andrew

[-- Attachment #2: Type: text/html, Size: 3355 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-09-18 14:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14 22:02 GPIO Centralized Control Daemon Patrick Venture
2017-09-14 22:29 ` Rick Altherr
2017-09-14 22:55   ` Patrick Venture
2017-09-16 22:12 ` Brad Bishop
2017-09-17  4:22   ` Patrick Venture
2017-09-17 16:26     ` Brad Bishop
2017-09-17 16:39       ` Patrick Venture
2017-09-17 17:33         ` Brad Bishop
2017-09-17 21:04           ` Patrick Venture
2017-09-17 21:38             ` Brad Bishop
2017-09-17 21:49               ` Patrick Venture
2017-09-17 22:10                 ` Brad Bishop
2017-09-18  7:32                 ` Andrew Jeffery
2017-09-18 14:52                   ` Patrick Venture
2017-09-18  7:07     ` Andrew Jeffery

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.