From 286581f7a1cd2536b70cc0cfbc742ee45260252e Mon Sep 17 00:00:00 2001 From: Andy Green Date: Fri, 25 Jul 2008 23:06:12 +0100 Subject: [PATCH] fix-pcf50633-suspend-resume-onehit-i2c-other-meddling.patch - speed up suspend and resume by using one hit i2c bulk transactions - don't bother storing int mask set on suspend, the default one is what we use anyway - put stack_trace() on pcf50633 low level access that fire if we try to touch them before we resumed - cosmetic source cleanup - reduces resume time for pcf50633 from 450ms to 255ms Signed-off-by: Andy Green --- arch/arm/mach-s3c2440/mach-gta02.c | 21 ++------- drivers/i2c/chips/pcf50633.c | 83 +++++++++++++++++++---------------- 2 files changed, 49 insertions(+), 55 deletions(-) diff --git a/arch/arm/mach-s3c2440/mach-gta02.c b/arch/arm/mach-s3c2440/mach-gta02.c index 9ba1036..22de181 100644 --- a/arch/arm/mach-s3c2440/mach-gta02.c +++ b/arch/arm/mach-s3c2440/mach-gta02.c @@ -477,24 +477,11 @@ static struct pcf50633_platform_data gta02_pcf_pdata = { .r_fix_batt_par = 10000, .r_sense_milli = 220, .resumers = { - [0] = /* PCF50633_INT1_ADPINS | */ - /* PCF50633_INT1_ADPREM | */ - PCF50633_INT1_USBINS | - PCF50633_INT1_USBREM | - PCF50633_INT1_ALARM, + [0] = PCF50633_INT1_USBINS | + PCF50633_INT1_USBREM | + PCF50633_INT1_ALARM, [1] = PCF50633_INT2_ONKEYF, - [2] = /* PCF50633_INT3_BATFULL | */ - /* PCF50633_INT3_CHGHALT | */ - /* PCF50633_INT3_THLIMON | */ - /* PCF50633_INT3_THLIMOFF | */ - /* PCF50633_INT3_USBLIMON | */ - /* PCF50633_INT3_USBLIMOFF | */ - PCF50633_INT3_ONKEY1S , - [3] = 0 /* | - PCF50633_INT4_LOWSYS | */ - /* PCF50633_INT4_LOWBAT | */ - /* PCF50633_INT4_HIGHTMP */, - [4] = 0 + [2] = PCF50633_INT3_ONKEY1S }, .rails = { [PCF50633_REGULATOR_AUTO] = { diff --git a/drivers/i2c/chips/pcf50633.c b/drivers/i2c/chips/pcf50633.c index f6886d7..c148ea7 100644 --- a/drivers/i2c/chips/pcf50633.c +++ b/drivers/i2c/chips/pcf50633.c @@ -163,10 +163,7 @@ struct pcf50633_data { u_int8_t down2out, down2ena; u_int8_t memldoout, memldoena; u_int8_t ledout, ledena, leddim; - struct { - u_int8_t out; - u_int8_t ena; - } ldo[__NUM_PCF50633_REGS]; + u_int8_t ldo[__NUM_PCF50633_REGS][2]; } standby_regs; struct resume_dependency resume_dependency; @@ -187,6 +184,10 @@ static struct platform_device *pcf50633_pdev; static int __reg_write(struct pcf50633_data *pcf, u_int8_t reg, u_int8_t val) { + if (pcf->have_been_suspended == 1) { + dev_err(&pcf->client.dev, "__reg_write while suspended\n"); + dump_stack(); + } return i2c_smbus_write_byte_data(&pcf->client, reg, val); } @@ -205,6 +206,10 @@ static int32_t __reg_read(struct pcf50633_data *pcf, u_int8_t reg) { int32_t ret; + if (pcf->have_been_suspended == 1) { + dev_err(&pcf->client.dev, "__reg_read while suspended\n"); + dump_stack(); + } ret = i2c_smbus_read_byte_data(&pcf->client, reg); return ret; @@ -2155,6 +2160,13 @@ static int pcf50633_suspend(struct device *dev, pm_message_t state) struct i2c_client *client = to_i2c_client(dev); struct pcf50633_data *pcf = i2c_get_clientdata(client); int i; + int ret; + u_int8_t tmp; + + /* we suspend once (!) as late as possible in the suspend sequencing */ + + if ((state.event != PM_EVENT_SUSPEND) || (pcf->have_been_suspended)) + return 0; /* The general idea is to power down all unused power supplies, * and then mask all PCF50606 interrup sources but EXTONR, ONKEYF @@ -2176,25 +2188,25 @@ static int pcf50633_suspend(struct device *dev, pm_message_t state) pcf->standby_regs.ledout = __reg_read(pcf, PCF50633_REG_LEDOUT); pcf->standby_regs.ledena = __reg_read(pcf, PCF50633_REG_LEDENA); pcf->standby_regs.leddim = __reg_read(pcf, PCF50633_REG_LEDDIM); - /* FIXME: one big read? */ - for (i = 0; i < 7; i++) { - u_int8_t reg_out = PCF50633_REG_LDO1OUT + 2*i; - pcf->standby_regs.ldo[i].out = __reg_read(pcf, reg_out); - pcf->standby_regs.ldo[i].ena = __reg_read(pcf, reg_out+1); - } + + /* regulator voltages and enable states */ + ret = i2c_smbus_read_i2c_block_data(&pcf->client, + PCF50633_REG_LDO1OUT, 14, + &pcf->standby_regs.ldo[0][0]); + if (ret != 14) + dev_err(dev, "Failed to save LDO levels and enables :-(\n"); /* switch off power supplies that are not needed during suspend */ for (i = 0; i < __NUM_PCF50633_REGULATORS; i++) { - if (!(pcf->pdata->rails[i].flags & PMU_VRAIL_F_SUSPEND_ON)) { - u_int8_t tmp; - - DEBUGP("disabling pcf50633 regulator %u\n", i); - /* we cannot use pcf50633_onoff_set() because we're - * already under the mutex */ - tmp = __reg_read(pcf, regulator_registers[i]+1); - tmp &= 0xfe; - __reg_write(pcf, regulator_registers[i]+1, tmp); - } + if ((pcf->pdata->rails[i].flags & PMU_VRAIL_F_SUSPEND_ON)) + continue; + + dev_dbg(dev, "disabling regulator %u\n", i); + /* we cannot use pcf50633_onoff_set() because we're + * already under the mutex */ + tmp = __reg_read(pcf, regulator_registers[i]+1); + tmp &= 0xfe; + __reg_write(pcf, regulator_registers[i]+1, tmp); } /* turn off the backlight */ @@ -2202,11 +2214,9 @@ static int pcf50633_suspend(struct device *dev, pm_message_t state) __reg_write(pcf, PCF50633_REG_LEDOUT, 2); __reg_write(pcf, PCF50633_REG_LEDENA, 0x00); - pcf->standby_regs.int1m = __reg_read(pcf, PCF50633_REG_INT1M); - pcf->standby_regs.int2m = __reg_read(pcf, PCF50633_REG_INT2M); - pcf->standby_regs.int3m = __reg_read(pcf, PCF50633_REG_INT3M); - pcf->standby_regs.int4m = __reg_read(pcf, PCF50633_REG_INT4M); - pcf->standby_regs.int5m = __reg_read(pcf, PCF50633_REG_INT5M); + /* set interrupt masks so only those sources we want to wake + * us are able to + */ __reg_write(pcf, PCF50633_REG_INT1M, ~pcf->pdata->resumers[0]); __reg_write(pcf, PCF50633_REG_INT2M, ~pcf->pdata->resumers[1]); __reg_write(pcf, PCF50633_REG_INT3M, ~pcf->pdata->resumers[2]); @@ -2240,16 +2250,13 @@ static int pcf50633_resume(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); struct pcf50633_data *pcf = i2c_get_clientdata(client); - int i; + int ret; mutex_lock(&pcf->lock); - /* Resume all saved registers that don't "survive" standby state */ - __reg_write(pcf, PCF50633_REG_INT1M, pcf->standby_regs.int1m); - __reg_write(pcf, PCF50633_REG_INT2M, pcf->standby_regs.int2m); - __reg_write(pcf, PCF50633_REG_INT3M, pcf->standby_regs.int3m); - __reg_write(pcf, PCF50633_REG_INT4M, pcf->standby_regs.int4m); - __reg_write(pcf, PCF50633_REG_INT5M, pcf->standby_regs.int5m); + pcf->have_been_suspended = 2; /* resuming */ + + /* these guys get reset while pcf50633 is suspend state, refresh */ __reg_write(pcf, PCF50633_REG_OOCTIM2, pcf->standby_regs.ooctim2); __reg_write(pcf, PCF50633_REG_AUTOOUT, pcf->standby_regs.autoout); @@ -2265,12 +2272,12 @@ static int pcf50633_resume(struct device *dev) if (!pcf->pdata->defer_resume_backlight) pcf50633_backlight_resume(pcf); - /* FIXME: one big read? */ - for (i = 0; i < 7; i++) { - u_int8_t reg_out = PCF50633_REG_LDO1OUT + 2*i; - __reg_write(pcf, reg_out, pcf->standby_regs.ldo[i].out); - __reg_write(pcf, reg_out+1, pcf->standby_regs.ldo[i].ena); - } + /* regulator voltages and enable states */ + ret = i2c_smbus_write_i2c_block_data(&pcf->client, + PCF50633_REG_LDO1OUT, 14, + &pcf->standby_regs.ldo[0][0]); + if (ret) + dev_err(dev, "Failed to restore LDOs :-( %d\n", ret); mutex_unlock(&pcf->lock); -- 1.5.6.3