linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: AngeloGioacchino Del Regno  <angelogioacchino.delregno@somainline.org>
To: matti.vaittinen@fi.rohmeurope.com, Mark Brown <broonie@kernel.org>
Cc: "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: short-circuit and over-current IRQs
Date: Wed, 27 Jan 2021 15:34:46 +0100	[thread overview]
Message-ID: <c10cf8d6-f36a-60f4-93cc-807e11a7cec9@somainline.org> (raw)
In-Reply-To: <6d60af3516161bd04332cd60b50aa4becf92e17a.camel@fi.rohmeurope.com>

Il 27/01/21 13:56, Matti Vaittinen ha scritto:
> Hello Mark,
> 

Hey Matti, hey Mark!

> Nice to hear from you. :)
> 
> On Wed, 2021-01-27 at 12:27 +0000, Mark Brown wrote:
>> On Wed, Jan 27, 2021 at 12:01:55PM +0000, Vaittinen, Matti wrote:
>>
>>> Anyways - I was wondering if this is common thing amongst many
>>> PMICs?
>>> If yes - then, perhaps some generally useful regulator helper could
>>> be
>>> added to help implementing the IRQ disabling + scheduling worker to
>>> check status and re-enable IRQs? I think it *might* save some time
>>> in
>>> the future - and help making same mistakes many times :]
>>

It's probably worth it if more drivers need that: sometimes there is
HW supporting this feature and it doesn't get done because of the usual
lack of time.

Providing a helper would probably help.

>> If we've got two that's enough for a helper.  TBH I'm a bit surprised
>> that people are implementing hardware that leaves the outputs enabled
>> when it detects this sort of error, it's something that's usually an
>> emergency that needs shutting off quickly to prevent hardware damage.
> 
> I can only speak for BD9576MUF - which has two limits for monitored
> entity (temperature/voltage). One limit being 'warning' limit (or
> 'detection' as data-sheet says), the other being 'protection' limit.
> 
> I don't know guys who designed HW - I am located to a remote spot on
> the other side of the world and on top of that I am the odd "SW guy" so
> it's better to keep me out of the HW R&D decisions and especially the
> customers ;) - but I *guess* the idea has been that consumer driver(s)
> could do some 'recovery action' at 'warning' limit (which might make
> sense for example when temperature is increased to 'high' but not yet
> 'damaging' - I guess there is something that can be done with
> over/under voltages too...) and only kill the power if that doesn't
> help and situation (with temperature/voltage) gets worse.

I would tend to agree with you here, Matti. Also from what I understand,
the wanted outcome is software handling a possibly temporary issue with
you charging caps, external IC initialization using (expectedly) much
more power than needed before stabilizing, and eventually handling other
"real" issues for which there is a solution that may not even include
disabling the regulator itself, but some other handling on the connected
device driver.

Though, Mark: for example, on qcom labibb, there's "PBS" that is killing
the regulators on short-circuit condition and as you see, handling that
is a little trickier compared to the over-current one, where there is no
such auto-magic-thing...
.... I wouldn't know if it'd be a good idea to have a system like qcom's
PBS everywhere.
For the sake of protecting HW "paranoidly" though.. maybe :))

> 
> What I don't like is the fact that HW keeps IRQ asserted instead of
> having a state machine which would only generate IRQ when condition
> changes + status register to read current condition.
> 

Unless I've misunderstood this, you're describing a *very* common
behavior across regulators and other kinds of devices, but that's
not a problem. IMO, it's a solution (to quirky MCUs/SoCs/CPUs/blah).

Of course reading a register means that you waste more time before
deciding to "press the red button", but even on a slow bus like I2C,
it's anyway not reaching the point where that wasted time is relevant.
At least, in many cases.

> I will see if I can cook-up something decent - but as I said, I would
> appreciate any/all testing if I get patch crafted :)

I develop this stuff in my spare time: I can't make big promises, but
I can tell you that I will try to test your proposal on qcom-labibb as
soon as I will be able to.

Yours,
--Angelo

> 
> Best Regards
> 	Matti
> 


  reply	other threads:[~2021-01-27 14:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27 12:01 short-circuit and over-current IRQs Vaittinen, Matti
2021-01-27 12:27 ` Mark Brown
2021-01-27 12:56   ` Matti Vaittinen
2021-01-27 14:34     ` AngeloGioacchino Del Regno [this message]
2021-01-27 14:42       ` Vaittinen, Matti
2021-01-27 16:32       ` Mark Brown
2021-01-28  9:23         ` Vaittinen, Matti
2021-01-28 12:10           ` Mark Brown
2021-01-28 12:49             ` Vaittinen, Matti
     [not found]             ` <a89bf6f0e6c1e4b9afe980908b7e36b70b304a96.camel@fi.rohmeurope.com>
2021-01-30 15:43               ` AngeloGioacchino Del Regno
2021-02-01  7:14                 ` Matti Vaittinen
2021-02-01 13:17                 ` Mark Brown

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=c10cf8d6-f36a-60f4-93cc-807e11a7cec9@somainline.org \
    --to=angelogioacchino.delregno@somainline.org \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matti.vaittinen@fi.rohmeurope.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).