lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 24 Sep 2009 01:12:22 +0200
From:	Jan Scholz <scholz@...s.uni-frankfurt.de>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Jan Scholz <scholz@...s.uni-frankfurt.de>,
	Paul Mackerras <paulus@...ba.org>, Ingo Molnar <mingo@...e.hu>,
	linux-kernel@...r.kernel.org, Adrian Bunk <bunk@...nel.org>,
	pm list <linux-pm@...ts.linux-foundation.org>,
	Johannes Berg <johannes@...solutions.net>
Subject: Re: [regression, bisected] adb trackpad disappears after suspend to ram

I have tested the patch against v2.6.30 and v2.6.31 and in both cases
the bug is fixed.

Thanks,
   Jan

"Rafael J. Wysocki" <rjw@...k.pl> writes:

> On Wednesday 23 September 2009, Benjamin Herrenschmidt wrote:
>> Allright, I think I nailed it (finally !)
>
> Great, thanks a lot for taking care of this!
>
>> Can everybody try this patch:
>> 
>> [PATCH] powerpc/pmac: Fix issues with sleep on some powerbooks
>> 
>> From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>> 
>> Since the change of how interrupts are disabled during suspend,
>> certain PowerBook models started exhibiting various issues during
>> suspend or resume from sleep.
>> 
>> I finally tracked it down to the code that runs various "platform"
>> functions (kind of little scripts extracted from the device-tree),
>> which uses our i2c and PMU drivers expecting interrutps to work,
>> and at a time where with the new scheme, they have been disabled.
>> 
>> This causes timeouts internally which for some reason results in
>> the PMU being unable to see the trackpad, among other issues, really
>> it depends on the machine. Most of the time, we fail to properly adjust
>> some clocks for suspend/resume so the results are not always
>> predictable.
>> 
>> This patch fixes it by using IRQF_TIMER for both the PMU and the I2C
>> interrupts. I prefer doing it this way than moving the call sites since
>> I really want those platform functions to still be called after all
>> drivers (and before sysdevs).
>
> Alternatively, you could introduce a new flag IRQF_NOSUSPEND and use that
> instead of IRQF_TIMER.  That would be cleaner than using IRQF_TIMER for
> non-timer interrupts IMHO.
>
>> We also do a slight cleanup to via-pmu.c driver to make sure the
>> ADB autopoll mask is handled correctly when doing bus resets
>> 
>> Signed-off-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>> ---
>> 
>> diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
>> index 21226b7..414ca98 100644
>> --- a/arch/powerpc/platforms/powermac/low_i2c.c
>> +++ b/arch/powerpc/platforms/powermac/low_i2c.c
>> @@ -540,8 +540,11 @@ static struct pmac_i2c_host_kw *__init kw_i2c_host_init(struct device_node *np)
>>  	/* Make sure IRQ is disabled */
>>  	kw_write_reg(reg_ier, 0);
>>  
>> -	/* Request chip interrupt */
>> -	if (request_irq(host->irq, kw_i2c_irq, 0, "keywest i2c", host))
>> +	/* Request chip interrupt. We set IRQF_TIMER because we don't
>> +	 * want that interrupt disabled between the 2 passes of driver
>> +	 * suspend or we'll have issues running the pfuncs
>> +	 */
>> +	if (request_irq(host->irq, kw_i2c_irq, IRQF_TIMER, "keywest i2c", host))
>>  		host->irq = NO_IRQ;
>>  
>>  	printk(KERN_INFO "KeyWest i2c @0x%08x irq %d %s\n",
>> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
>> index b40fb9b..6f308a4 100644
>> --- a/drivers/macintosh/via-pmu.c
>> +++ b/drivers/macintosh/via-pmu.c
>> @@ -405,7 +405,11 @@ static int __init via_pmu_start(void)
>>  		printk(KERN_ERR "via-pmu: can't map interrupt\n");
>>  		return -ENODEV;
>>  	}
>> -	if (request_irq(irq, via_pmu_interrupt, 0, "VIA-PMU", (void *)0)) {
>> +	/* We set IRQF_TIMER because we don't want the interrupt to be disabled
>> +	 * between the 2 passes of driver suspend, we control our own disabling
>> +	 * for that one
>> +	 */
>> +	if (request_irq(irq, via_pmu_interrupt, IRQF_TIMER, "VIA-PMU", (void *)0)) {
>>  		printk(KERN_ERR "via-pmu: can't request irq %d\n", irq);
>>  		return -ENODEV;
>>  	}
>> @@ -419,7 +423,7 @@ static int __init via_pmu_start(void)
>>  			gpio_irq = irq_of_parse_and_map(gpio_node, 0);
>>  
>>  		if (gpio_irq != NO_IRQ) {
>> -			if (request_irq(gpio_irq, gpio1_interrupt, 0,
>> +			if (request_irq(gpio_irq, gpio1_interrupt, IRQF_TIMER,
>>  					"GPIO1 ADB", (void *)0))
>>  				printk(KERN_ERR "pmu: can't get irq %d"
>>  				       " (GPIO1)\n", gpio_irq);
>> @@ -925,8 +929,7 @@ proc_write_options(struct file *file, const char __user *buffer,
>>  
>>  #ifdef CONFIG_ADB
>>  /* Send an ADB command */
>> -static int
>> -pmu_send_request(struct adb_request *req, int sync)
>> +static int pmu_send_request(struct adb_request *req, int sync)
>>  {
>>  	int i, ret;
>>  
>> @@ -1005,16 +1008,11 @@ pmu_send_request(struct adb_request *req, int sync)
>>  }
>>  
>>  /* Enable/disable autopolling */
>> -static int
>> -pmu_adb_autopoll(int devs)
>> +static int __pmu_adb_autopoll(int devs)
>>  {
>>  	struct adb_request req;
>>  
>> -	if ((vias == NULL) || (!pmu_fully_inited) || !pmu_has_adb)
>> -		return -ENXIO;
>> -
>>  	if (devs) {
>> -		adb_dev_map = devs;
>>  		pmu_request(&req, NULL, 5, PMU_ADB_CMD, 0, 0x86,
>>  			    adb_dev_map >> 8, adb_dev_map);
>>  		pmu_adb_flags = 2;
>> @@ -1027,9 +1025,17 @@ pmu_adb_autopoll(int devs)
>>  	return 0;
>>  }
>>  
>> +static int pmu_adb_autopoll(int devs)
>> +{
>> +	if ((vias == NULL) || (!pmu_fully_inited) || !pmu_has_adb)
>> +		return -ENXIO;
>> +
>> +	adb_dev_map = devs;
>> +	return __pmu_adb_autopoll(devs);
>> +}
>> +
>>  /* Reset the ADB bus */
>> -static int
>> -pmu_adb_reset_bus(void)
>> +static int pmu_adb_reset_bus(void)
>>  {
>>  	struct adb_request req;
>>  	int save_autopoll = adb_dev_map;
>> @@ -1038,13 +1044,13 @@ pmu_adb_reset_bus(void)
>>  		return -ENXIO;
>>  
>>  	/* anyone got a better idea?? */
>> -	pmu_adb_autopoll(0);
>> +	__pmu_adb_autopoll(0);
>>  
>> -	req.nbytes = 5;
>> +	req.nbytes = 4;
>>  	req.done = NULL;
>>  	req.data[0] = PMU_ADB_CMD;
>> -	req.data[1] = 0;
>> -	req.data[2] = ADB_BUSRESET;
>> +	req.data[1] = ADB_BUSRESET;
>> +	req.data[2] = 0;
>>  	req.data[3] = 0;
>>  	req.data[4] = 0;
>>  	req.reply_len = 0;
>> @@ -1056,7 +1062,7 @@ pmu_adb_reset_bus(void)
>>  	pmu_wait_complete(&req);
>>  
>>  	if (save_autopoll != 0)
>> -		pmu_adb_autopoll(save_autopoll);
>> +		__pmu_adb_autopoll(save_autopoll);
>>  
>>  	return 0;
>>  }
>> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
 Jan Scholz                                ____  ____    __    ___ 
                                          ( ___)(_  _)  /__\  / __)
 Frankfurt Institute for Advanced Studies  )__)  _)(_  /(__)\ \__ \
                                          (__)  (____)(__)(__)(___/
 Goethe Universitaet Frankfurt
 Ruth-Moufang-Str. 1                             Tel. 069-798-47534
 60438 Frankfurt am Main
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