All of lore.kernel.org
 help / color / mirror / Atom feed
* Question about regulator API
@ 2016-12-14 14:52 Harald Geyer
  2016-12-14 18:14 ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Harald Geyer @ 2016-12-14 14:52 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown; +Cc: linux-kernel

Hi all!

I have a quite complex situation with regulator consumers, which I'm not
sure how to handle best.

Suppose there is some quirky HW that after some random time crashes - i.e.
it doesn't respond to requests anymore at all. The driver can detect this
situation and the only way to fix it is to disable the supply for 2 seconds,
then enable it again. Also after enabling the power supply the driver needs
to wait 2 seconds before talking to the HW.

Now the complicated part: The HW is some kind of sensor which often is
used in an array of identical devices. To make the wiring simpler all
these devices are attatched to the same supply (and maybe the same bus),
however they are read independently and thus also are independent devices
from the kernel POV.

Thus the following constraints should be met:
* When user space asks the driver to read a device, it needs to "claim"
  the supply and ensure that it has been up for at least 2 seconds
  before proceeding to read the HW. The supply would be "locked" enabled.
  (I think this is standard regulator API.)
* When HW failure is detected, the driver needs to tell the supply to
  turn off for at least 2 seconds, wait for all other devices to release
  the supply so it can actually be turned off, then wait for the off period,
  then start over again.
  (This is easy only with getting the supply exclusively.)
