Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1511341 > unrolled thread

Re: [RFC 1/3] regulator: core: Add over current changed event

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC 1/3] regulator: core: Add over current changed event Mark Brown <broonie@kernel.org> - 2016-10-28 20:40 +0200
    Re: [RFC 1/3] regulator: core: Add over current changed event Axel Haslam <ahaslam@baylibre.com> - 2016-10-28 21:50 +0200
      Re: [RFC 1/3] regulator: core: Add over current changed event Axel Haslam <ahaslam@baylibre.com> - 2016-10-29 11:00 +0200
      Re: [RFC 1/3] regulator: core: Add over current changed event Mark Brown <broonie@kernel.org> - 2016-10-29 20:50 +0200
        Re: [RFC 1/3] regulator: core: Add over current changed event Axel Haslam <ahaslam@baylibre.com> - 2016-10-30 13:10 +0100
          Re: [RFC 1/3] regulator: core: Add over current changed event Mark Brown <broonie@kernel.org> - 2016-10-31 17:30 +0100
            Re: [RFC 1/3] regulator: core: Add over current changed event Axel Haslam <ahaslam@baylibre.com> - 2016-11-01 16:50 +0100

#1511341 — Re: [RFC 1/3] regulator: core: Add over current changed event

[Multipart message — attachments visible in raw view] — view raw

On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
> 
> Regulator consumers may be interested to know when the
> over current condition is over.
> 
> Add an over currerent "changed" event. The registered useres
> for this event can then check the over current flag to know
> the status of the over current condition.

Would a more general event for error conditions work as well?  Thinking
about this I'm unclear how interested consumers are going to be in the
specific error condition as opposed to the fact that the regulator ran
into trouble, and I can imagine that some regulators will report the
same root cause differently - another regulator might detect an
excessive current draw by seeing the output voltage collapse and the
regulator go out of regulation for example.

[toc] | [next] | [standalone]

#1511372

Hi Mark,

On Fri, Oct 28, 2016 at 8:22 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> Regulator consumers may be interested to know when the
>> over current condition is over.
>>
>> Add an over currerent "changed" event. The registered useres
>> for this event can then check the over current flag to know
>> the status of the over current condition.
>
> Would a more general event for error conditions work as well?  Thinking
> about this I'm unclear how interested consumers are going to be in the
> specific error condition as opposed to the fact that the regulator ran
> into trouble, and I can imagine that some regulators will report the
> same root cause differently - another regulator might detect an
> excessive current draw by seeing the output voltage collapse and the
> regulator go out of regulation for example.

Sorry if i misunderstood, but if we make the name generic,
i think it might change a bit the definition of the flags,
The flags will not represent events, but states.

i think today each time an event occurs a notification is sent with the
corresponding flag(s) set.

if we use a generic name, It means that each time the regulator driver
sends an event, it should check which "other" error conditons tied to the
generic flag are present and set the corresponding bits too.

illustrative example:
today over current and over temp are two different events
we send one notification for each with only the bits tied to the
event that is happening set.

if we add a generic error flag, it would mean that if over current happens
and we set the generic error flag, we would also have to check
if over temp is present to set or not that flag. similarly, when the over
temp event happens the regulator driver would have to check if over
current is present too.


Regards
Axel.

Regards
Axel.

[toc] | [prev] | [next] | [standalone]

#1511604

Hi Mark,

On Fri, Oct 28, 2016 at 9:41 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> Hi Mark,
>
> On Fri, Oct 28, 2016 at 8:22 PM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
>>> From: Axel Haslam <ahaslam@baylibre.com>
>>>
>>> Regulator consumers may be interested to know when the
>>> over current condition is over.
>>>
>>> Add an over currerent "changed" event. The registered useres
>>> for this event can then check the over current flag to know
>>> the status of the over current condition.
>>
>> Would a more general event for error conditions work as well?  Thinking
>> about this I'm unclear how interested consumers are going to be in the
>> specific error condition as opposed to the fact that the regulator ran
>> into trouble, and I can imagine that some regulators will report the
>> same root cause differently - another regulator might detect an
>> excessive current draw by seeing the output voltage collapse and the
>> regulator go out of regulation for example.
>

After some more thought,

I can change the logic a bit, and send an event named something like:

REGULATOR_EVENT_ERRORS_CLEARED

would that make more sense?

-Axel.


