Re: [coreboot] New patch to review for coreboot: 9dcc23d w83627hf: add method to retrive wake event source register to ASL include

[Paul Menzel]

Dear Christoph,

Am Mittwoch, den 19.10.2011, 02:28 +0200 schrieb Christoph Grenz:
> Christoph Grenz ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/297
>
> -gerrit
>
> commit 9dcc23d61a765a6967dc0351754eebedbedab7ac
> Author: Christoph Grenz 
> Date:   Wed Oct 19 02:25:08 2011 +0200
> w83627hf: add method to retrive wake event source register to ASL include

s/retrive/retrieve/
> Add a method WAKS to devtree.asl which returns the wake-up status register

Should it be »wake-up *source* register«? The data sheet [1] calls it
»CRE3 (Read only) Keyboard/Mouse Wake-Up Status register« but source
seems to be a better description as you also used it in the code.

I send this message because I got confused by all the different names
for the same register, if I am not mistaken.
> to simplify retrieving the wake source e.g. in \_WAK.    
>
> Change-Id: Ia258f8fc9ff79b18391c55464da73863889e2255
> Signed-off-by: Christoph Grenz <[e-mail]>
>
> src/superio/winbond/w83627hf/devtree.asl |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/src/superio/winbond/w83627hf/devtree.asl b/src/superio/winbond/w83627hf/devtree.asl
> index cbc132d..2e31de5 100644
> --- a/src/superio/winbond/w83627hf/devtree.asl
> +++ b/src/superio/winbond/w83627hf/devtree.asl
> @@ -1503,5 +1503,22 @@ Scope (_SB)
>    		}
>    	}
>    	#endif
> /* Returns the wake source register.
>    It is cleared after reading.
>    Bit 0: Keyboard wake-up event
>    Bit 1: Mouse wake-up event
>    Bit 2: Power button event
>    Bit 3: CIR wake-up event
>    Bit 4: Power loss event
>    Bit 5: VSB power loss status
> */
> Method (WAKS) {
> 	ENCM ()
> 	Store (0x0B, LDN)
> 	Store (CRE3, Local0)
> 	EXCM ()
> 	Return (Local0)
> }
> }
>
> }

Otherwise I do not know ASL, but I guess you tested that, so it looks
good to me.

Thanks,

Paul

[1] http://www.itox.com/pages/support/wdt/W83627HF.pdf