Re: [PATCH v3 09/10] mfd: bd96801: Add ERRB IRQ

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/14/24 10:50, Lee Jones wrote:
On Fri, 14 Jun 2024, Matti Vaittinen wrote:

On 6/13/24 19:32, Lee Jones wrote:
On Tue, 04 Jun 2024, Matti Vaittinen wrote:

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB
handling can in many cases be omitted because it is used to inform fatal
IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency
power source which allows some very short time of operation to try to
gracefully shut down sensitive hardware. Furthermore, it is possible the
processor controlling the PMIC is not powered by the PMIC. In such cases
handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
---
Revision history:
v2 =>:
	- No changes
v1 => v2:
	- New patch
---
   drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
   1 file changed, 253 insertions(+), 38 deletions(-)

diff --git a/drivers/mfd/rohm-bd96801.c b/drivers/mfd/rohm-bd96801.c
index 1c2a9591be7b..b7f073318873 100644
--- a/drivers/mfd/rohm-bd96801.c
+++ b/drivers/mfd/rohm-bd96801.c
@@ -5,13 +5,9 @@
    * ROHM BD96801 PMIC driver
    *
    * This version of the "BD86801 scalable PMIC"'s driver supports only very
- * basic set of the PMIC features. Most notably, there is no support for
- * the ERRB interrupt and the configurations which should be done when the
- * PMIC is in STBY mode.
- *
- * Supporting the ERRB interrupt would require dropping the regmap-IRQ
- * usage or working around (or accepting a presense of) a naming conflict
- * in debugFS IRQs.

Why bother adding all that blurb in the first place?

Because, I assume there are users who would like to have the ERRB in use.
The main purpose of this comment is that any such users could
	a) see this version does not support ERRB.
	b) can find the original RFC with ERRB supportn and a workaround.
	c) know why this version does not work with ERRB and thus fix this

It seems this ERRB support may be missing from upstream for a while, hence I
think having this note is worthy until (if) this ERRB patch lands in
upstream.

What I mean is - you're adding all of these extra lines in patch 3 and
removing them in patch 9.


True. This is because I had a feeling the irqdomain changes might not get merged that fast as it seemed like something that is not completely trivial. This comment is useful if patches 7-10 aren't merged together with 1-6 - which I now also hope is the case XD

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
    *
    * Being able to reliably do the configurations like changing the
    * regulator safety limits (like limits for the over/under -voltages, over
@@ -23,16 +19,14 @@
    * be the need to configure these safety limits. Hence it's not simple to
    * come up with a generic solution.
    *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
    * https://lore.kernel.org/all/cover.1712920132.git.mazziesaccount@xxxxxxxxx/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.
+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
    *
    * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
    */

...

Thanks for comments Lee. Reworking this will have to wait for the irqdomain
name suffix, which I will continue after Hervé has done his part of the
irqdomain changes. I will omit this patch from the next re-spin of the
series.

I'm in no rush. :)

Well, glad to hear ;) I still usually try to avoid delaying sending the follow-up patches. I am under impression it is easier to review the new revision if the previous revision was not reviewed too long ago... ;)

I feel it is polite to tell the reviewers there will be some delay when I know it.

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux