Re: [PATCH 4/6] mfd: max77759: modify irq configs
From: Amit Sunil Dhamne
Date: Wed Nov 26 2025 - 23:15:41 EST
On 11/25/25 10:44 PM, André Draszik wrote:
Hi Amit,
On Tue, 2025-11-25 at 17:10 -0800, Amit Sunil Dhamne wrote:
Hi André,You should also add the remaining bits in each register here, so that the
On 11/23/25 10:21 PM, André Draszik wrote:
Hi Amit,Ack!
Thanks for your patches to enable the charger!
From: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
Define specific bit-level masks for charger's registers and modify the
irq mask for charger irq_chip. Also, configure the max77759 interrupt
lines as active low to all interrupt registrations to ensure the
interrupt controllers are configured with the correct trigger type.
Signed-off-by: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
---
drivers/mfd/max77759.c | 24 +++++++++++++++++-------
include/linux/mfd/max77759.h | 9 +++++++++
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
index 6cf6306c4a3b..5fe22884f362 100644
--- a/drivers/mfd/max77759.c
+++ b/drivers/mfd/max77759.c
@@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
};
static const struct regmap_irq max77759_chgr_irqs[] = {
- REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
- REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
+ REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
+ MAX77759_CHGR_REG_CHG_INT_AICL |
+ MAX77759_CHGR_REG_CHG_INT_CHGIN |
+ MAX77759_CHGR_REG_CHG_INT_CHG |
+ MAX77759_CHGR_REG_CHG_INT_INLIM),
+ REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
+ MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
+ MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
+ MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
+ MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
+ MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
};
regulator-irq can mask them when no user exists. It will only touch the
bits it knows about, so the state of the mask register is non-
deterministic with this change as-is (depends on how the bootloader
configured it).
[...]
I see what you're saying. I will remove this and keep it the way it was before.
It makes sense to add them now, as per above.I would prefer to only define the macros I am currently using to keep
diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.hEven if wireless out of scope, it'd still be nice to add macros for
index c6face34e385..0ef29a48deec 100644
--- a/include/linux/mfd/max77759.h
+++ b/include/linux/mfd/max77759.h
@@ -62,7 +62,16 @@
#define MAX77759_CHGR_REG_CHG_INT 0xb0
#define MAX77759_CHGR_REG_CHG_INT2 0xb1
#define MAX77759_CHGR_REG_CHG_INT_MASK 0xb2
+#define MAX77759_CHGR_REG_CHG_INT_AICL BIT(7)
+#define MAX77759_CHGR_REG_CHG_INT_CHGIN BIT(6)
+#define MAX77759_CHGR_REG_CHG_INT_CHG BIT(4)
+#define MAX77759_CHGR_REG_CHG_INT_INLIM BIT(2)
#define MAX77759_CHGR_REG_CHG_INT2_MASK 0xb3
+#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO BIT(4)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC BIT(3)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV BIT(2)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO BIT(1)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
the remaining bits to make this complete and avoid having to update
these again in case wireless support gets added in the future.
the review focused, unless you consider this a strict requirement.
Okay. I will add them.
Yes, would be nice to have them all in one place (in this file). That said,Also, would be nice to keep existing style and indent the bits fromI will fix the indentation and ordering in the next revision.
the registers (see existing bit definitions in this file a few lines
further up).
Finally, can you add the bits below the respective register (0xb0 / 0xb1)
please, to keep suffix meaningful, and to follow existing style for cases
like this (see MAX77759_MAXQ_REG_UIC_INT1).
Regarding bit definitions: In [PATCH 5/6], the max77759_charger.c driver
defines bits for the register addresses defined in this file. Currently,
those macros are only used locally by the max77759 charger driver. Would
you prefer I move those definitions to this header file as well?
CHG_INT, CHG_INT_MASK, and CHG_INT_OK all have the same layout and share
the same bits, so I personally would probably reuse the ones you added for
INT for all three of them - MASK (as you did already), and also for the OK
register. But up to you.
Sounds good.
Thanks,
Amit
Cheers,
Andre'