* [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use @ 2010-04-05 15:14 Mark Brown 2010-04-05 15:14 ` [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending Mark Brown 2010-04-07 8:50 ` [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Samuel Ortiz 0 siblings, 2 replies; 4+ messages in thread From: Mark Brown @ 2010-04-05 15:14 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel, Mark Brown Ensure that the hardware has interrupts masked if we are not using the interrupt controller on the WM831x by initialising the masks before we check for the setup data required for the IRQ line. This avoids signalling an unused IRQ line and improves the robustness of checks that the IRQ is in use. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/mfd/wm831x-irq.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/mfd/wm831x-irq.c b/drivers/mfd/wm831x-irq.c index b1d66b8..f8b51be 100644 --- a/drivers/mfd/wm831x-irq.c +++ b/drivers/mfd/wm831x-irq.c @@ -460,6 +460,14 @@ int wm831x_irq_init(struct wm831x *wm831x, int irq) mutex_init(&wm831x->irq_lock); + /* Mask the individual interrupt sources */ + for (i = 0; i < ARRAY_SIZE(wm831x->irq_masks_cur); i++) { + wm831x->irq_masks_cur[i] = 0xffff; + wm831x->irq_masks_cache[i] = 0xffff; + wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_1_MASK + i, + 0xffff); + } + if (!irq) { dev_warn(wm831x->dev, "No interrupt specified - functionality limited\n"); @@ -475,14 +483,6 @@ int wm831x_irq_init(struct wm831x *wm831x, int irq) wm831x->irq = irq; wm831x->irq_base = pdata->irq_base; - /* Mask the individual interrupt sources */ - for (i = 0; i < ARRAY_SIZE(wm831x->irq_masks_cur); i++) { - wm831x->irq_masks_cur[i] = 0xffff; - wm831x->irq_masks_cache[i] = 0xffff; - wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_1_MASK + i, - 0xffff); - } - /* Register them with genirq */ for (cur_irq = wm831x->irq_base; cur_irq < ARRAY_SIZE(wm831x_irqs) + wm831x->irq_base; -- 1.7.0.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending 2010-04-05 15:14 [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Mark Brown @ 2010-04-05 15:14 ` Mark Brown 2010-04-07 8:51 ` Samuel Ortiz 2010-04-07 8:50 ` [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Samuel Ortiz 1 sibling, 1 reply; 4+ messages in thread From: Mark Brown @ 2010-04-05 15:14 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel, Mark Brown The charger interrupts on the WM831x are unconditionally a wake source for the system. If the power driver is not able to monitor them (for example, due to the IRQ line not having been wired up on the system) then any charger interrupt will prevent the system suspending for any meaningful amount of time since nothing will ack them. Avoid this issue by manually acknowledging these interrupts when we suspend the WM831x core device if they are masked. If software is actually using the interrupts then they will be unmasked and this change will have no effect. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/mfd/wm831x-core.c | 47 +++++++++++++++++++++++++++++++++++++++ include/linux/mfd/wm831x/core.h | 5 ++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c index f8a8cdf..8152529 100644 --- a/drivers/mfd/wm831x-core.c +++ b/drivers/mfd/wm831x-core.c @@ -1493,6 +1493,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) case WM8310: parent = WM8310; wm831x->num_gpio = 16; + wm831x->charger_irq_wake = 1; if (rev > 0) { wm831x->has_gpio_ena = 1; wm831x->has_cs_sts = 1; @@ -1504,6 +1505,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) case WM8311: parent = WM8311; wm831x->num_gpio = 16; + wm831x->charger_irq_wake = 1; if (rev > 0) { wm831x->has_gpio_ena = 1; wm831x->has_cs_sts = 1; @@ -1515,6 +1517,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) case WM8312: parent = WM8312; wm831x->num_gpio = 16; + wm831x->charger_irq_wake = 1; if (rev > 0) { wm831x->has_gpio_ena = 1; wm831x->has_cs_sts = 1; @@ -1653,6 +1656,42 @@ static void wm831x_device_exit(struct wm831x *wm831x) kfree(wm831x); } +static int wm831x_device_suspend(struct wm831x *wm831x) +{ + int reg, mask; + + /* If the charger IRQs are a wake source then make sure we ack + * them even if they're not actively being used (eg, no power + * driver or no IRQ line wired up) then acknowledge the + * interrupts otherwise suspend won't last very long. + */ + if (wm831x->charger_irq_wake) { + reg = wm831x_reg_read(wm831x, WM831X_INTERRUPT_STATUS_2_MASK); + + mask = WM831X_CHG_BATT_HOT_EINT | + WM831X_CHG_BATT_COLD_EINT | + WM831X_CHG_BATT_FAIL_EINT | + WM831X_CHG_OV_EINT | WM831X_CHG_END_EINT | + WM831X_CHG_TO_EINT | WM831X_CHG_MODE_EINT | + WM831X_CHG_START_EINT; + + /* If any of the interrupts are masked read the statuses */ + if (reg & mask) + reg = wm831x_reg_read(wm831x, + WM831X_INTERRUPT_STATUS_2); + + if (reg & mask) { + dev_info(wm831x->dev, + "Acknowledging masked charger IRQs: %x\n", + reg & mask); + wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_2, + reg & mask); + } + } + + return 0; +} + static int wm831x_i2c_read_device(struct wm831x *wm831x, unsigned short reg, int bytes, void *dest) { @@ -1727,6 +1766,13 @@ static int wm831x_i2c_remove(struct i2c_client *i2c) return 0; } +static int wm831x_i2c_suspend(struct i2c_client *i2c) +{ + struct wm831x *wm831x = i2c_get_clientdata(i2c); + + return wm831x_device_suspend(wm831x); +} + static const struct i2c_device_id wm831x_i2c_id[] = { { "wm8310", WM8310 }, { "wm8311", WM8311 }, @@ -1744,6 +1790,7 @@ static struct i2c_driver wm831x_i2c_driver = { }, .probe = wm831x_i2c_probe, .remove = wm831x_i2c_remove, + .suspend = wm831x_i2c_suspend, .id_table = wm831x_i2c_id, }; diff --git a/include/linux/mfd/wm831x/core.h b/include/linux/mfd/wm831x/core.h index 5915f6e..eb5bd4e 100644 --- a/include/linux/mfd/wm831x/core.h +++ b/include/linux/mfd/wm831x/core.h @@ -256,8 +256,9 @@ struct wm831x { int irq_masks_cache[WM831X_NUM_IRQ_REGS]; /* Cached hardware value */ /* Chip revision based flags */ - unsigned has_gpio_ena:1; /* Has GPIO enable bit */ - unsigned has_cs_sts:1; /* Has current sink status bit */ + unsigned has_gpio_ena:1; /* Has GPIO enable bit */ + unsigned has_cs_sts:1; /* Has current sink status bit */ + unsigned charger_irq_wake:1; /* Are charger IRQs a wake source? */ int num_gpio; -- 1.7.0.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending 2010-04-05 15:14 ` [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending Mark Brown @ 2010-04-07 8:51 ` Samuel Ortiz 0 siblings, 0 replies; 4+ messages in thread From: Samuel Ortiz @ 2010-04-07 8:51 UTC (permalink / raw) To: Mark Brown; +Cc: linux-kernel Hi Mark, On Mon, Apr 05, 2010 at 04:14:18PM +0100, Mark Brown wrote: > The charger interrupts on the WM831x are unconditionally a wake source > for the system. If the power driver is not able to monitor them (for > example, due to the IRQ line not having been wired up on the system) > then any charger interrupt will prevent the system suspending for any > meaningful amount of time since nothing will ack them. > > Avoid this issue by manually acknowledging these interrupts when we > suspend the WM831x core device if they are masked. If software is > actually using the interrupts then they will be unmasked and this > change will have no effect. Patch applied, many thanks. Cheers, Samuel. > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/mfd/wm831x-core.c | 47 +++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/wm831x/core.h | 5 ++- > 2 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c > index f8a8cdf..8152529 100644 > --- a/drivers/mfd/wm831x-core.c > +++ b/drivers/mfd/wm831x-core.c > @@ -1493,6 +1493,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) > case WM8310: > parent = WM8310; > wm831x->num_gpio = 16; > + wm831x->charger_irq_wake = 1; > if (rev > 0) { > wm831x->has_gpio_ena = 1; > wm831x->has_cs_sts = 1; > @@ -1504,6 +1505,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) > case WM8311: > parent = WM8311; > wm831x->num_gpio = 16; > + wm831x->charger_irq_wake = 1; > if (rev > 0) { > wm831x->has_gpio_ena = 1; > wm831x->has_cs_sts = 1; > @@ -1515,6 +1517,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) > case WM8312: > parent = WM8312; > wm831x->num_gpio = 16; > + wm831x->charger_irq_wake = 1; > if (rev > 0) { > wm831x->has_gpio_ena = 1; > wm831x->has_cs_sts = 1; > @@ -1653,6 +1656,42 @@ static void wm831x_device_exit(struct wm831x *wm831x) > kfree(wm831x); > } > > +static int wm831x_device_suspend(struct wm831x *wm831x) > +{ > + int reg, mask; > + > + /* If the charger IRQs are a wake source then make sure we ack > + * them even if they're not actively being used (eg, no power > + * driver or no IRQ line wired up) then acknowledge the > + * interrupts otherwise suspend won't last very long. > + */ > + if (wm831x->charger_irq_wake) { > + reg = wm831x_reg_read(wm831x, WM831X_INTERRUPT_STATUS_2_MASK); > + > + mask = WM831X_CHG_BATT_HOT_EINT | > + WM831X_CHG_BATT_COLD_EINT | > + WM831X_CHG_BATT_FAIL_EINT | > + WM831X_CHG_OV_EINT | WM831X_CHG_END_EINT | > + WM831X_CHG_TO_EINT | WM831X_CHG_MODE_EINT | > + WM831X_CHG_START_EINT; > + > + /* If any of the interrupts are masked read the statuses */ > + if (reg & mask) > + reg = wm831x_reg_read(wm831x, > + WM831X_INTERRUPT_STATUS_2); > + > + if (reg & mask) { > + dev_info(wm831x->dev, > + "Acknowledging masked charger IRQs: %x\n", > + reg & mask); > + wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_2, > + reg & mask); > + } > + } > + > + return 0; > +} > + > static int wm831x_i2c_read_device(struct wm831x *wm831x, unsigned short reg, > int bytes, void *dest) > { > @@ -1727,6 +1766,13 @@ static int wm831x_i2c_remove(struct i2c_client *i2c) > return 0; > } > > +static int wm831x_i2c_suspend(struct i2c_client *i2c) > +{ > + struct wm831x *wm831x = i2c_get_clientdata(i2c); > + > + return wm831x_device_suspend(wm831x); > +} > + > static const struct i2c_device_id wm831x_i2c_id[] = { > { "wm8310", WM8310 }, > { "wm8311", WM8311 }, > @@ -1744,6 +1790,7 @@ static struct i2c_driver wm831x_i2c_driver = { > }, > .probe = wm831x_i2c_probe, > .remove = wm831x_i2c_remove, > + .suspend = wm831x_i2c_suspend, > .id_table = wm831x_i2c_id, > }; > > diff --git a/include/linux/mfd/wm831x/core.h b/include/linux/mfd/wm831x/core.h > index 5915f6e..eb5bd4e 100644 > --- a/include/linux/mfd/wm831x/core.h > +++ b/include/linux/mfd/wm831x/core.h > @@ -256,8 +256,9 @@ struct wm831x { > int irq_masks_cache[WM831X_NUM_IRQ_REGS]; /* Cached hardware value */ > > /* Chip revision based flags */ > - unsigned has_gpio_ena:1; /* Has GPIO enable bit */ > - unsigned has_cs_sts:1; /* Has current sink status bit */ > + unsigned has_gpio_ena:1; /* Has GPIO enable bit */ > + unsigned has_cs_sts:1; /* Has current sink status bit */ > + unsigned charger_irq_wake:1; /* Are charger IRQs a wake source? */ > > int num_gpio; > > -- > 1.7.0.3 > -- Intel Open Source Technology Centre http://oss.intel.com/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use 2010-04-05 15:14 [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Mark Brown 2010-04-05 15:14 ` [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending Mark Brown @ 2010-04-07 8:50 ` Samuel Ortiz 1 sibling, 0 replies; 4+ messages in thread From: Samuel Ortiz @ 2010-04-07 8:50 UTC (permalink / raw) To: Mark Brown; +Cc: linux-kernel Hi Mark, On Mon, Apr 05, 2010 at 04:14:17PM +0100, Mark Brown wrote: > Ensure that the hardware has interrupts masked if we are not using > the interrupt controller on the WM831x by initialising the masks > before we check for the setup data required for the IRQ line. This > avoids signalling an unused IRQ line and improves the robustness > of checks that the IRQ is in use. Patch applied, thanks. Cheers, Samuel. > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/mfd/wm831x-irq.c | 16 ++++++++-------- > 1 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/mfd/wm831x-irq.c b/drivers/mfd/wm831x-irq.c > index b1d66b8..f8b51be 100644 > --- a/drivers/mfd/wm831x-irq.c > +++ b/drivers/mfd/wm831x-irq.c > @@ -460,6 +460,14 @@ int wm831x_irq_init(struct wm831x *wm831x, int irq) > > mutex_init(&wm831x->irq_lock); > > + /* Mask the individual interrupt sources */ > + for (i = 0; i < ARRAY_SIZE(wm831x->irq_masks_cur); i++) { > + wm831x->irq_masks_cur[i] = 0xffff; > + wm831x->irq_masks_cache[i] = 0xffff; > + wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_1_MASK + i, > + 0xffff); > + } > + > if (!irq) { > dev_warn(wm831x->dev, > "No interrupt specified - functionality limited\n"); > @@ -475,14 +483,6 @@ int wm831x_irq_init(struct wm831x *wm831x, int irq) > wm831x->irq = irq; > wm831x->irq_base = pdata->irq_base; > > - /* Mask the individual interrupt sources */ > - for (i = 0; i < ARRAY_SIZE(wm831x->irq_masks_cur); i++) { > - wm831x->irq_masks_cur[i] = 0xffff; > - wm831x->irq_masks_cache[i] = 0xffff; > - wm831x_reg_write(wm831x, WM831X_INTERRUPT_STATUS_1_MASK + i, > - 0xffff); > - } > - > /* Register them with genirq */ > for (cur_irq = wm831x->irq_base; > cur_irq < ARRAY_SIZE(wm831x_irqs) + wm831x->irq_base; > -- > 1.7.0.3 > -- Intel Open Source Technology Centre http://oss.intel.com/ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-04-07 8:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-05 15:14 [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Mark Brown 2010-04-05 15:14 ` [PATCH 2/2] mfd: Ensure WM831x charger interrupts are acknowledged when suspending Mark Brown 2010-04-07 8:51 ` Samuel Ortiz 2010-04-07 8:50 ` [PATCH 1/2] mfd: Initialise WM831x IRQ masks on chip even if interrupts not in use Samuel Ortiz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox