Hi, On 1/15/22 01:05, Mark Gross wrote: > On Wed, Jan 12, 2022 at 12:23:09AM +0100, Hans de Goede wrote: >> The driver as originally submitted accidentally relied on Android having >> run before and Android having unmasked the 2nd level IRQ-mask for the >> charger IRQ. This worked since these are PMIC registers which are only >> reset when the battery is fully drained or disconnected. > Correct me if I'm wrong but Android is a usermode stack. It cannot unmask > IRQ's form user mode. Android is an OS, consisting of what in many cases amounts to a fork of the Linux kernel + a usermode stack. With "Android" in the commit msg before I was referring to the entire OS, including the custom kernel shipped with the OS. > I'm guessing the persistant PMIC registeres where set when running a vendor > kernel that was in use with Anroid prior to installing one using this change > and a modern kernel, and after the batteries ran out or where removed the > charger didn't work. Right. Regards, Hans >> Fix the charger IRQ no longer working after loss of battery power by >> properly setting the 2nd level IRQ-mask for the charger IRQ. >> >> Note this removes the need to enable/disable our parent IRQ which just >> sets the mask bit in the 1st level IRQ-mask register, setting one of >> the 2 level masks is enough to stop the IRQ from getting reported. >> >> Fixes: 761db353d9e2 ("platform/x86: Add intel_crystal_cove_charger driver") >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> .../platform/x86/intel/crystal_cove_charger.c | 26 +++++++++---------- >> 1 file changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/platform/x86/intel/crystal_cove_charger.c b/drivers/platform/x86/intel/crystal_cove_charger.c >> index 0374bc742513..eeaa926d2058 100644 >> --- a/drivers/platform/x86/intel/crystal_cove_charger.c >> +++ b/drivers/platform/x86/intel/crystal_cove_charger.c >> @@ -17,6 +17,7 @@ >> #include <linux/regmap.h> >> >> #define CHGRIRQ_REG 0x0a >> +#define MCHGRIRQ_REG 0x17 >> >> struct crystal_cove_charger_data { >> struct mutex buslock; /* irq_bus_lock */ >> @@ -25,8 +26,8 @@ struct crystal_cove_charger_data { >> struct irq_domain *irq_domain; >> int irq; >> int charger_irq; >> - bool irq_enabled; >> - bool irq_is_enabled; >> + u8 mask; >> + u8 new_mask; >> }; >> >> static irqreturn_t crystal_cove_charger_irq(int irq, void *data) >> @@ -53,13 +54,9 @@ static void crystal_cove_charger_irq_bus_sync_unlock(struct irq_data *data) >> { >> struct crystal_cove_charger_data *charger = irq_data_get_irq_chip_data(data); >> >> - if (charger->irq_is_enabled != charger->irq_enabled) { >> - if (charger->irq_enabled) >> - enable_irq(charger->irq); >> - else >> - disable_irq(charger->irq); >> - >> - charger->irq_is_enabled = charger->irq_enabled; >> + if (charger->mask != charger->new_mask) { >> + regmap_write(charger->regmap, MCHGRIRQ_REG, charger->new_mask); >> + charger->mask = charger->new_mask; >> } >> >> mutex_unlock(&charger->buslock); >> @@ -69,14 +66,14 @@ static void crystal_cove_charger_irq_unmask(struct irq_data *data) >> { >> struct crystal_cove_charger_data *charger = irq_data_get_irq_chip_data(data); >> >> - charger->irq_enabled = true; >> + charger->new_mask &= ~BIT(data->hwirq); >> } >> >> static void crystal_cove_charger_irq_mask(struct irq_data *data) >> { >> struct crystal_cove_charger_data *charger = irq_data_get_irq_chip_data(data); >> >> - charger->irq_enabled = false; >> + charger->new_mask |= BIT(data->hwirq); >> } >> >> static void crystal_cove_charger_rm_irq_domain(void *data) >> @@ -130,10 +127,13 @@ static int crystal_cove_charger_probe(struct platform_device *pdev) >> irq_set_nested_thread(charger->charger_irq, true); >> irq_set_noprobe(charger->charger_irq); >> >> + /* Mask the single 2nd level IRQ before enabling the 1st level IRQ */ >> + charger->mask = BIT(0); >> + regmap_write(charger->regmap, MCHGRIRQ_REG, charger->mask); >> + >> ret = devm_request_threaded_irq(&pdev->dev, charger->irq, NULL, >> crystal_cove_charger_irq, >> - IRQF_ONESHOT | IRQF_NO_AUTOEN, >> - KBUILD_MODNAME, charger); >> + IRQF_ONESHOT, KBUILD_MODNAME, charger); >> if (ret) >> return dev_err_probe(&pdev->dev, ret, "requesting irq\n"); >> >> -- >> 2.33.1 >> >