> Sorry if i misunderstood, but if we make the name generic,
> i think it might change a bit the definition of the flags,
> The flags will not represent events, but states.
>
> i think today each time an event occurs a notification is sent with the
> corresponding flag(s) set.
>
> if we use a generic name, It means that each time the regulator driver
> sends an event, it should check which "other" error conditons tied to the
> generic flag are present and set the corresponding bits too.
>
> illustrative example:
> today over current and over temp are two different events
> we send one notification for each with only the bits tied to the
> event that is happening set.
>
> if we add a generic error flag, it would mean that if over current happens
> and we set the generic error flag, we would also have to check
> if over temp is present to set or not that flag. similarly, when the over
> temp event happens the regulator driver would have to check if over
> current is present too.
>

[toc] | [prev] | [next] | [standalone]

#1511859

[Multipart message — attachments visible in raw view] — view raw

On Fri, Oct 28, 2016 at 09:41:44PM +0200, Axel Haslam wrote:

> i think today each time an event occurs a notification is sent with the
> corresponding flag(s) set.

Right, so I think the problem here is actually that you called this 
REGULATOR_EVENT_OVER_CURRENT_CHANGE with the _CHANGE on the end which
means it's just saying that the user has to go poll to see if the device
is or is not over current separately at which point you may as well pull
in all the other error things into what you're polling for.  If you'd
dropped the _CHANGE it'd be consistent with the other events we have for
errors and fine.

[toc] | [prev] | [next] | [standalone]

#1512053

Hi Mark,

On Sat, Oct 29, 2016 at 8:40 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Oct 28, 2016 at 09:41:44PM +0200, Axel Haslam wrote:
>
>> i think today each time an event occurs a notification is sent with the
>> corresponding flag(s) set.
>
> Right, so I think the problem here is actually that you called this
> REGULATOR_EVENT_OVER_CURRENT_CHANGE with the _CHANGE on the end which
> means it's just saying that the user has to go poll to see if the device
> is or is not over current separately at which point you may as well pull
> in all the other error things into what you're polling for.  If you'd
> dropped the _CHANGE it'd be consistent with the other events we have for
> errors and fine.

The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
what is missing and what i would need form the usb driver, is a way for
the consumer to know that the over current condition is over.
since i cannot do this with get mode, and get status is not exported,

We can do this adding a more generic event flag:
REGULATOR_EVENT_ERRORS_CLEARED

that would be sent by the supply when all errors are over, and the
regulator is back to  normal operation.

Regards
Axel.

[toc] | [prev] | [next] | [standalone]

#1512680

[Multipart message — attachments visible in raw view] — view raw

On Sun, Oct 30, 2016 at 01:02:21PM +0100, Axel Haslam wrote:

> The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
> what is missing and what i would need form the usb driver, is a way for
> the consumer to know that the over current condition is over.
> since i cannot do this with get mode, and get status is not exported,

> We can do this adding a more generic event flag:
> REGULATOR_EVENT_ERRORS_CLEARED

> that would be sent by the supply when all errors are over, and the
> regulator is back to  normal operation.

That's a different thing and definitely not what you were saying in the
changelog.  I don't think this is something that it makes sense to do
with events as it's not something that devices will tend to generate
interrupts for, anything that is going to rely on events for that is
going to be broken.  Hardware is mostly designed with the idea that
errors are catastrophic.

If you really care about things clearing then you need to add a sensible
interface for exposting all the possible error conditions that users can
poll.  The reason get_mode() got rejected was that error statuses and
modes are completely different things, get_status() is not going to work
for you since it is very common for multiple errors to happen at the
same time.

[toc] | [prev] | [next] | [standalone]

#1513319

Hi Mark,

On Mon, Oct 31, 2016 at 5:22 PM, Mark Brown <broonie@kernel.org> wrote:
> On Sun, Oct 30, 2016 at 01:02:21PM +0100, Axel Haslam wrote:
>
>> The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
>> what is missing and what i would need form the usb driver, is a way for
>> the consumer to know that the over current condition is over.
>> since i cannot do this with get mode, and get status is not exported,
>
>> We can do this adding a more generic event flag:
>> REGULATOR_EVENT_ERRORS_CLEARED
>
>> that would be sent by the supply when all errors are over, and the
>> regulator is back to  normal operation.
>
> That's a different thing and definitely not what you were saying in the
> changelog.  I don't think this is something that it makes sense to do
> with events as it's not something that devices will tend to generate
> interrupts for, anything that is going to rely on events for that is
> going to be broken.  Hardware is mostly designed with the idea that
> errors are catastrophic.
>
> If you really care about things clearing then you need to add a sensible
> interface for exposting all the possible error conditions that users can
> poll.  The reason get_mode() got rejected was that error statuses and
> modes are completely different things, get_status() is not going to work
> for you since it is very common for multiple errors to happen at the
> same time.

Ok, sorry if i was unclear in the change log.
ill add a new interface and lets see if it makes more sense in v2.

Regards
Axel.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web