linux-kernel - Re: [alsa-devel] PNP_DRIVER_RES_DISABLE breaks swsusp at least with snd

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]

Message-ID: <4786C40B.8050005@keyaccess.nl>
Date:	Fri, 11 Jan 2008 02:19:07 +0100
From:	Rene Herman <rene.herman@...access.nl>
To:	Jaroslav Kysela <perex@...ex.cz>
CC:	ALSA development <alsa-devel@...a-project.org>,
	Ondrej Zary <linux@...nbow-software.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <bjorn.helgaas@...com>,
	Pierre Ossman <drzeus@...eus.cx>,
	Andrew Morton <akpm@...l.org>, Takashi Iwai <tiwai@...e.de>
Subject: Re: [alsa-devel] PNP_DRIVER_RES_DISABLE breaks swsusp at least with
 snd_cs4236

On 10-01-08 08:58, Jaroslav Kysela wrote:

> On Thu, 10 Jan 2008, Rene Herman wrote:
> 
>> On 09-01-08 23:43, Ondrej Zary wrote:
>>
>> Jaroslav -- in your role as ISA-PnP maintainer and Bjorn, in yours as 
>> having been foollish enough to touch PnP recently:
>>
>>> as hibernation (swsusp) started to work with my CPU, I found that my Turtle 
>>> Beach Malibu stops working after resume from hibernation. It's caused by fact 
>>> that the card is not enabled on the pnp layer during resume - and thus card 
>>> registers are inaccessible (reads return FFs, writes go nowhere).
>>>
>>> During resume, pnp_bus_resume() in drivers/pnp/driver.c is called for each pnp 
>>> device. This function calls pnp_start_dev() only when the 
>>> PNP_DRIVER_RES_DO_NOT_CHANGE bit is NOT seting pnp_drv->flags. But the cs4236 
>>> driver in sound/isa/cs423x/cs4236.c explicitly sets the .flags to 
>>> PNP_DRIVER_RES_DISABLE - it's value is 3 and that includes 
>>> PNP_DRIVER_RES_DO_NOT_CHANGE bit.
>> Ehm. Isn't that a bit unexpected:
>>
>> #define PNP_DRIVER_RES_DO_NOT_CHANGE    0x0001  /* do not change the state 
>> of the device */
>> #define PNP_DRIVER_RES_DISABLE          0x0003  /* ensure the device is 
>> disabled */
>>
>> I'd say that disabling is changing, so isn't this just a braino where 
>> someone meant to write 2 instead of 3?
> 
> It's irrelevant. I think that condition in driver.c in suspend and resume 
> callbacks is invalid, because PNP_DRIVER_RES_DO_NOT_CHANGE means that 
> resources should not be changed in the pnp core but only in the driver, 
> but in suspend/resume process are resources preserved, so the condition 
> should be removed.

I see a PnP resume _consists_ of setting the resources so I can see the test 
implementation wise, but yes, conceptually it seems this test shouldn't be 
present. So just like the attached then?

Pierre, what was the idea here? Added the other SOBs to the CC as well...

> Author of this code is:
> 
> author Pierre Ossman <drzeus-list@...eus.cx> Tue, 29 Nov 2005 09:09:32 +0100
> committer Jaroslav Kysela <perex@...e.cz> Tue, 03 Jan 2006 12:31:30 +0100
> 
>     [ALSA] [PATCH] alsa: Improved PnP suspend support
> 
>     Also use the PnP functions to start/stop the devices during the suspend so
>     that drivers will not have to duplicate this code.
> 
>     Cc: Adam Belay <ambx1@....rr.com>
>     Cc: Jaroslav Kysela <perex@...e.cz>
>     Cc: Takashi Iwai <tiwai@...e.de>
> 
>     Signed-off-by: Pierre Ossman <drzeus@...eus.cx>
>     Signed-off-by: Andrew Morton <akpm@...l.org>
>     Signed-off-by: Takashi Iwai <tiwai@...e.de>

Rene.


View attachment "pnp_driver_res_do_not_test.diff" of type "text/plain" (945 bytes)