* It should be possible to read multiple devices quickly when everything
  is okay and working. (Having 6 2-second delays accumulate would be quite
  annoying.)
  (This won't work with exclusive supply usage.)
* Optionally: If all devices are idle the supply would be enabled if short
  response time is desired, but disabled if power saving is desired.

Can this somehow be solved with the existing API?
If not, do you think it would be reasonable/possible to extend the API
to cover situations like the one described above?

Any help or hints are appreciated.

TIA,
Harald

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

* Re: Question about regulator API
  2016-12-14 14:52 Question about regulator API Harald Geyer
@ 2016-12-14 18:14 ` Mark Brown
  2016-12-16 13:41   ` Harald Geyer
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2016-12-14 18:14 UTC (permalink / raw)
  To: Harald Geyer; +Cc: Liam Girdwood, linux-kernel

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

On Wed, Dec 14, 2016 at 03:52:54PM +0100, Harald Geyer wrote:

> Thus the following constraints should be met:
> * When user space asks the driver to read a device, it needs to "claim"
>   the supply and ensure that it has been up for at least 2 seconds
>   before proceeding to read the HW. The supply would be "locked" enabled.
>   (I think this is standard regulator API.)
> * When HW failure is detected, the driver needs to tell the supply to
>   turn off for at least 2 seconds, wait for all other devices to release
>   the supply so it can actually be turned off, then wait for the off period,
>   then start over again.
>   (This is easy only with getting the supply exclusively.)

You need to use a notification to figure out when the supply is actually
off.

> * It should be possible to read multiple devices quickly when everything
>   is okay and working. (Having 6 2-second delays accumulate would be quite
>   annoying.)
>   (This won't work with exclusive supply usage.)

Similarly using a notification to discover when the supply is on would
help here.

> * Optionally: If all devices are idle the supply would be enabled if short
>   response time is desired, but disabled if power saving is desired.
> 
> Can this somehow be solved with the existing API?
> If not, do you think it would be reasonable/possible to extend the API
> to cover situations like the one described above?

This doesn't feel like a regulator API problem exactly, a lot of what
you're talking about here seems like you really need the devices to
coopereate with each other and know what they're doing in order to work
well together.  From a regulator API point of view my first thought is
to use notifications to hook into the actual power on/off transitions
and then expressing the bit where the devices all work together at a
higher level.  It may be that a lot of that higher level coordination
just falls out of normal usage patterns do doesn't need explicitly
implementing, assuming the devices are only powered up during reads
anyway.  Using regulator_disable_deferred() in the driver may help
grease the wheels in terms of avoiding needless power bounces and
delays - just wait a little while before powering down in case you need
to power up again very soon after.

You'd end up with the devices all ignoring each other but keeping track
of when the supply was last enabled and disabled and individually
keeping timers to make sure that the needed delays are taken care of.
Userspace would then turn up and read all the devices, they'd then do
the enables and disables as though they were working alone but
coordinate through the notifications.  If device A powered things up
then device B would know the power was already on via the notifications
and when it came on so could take account of that when doing its own
delays.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: Question about regulator API
  2016-12-14 18:14 ` Mark Brown
@ 2016-12-16 13:41   ` Harald Geyer
  2016-12-16 16:53     ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Harald Geyer @ 2016-12-16 13:41 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, linux-kernel

Mark Brown writes:
> On Wed, Dec 14, 2016 at 03:52:54PM +0100, Harald Geyer wrote:
> 
> > Thus the following constraints should be met:
> > * When user space asks
> >   the driver to read a device, it needs to "claim" the supply and
> >   ensure that it has been up for at least 2 seconds before proceeding
> >   to read the HW. The supply would be "locked" enabled.
> >   (I think this is standard regulator API.)
> > * When HW failure is detected, the driver needs to tell the supply to
> >   turn off for at least 2 seconds, wait for all other devices to
> >   release the supply so it can actually be turned off, then wait
> >   for the off period, then start over again.
> >   (This is easy only with getting the supply exclusively.)
> 
> You need to use a notification to figure out when the supply is actually
> off.

I see.
 
> > * It should be possible to read multiple devices quickly when
> >   everything is okay and working. (Having 6 2-second delays accumulate
> >   would be quite annoying.)
> >   (This won't work with exclusive supply usage.)
> 
> Similarly using a notification to discover when the supply is on would
> help here.

Ok.
 
> > * Optionally: If all devices are idle the supply would be enabled
> > if short response time is desired, but disabled if power saving is
> > desired.
> >
> > Can this somehow be solved with the existing API? If not, do you think
> > it would be reasonable/possible to extend the API to cover situations
> > like the one described above?
> 
> This doesn't feel like a regulator API problem exactly, a lot of what
> you're talking about here seems like you really need the devices to
> coopereate with each other and know what they're doing in order to work
> well together.

I was hoping, that I somehow could get the necessary coordination from
the regulator framework. If the best I can get ATM is notifications, then
I'll try it and see what kind of code falls out of this.

It still seems a bit of a limitation to me, that the only way to really
switch off a regulator is with regulator_force_disable(), which is quite
a hard hammer.

> You'd end up with the devices all ignoring each other but keeping track
> of when the supply was last enabled and disabled and individually
> keeping timers to make sure that the needed delays are taken care of.
> Userspace would then turn up and read all the devices, they'd then
> do the enables and disables as though they were working alone but
> coordinate through the notifications. If device A powered things up then
> device B would know the power was already on via the notifications and
> when it came on so could take account of that when doing its own delays.

This only works as long as every consumer of the supply is cooperating
(which is my personal use case but doesn't look very future proof). I guess
there has to be some pain for using quirky, unreliable HW... ;)

Thanks for your input, it has been very helpful!

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

* Re: Question about regulator API
  2016-12-16 13:41   ` Harald Geyer
@ 2016-12-16 16:53     ` Mark Brown
  2016-12-16 22:00       ` Harald Geyer
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2016-12-16 16:53 UTC (permalink / raw)
  To: Harald Geyer; +Cc: Liam Girdwood, linux-kernel

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

On Fri, Dec 16, 2016 at 02:41:42PM +0100, Harald Geyer wrote:
> Mark Brown writes:
> > On Wed, Dec 14, 2016 at 03:52:54PM +0100, Harald Geyer wrote:

> > This doesn't feel like a regulator API problem exactly, a lot of what
> > you're talking about here seems like you really need the devices to
> > coopereate with each other and know what they're doing in order to work
> > well together.

> I was hoping, that I somehow could get the necessary coordination from
> the regulator framework. If the best I can get ATM is notifications, then
> I'll try it and see what kind of code falls out of this.

> It still seems a bit of a limitation to me, that the only way to really
> switch off a regulator is with regulator_force_disable(), which is quite
> a hard hammer.

I really have no idea what sort of communication you're envisaging here
- powering supplies down when other devices are trying to use them is a
really serious thing with very substantial consequences for userspace,
if the devices aren't cooperating at a level higher than the regulator
API level it's unlikely to go well.

> This only works as long as every consumer of the supply is cooperating
> (which is my personal use case but doesn't look very future proof). I guess
> there has to be some pain for using quirky, unreliable HW... ;)

That's going to be the case no matter what I think.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: Question about regulator API
  2016-12-16 16:53     ` Mark Brown
@ 2016-12-16 22:00       ` Harald Geyer
  2016-12-19 11:09         ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Harald Geyer @ 2016-12-16 22:00 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, linux-kernel

Mark Brown writes:
> On Fri, Dec 16, 2016 at 02:41:42PM +0100, Harald Geyer wrote:
> > Mark Brown writes:
> > > On Wed, Dec 14, 2016 at 03:52:54PM +0100, Harald Geyer wrote:
> 
> > > This doesn't feel like a regulator API problem exactly, a lot of what
> > > you're talking about here seems like you really need the devices to
> > > coopereate with each other and know what they're doing in order to work
> > > well together.
> 
> > I was hoping, that I somehow could get the necessary coordination from
> > the regulator framework. If the best I can get ATM is notifications, then
> > I'll try it and see what kind of code falls out of this.

Ok, tried that and it turns out there is no notification at
regulator_enable() ATM. I guess you'd merge a patch that added this
notification?

> > It still seems a bit of a limitation to me, that the only way to really
> > switch off a regulator is with regulator_force_disable(), which is quite
> > a hard hammer.
> 
> I really have no idea what sort of communication you're envisaging here
> - powering supplies down when other devices are trying to use them is a
> really serious thing with very substantial consequences for userspace,

Sure, this is why regulator_force_disable() is the wrong tool for the job.
I was more looking for something (call it regulator_shutdown()) that
* blocked until the regulator is actually off
* made all calls to regulator_enable() from other consumers block (or fail)
* maybe sent notifications to consumers, so that they have a chance to
  cooperate and call regulator_disable() at their earliest convenience.
* was undone by a call to either regulator_disable() or regulator_enable()
  from the consumer that initially called regulator_shutdown()

I haven't explored this in detail - maybe there are good reasons not to
have it.

As a convenience API there could be something like regulator_enable_wait()
(mirrored by regulator_shutdown_wait()) to avoid having identical
notification callbacks and timers in every consumer device. Of course this
would require adding timing information to struct regulator. If nobody
besides me has any use for that code, then maybe it's not a good idea.

> if the devices aren't cooperating at a level higher than the regulator
> API level it's unlikely to go well.

The devices would need to cooperate in the sense, that they call
regulator_disable() whenever they don't strictly need the supply, so that
regulator_shutdown() won't block forever, of course. But this is a good
idea, in terms of energy saving, anyway. The devices wouldn't need to
know the (timing) constraints of other consumers of the same supply - so
it's no hard cooperation.

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

* Re: Question about regulator API
  2016-12-16 22:00       ` Harald Geyer
@ 2016-12-19 11:09         ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2016-12-19 11:09 UTC (permalink / raw)
  To: Harald Geyer; +Cc: Liam Girdwood, linux-kernel

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

On Fri, Dec 16, 2016 at 11:00:35PM +0100, Harald Geyer wrote:
> Mark Brown writes:

> > > I was hoping, that I somehow could get the necessary coordination from
> > > the regulator framework. If the best I can get ATM is notifications, then
> > > I'll try it and see what kind of code falls out of this.

> Ok, tried that and it turns out there is no notification at
> regulator_enable() ATM. I guess you'd merge a patch that added this
> notification?

Probably.

> Sure, this is why regulator_force_disable() is the wrong tool for the job.
> I was more looking for something (call it regulator_shutdown()) that
> * blocked until the regulator is actually off
> * made all calls to regulator_enable() from other consumers block (or fail)
> * maybe sent notifications to consumers, so that they have a chance to
>   cooperate and call regulator_disable() at their earliest convenience.
> * was undone by a call to either regulator_disable() or regulator_enable()
>   from the consumer that initially called regulator_shutdown()

> I haven't explored this in detail - maybe there are good reasons not to
> have it.

That sounds like a complete mess from a user point of view - unless the
other devices are actually cooperating it's going to have the same
effect as doing nothing at all except it'll block the initiating
consumer and possibly others doing enables, and as far as I can see it
doesn't solve your problems with needing the regulator to be powered off
for a specific amount of time.  If it made enable calls block it'd be
very disruptive for other users as they would suddenly find operations
taking vastly longer than they were expecting which could end up being
visible all the way back up to userspace.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2016-12-19 11:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14 14:52 Question about regulator API Harald Geyer
2016-12-14 18:14 ` Mark Brown
2016-12-16 13:41   ` Harald Geyer
2016-12-16 16:53     ` Mark Brown
2016-12-16 22:00       ` Harald Geyer
2016-12-19 11:09         ` Mark Brown

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.