[PATCH] Optional Beeping During Resume From Suspend To Ram.

[PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 6179 bytes --]

Hi all

Here's what I have after today's work.

I haven't yet been able to test on x86, but can confirm that it works okay on x86_64. I'm currently working towards testing it on my old Omnibook. My P4 desktop won't resume from suspend to ram at all, and hasn't produced any beeps.

I needed to move the BEEP invocation to after the data segment is reloaded, so that the test could access the variable. That was pretty tricky to find - no oops or anything bad prior, it just didn't beep when expected.

A couple of notes:

- I'd like to put the BEEP macro somewhere that can be shared by x86 32 and 64. If that's a good idea, any suggestions on where? Nothing occurs to me straight off.
- I've just switched from Evo to Kmail. Please let me know if there's any mangling of the patch.

Regards,

Nigel

 arch/i386/kernel/acpi/wakeup.S   |   29 ++++++++++++++++++++++++++++-
 arch/x86_64/kernel/acpi/wakeup.S |   25 +++++++++++++++++++++++++
 include/linux/acpi.h             |    1 +
 kernel/power/main.c              |   23 +++++++++++++++++++++++
 4 files changed, 77 insertions(+), 1 deletion(-)
diff -ruNp 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S
--- 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S	2007-06-19 12:15:25.000000000 +1000
+++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S	2007-06-19 21:14:49.000000000 +1000
@@ -11,7 +11,22 @@
 #
 # If physical address of wakeup_code is 0x12345, BIOS should call us with
 # cs = 0x1234, eip = 0x05
-# 
+#
+
+#define BEEP \
+	inb	$97, %al; 	\
+	outb	%al, $0x80; 	\
+	movb	$3, %al; 	\
+	outb	%al, $97; 	\
+	outb	%al, $0x80; 	\
+	movb	$-74, %al; 	\
+	outb	%al, $67; 	\
+	outb	%al, $0x80; 	\
+	movb	$-119, %al; 	\
+	outb	%al, $66; 	\
+	outb	%al, $0x80; 	\
+	movb	$15, %al; 	\
+	outb	%al, $66;
 
 ALIGN
 	.align	4096
@@ -31,6 +46,11 @@ wakeup_code:
 	movw	%cs, %ax
 	movw	%ax, %ds					# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
+
+	testl   $1, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
 	mov	$(wakeup_stack - wakeup_code), %sp		# Private stack is needed for ASUS board
 	movw	$0x0e00 + 'S', %fs:(0x12)
 
@@ -88,6 +108,10 @@ wakeup_code:
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
+	testl   $2, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
 	ljmpl	$__KERNEL_CS,$wakeup_pmode_return
 
 real_save_gdt:	.word 0
@@ -98,6 +122,7 @@ real_save_cr4:	.long 0
 real_magic:	.long 0
 video_mode:	.long 0
 video_flags:	.long 0
+beep_flags:	.long 0
 real_efer_save_restore:	.long 0
 real_save_efer_edx: 	.long 0
 real_save_efer_eax: 	.long 0
@@ -261,6 +286,8 @@ ENTRY(acpi_copy_wakeup_routine)
 	movl	%edx, video_mode - wakeup_start (%eax)
 	movl	acpi_video_flags, %edx
 	movl	%edx, video_flags - wakeup_start (%eax)
+	movl	s2ram_beep, %edx
+	movl	%edx, beep_flags - wakeup_start (%eax)
 	movl	$0x12345678, real_magic - wakeup_start (%eax)
 	movl	$0x12345678, saved_magic
 	ret
diff -ruNp 970-str-beep.patch-old/arch/x86_64/kernel/acpi/wakeup.S 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S
--- 970-str-beep.patch-old/arch/x86_64/kernel/acpi/wakeup.S	2007-06-19 12:15:28.000000000 +1000
+++ 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S	2007-06-19 21:14:49.000000000 +1000
@@ -16,6 +16,21 @@
 # cs = 0x1234, eip = 0x05
 #
 
+#define BEEP \
+	inb	$97, %al; 	\
+	outb	%al, $0x80; 	\
+	movb	$3, %al; 	\
+	outb	%al, $97; 	\
+	outb	%al, $0x80; 	\
+	movb	$-74, %al; 	\
+	outb	%al, $67; 	\
+	outb	%al, $0x80; 	\
+	movb	$-119, %al; 	\
+	outb	%al, $66; 	\
+	outb	%al, $0x80; 	\
+	movb	$15, %al; 	\
+	outb	%al, $66;
+
 
 ALIGN
 	.align	16
@@ -33,6 +48,13 @@ wakeup_code:
 	movw	%cs, %ax
 	movw	%ax, %ds		# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
+
+	# Data segment must be set up before we can see whether to beep.
+	testl   $1, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
+
 					# Private stack is needed for ASUS board
 	mov	$(wakeup_stack - wakeup_code), %sp
 
@@ -229,6 +251,7 @@ gdt_48a:
 	.long   gdta - wakeup_code              # gdt base (relocated in later)
 	
 real_magic:	.quad 0
+beep_flags:	.quad 0
 video_mode:	.quad 0
 video_flags:	.quad 0
 
@@ -344,6 +367,8 @@ ENTRY(acpi_copy_wakeup_routine)
 	pushq	%rax
 	pushq	%rdx
 
+	movl	s2ram_beep, %edx
+	movl	%edx, beep_flags - wakeup_start (,%rdi)
 	movl	saved_video_mode, %edx
 	movl	%edx, video_mode - wakeup_start (,%rdi)
 	movl	acpi_video_flags, %edx
diff -ruNp 970-str-beep.patch-old/include/linux/acpi.h 970-str-beep.patch-new/include/linux/acpi.h
--- 970-str-beep.patch-old/include/linux/acpi.h	2007-06-19 21:15:08.000000000 +1000
+++ 970-str-beep.patch-new/include/linux/acpi.h	2007-06-19 21:14:49.000000000 +1000
@@ -123,6 +123,7 @@ extern int pci_mmcfg_config_num;
 
 extern int sbf_port;
 extern unsigned long acpi_video_flags;
+extern unsigned long s2ram_beep;
 
 #else	/* !CONFIG_ACPI */
 
diff -ruNp 970-str-beep.patch-old/kernel/power/main.c 970-str-beep.patch-new/kernel/power/main.c
--- 970-str-beep.patch-old/kernel/power/main.c	2007-06-19 12:15:55.000000000 +1000
+++ 970-str-beep.patch-new/kernel/power/main.c	2007-06-19 21:14:49.000000000 +1000
@@ -308,6 +308,27 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
+unsigned long s2ram_beep = 0;
+
+static ssize_t s2ram_beep_show(struct kset *kset, char *buf)
+{
+	return sprintf(buf, "%d\n", s2ram_beep);
+}
+
+static ssize_t
+s2ram_beep_store(struct kset *kset, const char *buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) > 0) {
+		s2ram_beep = val;
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(s2ram_beep);
+
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -333,11 +354,13 @@ power_attr(pm_trace);
 static struct attribute * g[] = {
 	&state_attr.attr,
 	&pm_trace_attr.attr,
+	&s2ram_beep_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&s2ram_beep_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */


-- 
Nigel, Michelle and Alisdair Cunningham
5 Mitchell Street
Cobden 3266
Victoria, Australia

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi Nigel,

On Tuesday, 19 June 2007 13:18, Nigel Cunningham wrote:
> Hi all
> 
> Here's what I have after today's work.
> 
> I haven't yet been able to test on x86, but can confirm that it works okay on
> x86_64. I'm currently working towards testing it on my old Omnibook. My P4
> desktop won't resume from suspend to ram at all, and hasn't produced any
> beeps.   
> 
> I needed to move the BEEP invocation to after the data segment is reloaded,
> so that the test could access the variable. That was pretty tricky to find -
> no oops or anything bad prior, it just didn't beep when expected.  
> 
> A couple of notes:
> 
> - I'd like to put the BEEP macro somewhere that can be shared by x86 32 and
> 64. If that's a good idea, any suggestions on where? Nothing occurs to me
> straight off.  

I don't know either.  I think Andi is the right person to ask (CC added).

> - I've just switched from Evo to Kmail. Please let me know if there's any
> mangling of the patch. 

The patch looks good to me.  I'll try to run it on an i386 tomorrow.

Greetings,
Rafael 


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi,

On Tuesday, 19 June 2007 23:33, Rafael J. Wysocki wrote:
> On Tuesday, 19 June 2007 13:18, Nigel Cunningham wrote:
> > Hi all
> > 
> > Here's what I have after today's work.
> > 
> > I haven't yet been able to test on x86, but can confirm that it works okay on
> > x86_64. I'm currently working towards testing it on my old Omnibook. My P4
> > desktop won't resume from suspend to ram at all, and hasn't produced any
> > beeps.   
> > 
> > I needed to move the BEEP invocation to after the data segment is reloaded,
> > so that the test could access the variable. That was pretty tricky to find -
> > no oops or anything bad prior, it just didn't beep when expected.  
> > 
> > A couple of notes:
> > 
> > - I'd like to put the BEEP macro somewhere that can be shared by x86 32 and
> > 64. If that's a good idea, any suggestions on where? Nothing occurs to me
> > straight off.  
> 
> I don't know either.  I think Andi is the right person to ask (CC added).
> 
> > - I've just switched from Evo to Kmail. Please let me know if there's any
> > mangling of the patch. 

Unfortunately, the message is encoded as "quoted printable" which results in
the whitespace being mangled.

> The patch looks good to me.  I'll try to run it on an i386 tomorrow.

I've recoded it and rediffed against -rc5 with the hibernation and suspend
patchset.  The resut is at

http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc5/patches/29-Optional-Beeping-During-Resume-From-RAM.patch

Well, my i386 test box doesn't seem to be able to beep at all.  I'll have to
look for another one ...

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 7323 bytes --]

Hi.

On Thursday 21 June 2007 08:09:26 Rafael J. Wysocki wrote:
> Hi,
> 
> On Tuesday, 19 June 2007 23:33, Rafael J. Wysocki wrote:
> > On Tuesday, 19 June 2007 13:18, Nigel Cunningham wrote:
> > > Hi all
> > > 
> > > Here's what I have after today's work.
> > > 
> > > I haven't yet been able to test on x86, but can confirm that it works okay on
> > > x86_64. I'm currently working towards testing it on my old Omnibook. My P4
> > > desktop won't resume from suspend to ram at all, and hasn't produced any
> > > beeps.   
> > > 
> > > I needed to move the BEEP invocation to after the data segment is reloaded,
> > > so that the test could access the variable. That was pretty tricky to find -
> > > no oops or anything bad prior, it just didn't beep when expected.  
> > > 
> > > A couple of notes:
> > > 
> > > - I'd like to put the BEEP macro somewhere that can be shared by x86 32 and
> > > 64. If that's a good idea, any suggestions on where? Nothing occurs to me
> > > straight off.  
> > 
> > I don't know either.  I think Andi is the right person to ask (CC added).
> > 
> > > - I've just switched from Evo to Kmail. Please let me know if there's any
> > > mangling of the patch. 
> 
> Unfortunately, the message is encoded as "quoted printable" which results in
> the whitespace being mangled.
> 
> > The patch looks good to me.  I'll try to run it on an i386 tomorrow.
> 
> I've recoded it and rediffed against -rc5 with the hibernation and suspend
> patchset.  The resut is at
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc5/patches/29-Optional-Beeping-During-Resume-From-RAM.patch
> 
> Well, my i386 test box doesn't seem to be able to beep at all.  I'll have to
> look for another one ...

Ok. No success with the Omnibook yet, though I did see it mentioned in Documentation/... as needing a vbepost.

Think I found the right option. Does this patch come through better?

Nigel

 arch/i386/kernel/acpi/wakeup.S   |   29 ++++++++++++++++++++++++++++-
 arch/x86_64/kernel/acpi/wakeup.S |   25 +++++++++++++++++++++++++
 include/linux/acpi.h             |    1 +
 kernel/power/main.c              |   23 +++++++++++++++++++++++
 4 files changed, 77 insertions(+), 1 deletion(-)
diff -ruNp 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S
--- 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S	2007-06-19 12:15:25.000000000 +1000
+++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S	2007-06-19 21:14:49.000000000 +1000
@@ -11,7 +11,22 @@
 #
 # If physical address of wakeup_code is 0x12345, BIOS should call us with
 # cs = 0x1234, eip = 0x05
-# 
+#
+
+#define BEEP \
+	inb	$97, %al; 	\
+	outb	%al, $0x80; 	\
+	movb	$3, %al; 	\
+	outb	%al, $97; 	\
+	outb	%al, $0x80; 	\
+	movb	$-74, %al; 	\
+	outb	%al, $67; 	\
+	outb	%al, $0x80; 	\
+	movb	$-119, %al; 	\
+	outb	%al, $66; 	\
+	outb	%al, $0x80; 	\
+	movb	$15, %al; 	\
+	outb	%al, $66;
 
 ALIGN
 	.align	4096
@@ -31,6 +46,11 @@ wakeup_code:
 	movw	%cs, %ax
 	movw	%ax, %ds					# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
+
+	testl   $1, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
 	mov	$(wakeup_stack - wakeup_code), %sp		# Private stack is needed for ASUS board
 	movw	$0x0e00 + 'S', %fs:(0x12)
 
@@ -88,6 +108,10 @@ wakeup_code:
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
+	testl   $2, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
 	ljmpl	$__KERNEL_CS,$wakeup_pmode_return
 
 real_save_gdt:	.word 0
@@ -98,6 +122,7 @@ real_save_cr4:	.long 0
 real_magic:	.long 0
 video_mode:	.long 0
 video_flags:	.long 0
+beep_flags:	.long 0
 real_efer_save_restore:	.long 0
 real_save_efer_edx: 	.long 0
 real_save_efer_eax: 	.long 0
@@ -261,6 +286,8 @@ ENTRY(acpi_copy_wakeup_routine)
 	movl	%edx, video_mode - wakeup_start (%eax)
 	movl	acpi_video_flags, %edx
 	movl	%edx, video_flags - wakeup_start (%eax)
+	movl	s2ram_beep, %edx
+	movl	%edx, beep_flags - wakeup_start (%eax)
 	movl	$0x12345678, real_magic - wakeup_start (%eax)
 	movl	$0x12345678, saved_magic
 	ret
diff -ruNp 970-str-beep.patch-old/arch/x86_64/kernel/acpi/wakeup.S 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S
--- 970-str-beep.patch-old/arch/x86_64/kernel/acpi/wakeup.S	2007-06-19 12:15:28.000000000 +1000
+++ 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S	2007-06-19 21:14:49.000000000 +1000
@@ -16,6 +16,21 @@
 # cs = 0x1234, eip = 0x05
 #
 
+#define BEEP \
+	inb	$97, %al; 	\
+	outb	%al, $0x80; 	\
+	movb	$3, %al; 	\
+	outb	%al, $97; 	\
+	outb	%al, $0x80; 	\
+	movb	$-74, %al; 	\
+	outb	%al, $67; 	\
+	outb	%al, $0x80; 	\
+	movb	$-119, %al; 	\
+	outb	%al, $66; 	\
+	outb	%al, $0x80; 	\
+	movb	$15, %al; 	\
+	outb	%al, $66;
+
 
 ALIGN
 	.align	16
@@ -33,6 +48,13 @@ wakeup_code:
 	movw	%cs, %ax
 	movw	%ax, %ds		# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
+
+	# Data segment must be set up before we can see whether to beep.
+	testl   $1, beep_flags - wakeup_code
+	jz      1f
+	BEEP
+1:
+
 					# Private stack is needed for ASUS board
 	mov	$(wakeup_stack - wakeup_code), %sp
 
@@ -229,6 +251,7 @@ gdt_48a:
 	.long   gdta - wakeup_code              # gdt base (relocated in later)
 	
 real_magic:	.quad 0
+beep_flags:	.quad 0
 video_mode:	.quad 0
 video_flags:	.quad 0
 
@@ -344,6 +367,8 @@ ENTRY(acpi_copy_wakeup_routine)
 	pushq	%rax
 	pushq	%rdx
 
+	movl	s2ram_beep, %edx
+	movl	%edx, beep_flags - wakeup_start (,%rdi)
 	movl	saved_video_mode, %edx
 	movl	%edx, video_mode - wakeup_start (,%rdi)
 	movl	acpi_video_flags, %edx
diff -ruNp 970-str-beep.patch-old/include/linux/acpi.h 970-str-beep.patch-new/include/linux/acpi.h
--- 970-str-beep.patch-old/include/linux/acpi.h	2007-06-19 21:15:08.000000000 +1000
+++ 970-str-beep.patch-new/include/linux/acpi.h	2007-06-19 21:14:49.000000000 +1000
@@ -123,6 +123,7 @@ extern int pci_mmcfg_config_num;
 
 extern int sbf_port;
 extern unsigned long acpi_video_flags;
+extern unsigned long s2ram_beep;
 
 #else	/* !CONFIG_ACPI */
 
diff -ruNp 970-str-beep.patch-old/kernel/power/main.c 970-str-beep.patch-new/kernel/power/main.c
--- 970-str-beep.patch-old/kernel/power/main.c	2007-06-19 12:15:55.000000000 +1000
+++ 970-str-beep.patch-new/kernel/power/main.c	2007-06-19 21:14:49.000000000 +1000
@@ -308,6 +308,27 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
+unsigned long s2ram_beep = 0;
+
+static ssize_t s2ram_beep_show(struct kset *kset, char *buf)
+{
+	return sprintf(buf, "%d\n", s2ram_beep);
+}
+
+static ssize_t
+s2ram_beep_store(struct kset *kset, const char *buf, size_t n)
+{
+	int val;
+
+	if (sscanf(buf, "%d", &val) > 0) {
+		s2ram_beep = val;
+		return n;
+	}
+	return -EINVAL;
+}
+
+power_attr(s2ram_beep);
+
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -333,11 +354,13 @@ power_attr(pm_trace);
 static struct attribute * g[] = {
 	&state_attr.attr,
 	&pm_trace_attr.attr,
+	&s2ram_beep_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
+	&s2ram_beep_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */


-- 
Nigel, Michelle and Alisdair Cunningham
5 Mitchell Street
Cobden 3266
Victoria, Australia

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi,

On Thursday, 21 June 2007 00:24, Nigel Cunningham wrote:
> On Thursday 21 June 2007 08:09:26 Rafael J. Wysocki wrote:
> > On Tuesday, 19 June 2007 23:33, Rafael J. Wysocki wrote:
> > > On Tuesday, 19 June 2007 13:18, Nigel Cunningham wrote:
> > > > Hi all
> > > > 
> > > > Here's what I have after today's work.
> > > > 
> > > > I haven't yet been able to test on x86, but can confirm that it works okay on
> > > > x86_64. I'm currently working towards testing it on my old Omnibook. My P4
> > > > desktop won't resume from suspend to ram at all, and hasn't produced any
> > > > beeps.   
> > > > 
> > > > I needed to move the BEEP invocation to after the data segment is reloaded,
> > > > so that the test could access the variable. That was pretty tricky to find -
> > > > no oops or anything bad prior, it just didn't beep when expected.  
> > > > 
> > > > A couple of notes:
> > > > 
> > > > - I'd like to put the BEEP macro somewhere that can be shared by x86 32 and
> > > > 64. If that's a good idea, any suggestions on where? Nothing occurs to me
> > > > straight off.  
> > > 
> > > I don't know either.  I think Andi is the right person to ask (CC added).
> > > 
> > > > - I've just switched from Evo to Kmail. Please let me know if there's any
> > > > mangling of the patch. 
> > 
> > Unfortunately, the message is encoded as "quoted printable" which results in
> > the whitespace being mangled.
> > 
> > > The patch looks good to me.  I'll try to run it on an i386 tomorrow.
> > 
> > I've recoded it and rediffed against -rc5 with the hibernation and suspend
> > patchset.  The resut is at
> > 
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc5/patches/29-Optional-Beeping-During-Resume-From-RAM.patch
> > 
> > Well, my i386 test box doesn't seem to be able to beep at all.  I'll have to
> > look for another one ...
> 
> Ok. No success with the Omnibook yet, though I did see it mentioned in Documentation/... as needing a vbepost.
> 
> Think I found the right option. Does this patch come through better?

No, it's still "quoted printable", at least on my system (but I use Kmail too).

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> Hi all
> 
> Here's what I have after today's work.
> 
> I haven't yet been able to test on x86, but can confirm that it works okay on x86_64. I'm currently working towards testing it on my old Omnibook. My P4 desktop won't resume from suspend to ram at all, and hasn't produced any beeps.
> 
> I needed to move the BEEP invocation to after the data segment is reloaded, so that the test could access the variable. That was pretty tricky to find - no oops or anything bad prior, it just didn't beep when expected.
> 
> A couple of notes:
> 
> - I'd like to put the BEEP macro somewhere that can be shared by x86 32 and 64. If that's a good idea, any suggestions on where? Nothing occurs to me straight off.
> - I've just switched from Evo to Kmail. Please let me know if there's any mangling of the patch.
> 
> Regards,
> 
> Nigel
> 
>  arch/i386/kernel/acpi/wakeup.S   |   29 ++++++++++++++++++++++++++++-
>  arch/x86_64/kernel/acpi/wakeup.S |   25 +++++++++++++++++++++++++
>  include/linux/acpi.h             |    1 +
>  kernel/power/main.c              |   23 +++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 1 deletion(-)
> diff -ruNp 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S
> --- 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S	2007-06-19 12:15:25.000000000 +1000
> +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S	2007-06-19 21:14:49.000000000 +1000
> @@ -11,7 +11,22 @@
>  #
>  # If physical address of wakeup_code is 0x12345, BIOS should call us with
>  # cs = 0x1234, eip = 0x05
> -# 
> +#
> +
> +#define BEEP \
> +	inb	$97, %al; 	\
> +	outb	%al, $0x80; 	\
> +	movb	$3, %al; 	\
> +	outb	%al, $97; 	\
> +	outb	%al, $0x80; 	\
> +	movb	$-74, %al; 	\
> +	outb	%al, $67; 	\
> +	outb	%al, $0x80; 	\
> +	movb	$-119, %al; 	\
> +	outb	%al, $66; 	\
> +	outb	%al, $0x80; 	\
> +	movb	$15, %al; 	\
> +	outb	%al, $66;
>  
>  ALIGN
>  	.align	4096
> @@ -31,6 +46,11 @@ wakeup_code:
>  	movw	%cs, %ax
>  	movw	%ax, %ds					# Make ds:0 point to wakeup_start
>  	movw	%ax, %ss
> +
> +	testl   $1, beep_flags - wakeup_code
> +	jz      1f
> +	BEEP
> +1:

Can we rename/reuse existing flag variable?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2911 bytes --]

Hi.

On Friday 29 June 2007 00:25:32 Pavel Machek wrote:
> Hi!
> 
> > Hi all
> > 
> > Here's what I have after today's work.
> > 
> > I haven't yet been able to test on x86, but can confirm that it works okay 
on x86_64. I'm currently working towards testing it on my old Omnibook. My P4 
desktop won't resume from suspend to ram at all, and hasn't produced any 
beeps.
> > 
> > I needed to move the BEEP invocation to after the data segment is 
reloaded, so that the test could access the variable. That was pretty tricky 
to find - no oops or anything bad prior, it just didn't beep when expected.
> > 
> > A couple of notes:
> > 
> > - I'd like to put the BEEP macro somewhere that can be shared by x86 32 
and 64. If that's a good idea, any suggestions on where? Nothing occurs to me 
straight off.
> > - I've just switched from Evo to Kmail. Please let me know if there's any 
mangling of the patch.
> > 
> > Regards,
> > 
> > Nigel
> > 
> >  arch/i386/kernel/acpi/wakeup.S   |   29 ++++++++++++++++++++++++++++-
> >  arch/x86_64/kernel/acpi/wakeup.S |   25 +++++++++++++++++++++++++
> >  include/linux/acpi.h             |    1 +
> >  kernel/power/main.c              |   23 +++++++++++++++++++++++
> >  4 files changed, 77 insertions(+), 1 deletion(-)
> > diff -ruNp 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S 
970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S
> > --- 970-str-beep.patch-old/arch/i386/kernel/acpi/wakeup.S	2007-06-19 
12:15:25.000000000 +1000
> > +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S	2007-06-19 
21:14:49.000000000 +1000
> > @@ -11,7 +11,22 @@
> >  #
> >  # If physical address of wakeup_code is 0x12345, BIOS should call us with
> >  # cs = 0x1234, eip = 0x05
> > -# 
> > +#
> > +
> > +#define BEEP \
> > +	inb	$97, %al; 	\
> > +	outb	%al, $0x80; 	\
> > +	movb	$3, %al; 	\
> > +	outb	%al, $97; 	\
> > +	outb	%al, $0x80; 	\
> > +	movb	$-74, %al; 	\
> > +	outb	%al, $67; 	\
> > +	outb	%al, $0x80; 	\
> > +	movb	$-119, %al; 	\
> > +	outb	%al, $66; 	\
> > +	outb	%al, $0x80; 	\
> > +	movb	$15, %al; 	\
> > +	outb	%al, $66;
> >  
> >  ALIGN
> >  	.align	4096
> > @@ -31,6 +46,11 @@ wakeup_code:
> >  	movw	%cs, %ax
> >  	movw	%ax, %ds					# Make ds:0 point to wakeup_start
> >  	movw	%ax, %ss
> > +
> > +	testl   $1, beep_flags - wakeup_code
> > +	jz      1f
> > +	BEEP
> > +1:
> 
> Can we rename/reuse existing flag variable?

Sorry, but I can't resist the opportunity to say "Send a patch!" :)

Seriously, though, I'd prefer not to. If we rename that acpi video flags 
variable (I assume this is what you're thinking of), we only create cause for 
confusion. A variable should for debugging or for controlling quirks, not for 
both at the same time.

Regards,

Nigel
-- 
Nigel, Michelle and Alisdair Cunningham
5 Mitchell Street
Cobden 3266
Victoria, Australia

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Stefan Seyfried]

On Fri, Jun 29, 2007 at 08:27:12AM +1000, Nigel Cunningham wrote:
> > Can we rename/reuse existing flag variable?
> 
> Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> 
> Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> variable (I assume this is what you're thinking of), we only create cause for 
> confusion. A variable should for debugging or for controlling quirks, not for 
> both at the same time.

I agree. And video_flags is something totally different :-)
I just used that one in my ad-hoc hack (which actually was only to illustrate
the idea) because a) it was enough to show the intent and b) i did not know
how to do it better ;-)
-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out." 

This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > >  ALIGN
> > >  	.align	4096
> > > @@ -31,6 +46,11 @@ wakeup_code:
> > >  	movw	%cs, %ax
> > >  	movw	%ax, %ds					# Make ds:0 point to wakeup_start
> > >  	movw	%ax, %ss
> > > +
> > > +	testl   $1, beep_flags - wakeup_code
> > > +	jz      1f
> > > +	BEEP
> > > +1:
> > 
> > Can we rename/reuse existing flag variable?
> 
> Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> 
> Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> variable (I assume this is what you're thinking of), we only create cause for 
> confusion. A variable should for debugging or for controlling quirks, not for 
> both at the same time.

Cause for confusion? We are currently using 2 bits of that variable,
and we want to add one more bit. I seriously doubt that can confuse
anyone.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > > Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> > > 
> > > Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> > > variable (I assume this is what you're thinking of), we only create cause for 
> > > confusion. A variable should for debugging or for controlling quirks, not for 
> > > both at the same time.
> > 
> > Cause for confusion? We are currently using 2 bits of that variable,
> > and we want to add one more bit. I seriously doubt that can confuse
> > anyone.
> 
> Well, indeed it would be more elegant to rename the existing flags variable
> and use another bit out of it, but I personally don't think it's _that_
> important.  At least, I don't think we should block the patch
> because of that.

It is not _that_ important.

> BTW, has anyone confirmed that it works on i386?

If you have patch somewhere nearby, I can test it on i386 and make it
use just one flags variable.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi,

On Saturday, 30 June 2007 00:35, Pavel Machek wrote:
> Hi!
> 
> > > >  ALIGN
> > > >  	.align	4096
> > > > @@ -31,6 +46,11 @@ wakeup_code:
> > > >  	movw	%cs, %ax
> > > >  	movw	%ax, %ds					# Make ds:0 point to wakeup_start
> > > >  	movw	%ax, %ss
> > > > +
> > > > +	testl   $1, beep_flags - wakeup_code
> > > > +	jz      1f
> > > > +	BEEP
> > > > +1:
> > > 
> > > Can we rename/reuse existing flag variable?
> > 
> > Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> > 
> > Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> > variable (I assume this is what you're thinking of), we only create cause for 
> > confusion. A variable should for debugging or for controlling quirks, not for 
> > both at the same time.
> 
> Cause for confusion? We are currently using 2 bits of that variable,
> and we want to add one more bit. I seriously doubt that can confuse
> anyone.

Well, indeed it would be more elegant to rename the existing flags variable
and use another bit out of it, but I personally don't think it's _that_
important.  At least, I don't think we should block the patch because of that.

BTW, has anyone confirmed that it works on i386?

BTW2, Nigel, please fix the formats in s2ram_beep_show()/_store(), they
cause gcc to spit ugly warnings on some architectures.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi,

On Saturday, 30 June 2007 12:11, Pavel Machek wrote:
> Hi!
> 
> > > > Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> > > > 
> > > > Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> > > > variable (I assume this is what you're thinking of), we only create cause for 
> > > > confusion. A variable should for debugging or for controlling quirks, not for 
> > > > both at the same time.
> > > 
> > > Cause for confusion? We are currently using 2 bits of that variable,
> > > and we want to add one more bit. I seriously doubt that can confuse
> > > anyone.
> > 
> > Well, indeed it would be more elegant to rename the existing flags variable
> > and use another bit out of it, but I personally don't think it's _that_
> > important.  At least, I don't think we should block the patch
> > because of that.
> 
> It is not _that_ important.
> 
> > BTW, has anyone confirmed that it works on i386?
> 
> If you have patch somewhere nearby, I can test it on i386 and make it
> use just one flags variable.

http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc6/patches/28-Optional-Beeping-During-Resume-From-RAM.patch

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > > > > Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> > > > > 
> > > > > Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> > > > > variable (I assume this is what you're thinking of), we only create cause for 
> > > > > confusion. A variable should for debugging or for controlling quirks, not for 
> > > > > both at the same time.
> > > > 
> > > > Cause for confusion? We are currently using 2 bits of that variable,
> > > > and we want to add one more bit. I seriously doubt that can confuse
> > > > anyone.
> > > 
> > > Well, indeed it would be more elegant to rename the existing flags variable
> > > and use another bit out of it, but I personally don't think it's _that_
> > > important.  At least, I don't think we should block the patch
> > > because of that.
> > 
> > It is not _that_ important.
> > 
> > > BTW, has anyone confirmed that it works on i386?
> > 
> > If you have patch somewhere nearby, I can test it on i386 and make it
> > use just one flags variable.
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc6/patches/28-Optional-Beeping-During-Resume-From-RAM.patch
> 

I can confirm that (Nigel's patch) works on i386; but it causes a warning:

kernel/power/main.c: In function 's2ram_beep_show':
kernel/power/main.c:319: warning: format '%d' expects type 'int', but argument 3 has type 'long unsigned int'


Here's the version that uses just one variable, relative to Nigel's
patch. Hmm, and it also closes nasty trap for the user in
acpi_sleep_setup; order of parameters actually mattered there,
acpi_sleep=s3_bios,s3_mode doing something different from
acpi_sleep=s3_mode,s3_bios .  It actually works here.

Will you fold it into  patch28, or should I create a changelog?
Testing welcome :-).
								Pavel

diff --git a/arch/i386/kernel/acpi/sleep.c b/arch/i386/kernel/acpi/sleep.c
index 4ee8357..5b67866 100644
--- a/arch/i386/kernel/acpi/sleep.c
+++ b/arch/i386/kernel/acpi/sleep.c
@@ -14,7 +14,7 @@ #include <asm/smp.h>
 
 /* address in low memory of the wakeup routine. */
 unsigned long acpi_wakeup_address = 0;
-unsigned long acpi_video_flags;
+unsigned long acpi_realmode_flags;
 extern char wakeup_start, wakeup_end;
 
 extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long));
@@ -68,9 +68,11 @@ static int __init acpi_sleep_setup(char 
 {
 	while ((str != NULL) && (*str != '\0')) {
 		if (strncmp(str, "s3_bios", 7) == 0)
-			acpi_video_flags = 1;
+			acpi_realmode_flags |= 1;
 		if (strncmp(str, "s3_mode", 7) == 0)
-			acpi_video_flags |= 2;
+			acpi_realmode_flags |= 2;
+		if (strncmp(str, "s3_beep", 7) == 0)
+			acpi_realmode_flags |= 4;
 		str = strchr(str, ',');
 		if (str != NULL)
 			str += strspn(str, ", \t");
@@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
 
 __setup("acpi_sleep=", acpi_sleep_setup);
 
+/* Ouch, we want to delete this. We already have better version in userspace, in 
+   s2ram from suspend.sf.net project */
 static __init int reset_videomode_after_s3(struct dmi_system_id *d)
 {
-	acpi_video_flags |= 2;
+	acpi_realmode_flags |= 2;
 	return 0;
 }
 
diff --git a/arch/i386/kernel/acpi/wakeup.S b/arch/i386/kernel/acpi/wakeup.S
index 68e58f1..443fe85 100644
--- a/arch/i386/kernel/acpi/wakeup.S
+++ b/arch/i386/kernel/acpi/wakeup.S
@@ -50,7 +50,7 @@ #	BEEP
 	movw	%ax, %ds					# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
 
-	testl   $1, beep_flags - wakeup_code
+	testl   $4, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
@@ -64,7 +64,7 @@ #	BEEP
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
-	testl	$1, video_flags - wakeup_code
+	testl	$1, realmode_flags - wakeup_code
 	jz	1f
 	lcall   $0xc000,$3
 	movw	%cs, %ax
@@ -72,7 +72,7 @@ #	BEEP
 	movw	%ax, %ss
 1:
 
-	testl	$2, video_flags - wakeup_code
+	testl	$2, realmode_flags - wakeup_code
 	jz	1f
 	mov	video_mode - wakeup_code, %ax
 	call	mode_set
@@ -111,11 +111,11 @@ #	BEEP
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
-	testl   $2, beep_flags - wakeup_code
+	testl   $8, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
-	ljmpl	$__KERNEL_CS,$wakeup_pmode_return
+	ljmpl	$__KERNEL_CS, $wakeup_pmode_return
 
 real_save_gdt:	.word 0
 		.long 0
@@ -124,7 +124,7 @@ real_save_cr3:	.long 0
 real_save_cr4:	.long 0
 real_magic:	.long 0
 video_mode:	.long 0
-video_flags:	.long 0
+realmode_flags:	.long 0
 beep_flags:	.long 0
 real_efer_save_restore:	.long 0
 real_save_efer_edx: 	.long 0
@@ -288,10 +288,8 @@ ENTRY(acpi_copy_wakeup_routine)
 
 	movl	saved_videomode, %edx
 	movl	%edx, video_mode - wakeup_start (%eax)
-	movl	acpi_video_flags, %edx
-	movl	%edx, video_flags - wakeup_start (%eax)
-	movl	s2ram_beep, %edx
-	movl	%edx, beep_flags - wakeup_start (%eax)
+	movl	acpi_realmode_flags, %edx
+	movl	%edx, realmode_flags - wakeup_start (%eax)
 	movl	$0x12345678, real_magic - wakeup_start (%eax)
 	movl	$0x12345678, saved_magic
 	popl	%ebx
diff --git a/arch/x86_64/kernel/acpi/sleep.c b/arch/x86_64/kernel/acpi/sleep.c
index 195b703..4277f2b 100644
--- a/arch/x86_64/kernel/acpi/sleep.c
+++ b/arch/x86_64/kernel/acpi/sleep.c
@@ -55,7 +55,7 @@ #ifdef CONFIG_ACPI_SLEEP
 
 /* address in low memory of the wakeup routine. */
 unsigned long acpi_wakeup_address = 0;
-unsigned long acpi_video_flags;
+unsigned long acpi_realmode_flags;
 extern char wakeup_start, wakeup_end;
 
 extern unsigned long acpi_copy_wakeup_routine(unsigned long);
@@ -103,9 +103,11 @@ static int __init acpi_sleep_setup(char 
 {
 	while ((str != NULL) && (*str != '\0')) {
 		if (strncmp(str, "s3_bios", 7) == 0)
-			acpi_video_flags = 1;
+			acpi_realmode_flags |= 1;
 		if (strncmp(str, "s3_mode", 7) == 0)
-			acpi_video_flags |= 2;
+			acpi_realmode_flags |= 2;
+		if (strncmp(str, "s3_beep", 7) == 0)
+			acpi_realmode_flags |= 4;
 		str = strchr(str, ',');
 		if (str != NULL)
 			str += strspn(str, ", \t");
diff --git a/arch/x86_64/kernel/acpi/wakeup.S b/arch/x86_64/kernel/acpi/wakeup.S
index ed63d18..13f1480 100644
--- a/arch/x86_64/kernel/acpi/wakeup.S
+++ b/arch/x86_64/kernel/acpi/wakeup.S
@@ -50,7 +50,7 @@ # Running in *copy* of this code, somewh
 	movw	%ax, %ss
 
 	# Data segment must be set up before we can see whether to beep.
-	testl   $1, beep_flags - wakeup_code
+	testl   $4, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
@@ -70,7 +70,7 @@ # Running in *copy* of this code, somewh
 	testl	%eax, %eax
 	jnz	no_longmode
 
-	testl	$1, video_flags - wakeup_code
+	testl	$1, realmode_flags - wakeup_code
 	jz	1f
 	lcall   $0xc000,$3
 	movw	%cs, %ax
@@ -78,7 +78,7 @@ # Running in *copy* of this code, somewh
 	movw	%ax, %ss
 1:
 
-	testl	$2, video_flags - wakeup_code
+	testl	$2, realmode_flags - wakeup_code
 	jz	1f
 	mov	video_mode - wakeup_code, %ax
 	call	mode_seta
@@ -251,9 +251,8 @@ gdt_48a:
 	.long   gdta - wakeup_code              # gdt base (relocated in later)
 	
 real_magic:	.quad 0
-beep_flags:	.quad 0
 video_mode:	.quad 0
-video_flags:	.quad 0
+realmode_flags:	.quad 0
 
 .code16
 bogus_real_magic:
@@ -367,12 +366,10 @@ ENTRY(acpi_copy_wakeup_routine)
 	pushq	%rax
 	pushq	%rdx
 
-	movl	s2ram_beep, %edx
-	movl	%edx, beep_flags - wakeup_start (,%rdi)
 	movl	saved_video_mode, %edx
 	movl	%edx, video_mode - wakeup_start (,%rdi)
-	movl	acpi_video_flags, %edx
-	movl	%edx, video_flags - wakeup_start (,%rdi)
+	movl	acpi_realmode_flags, %edx
+	movl	%edx, realmode_flags - wakeup_start (,%rdi)
 	movq	$0x12345678, real_magic - wakeup_start (,%rdi)
 	movq	$0x123456789abcdef0, %rdx
 	movq	%rdx, saved_magic
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f26df82..9287d89 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -122,8 +122,7 @@ extern struct acpi_mcfg_allocation *pci_
 extern int pci_mmcfg_config_num;
 
 extern int sbf_port;
-extern unsigned long acpi_video_flags;
-extern unsigned long s2ram_beep;
+extern unsigned long acpi_realmode_flags;
 
 #else	/* !CONFIG_ACPI */
 
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 516c24e..fc45ed2 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -312,27 +312,6 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
-unsigned long s2ram_beep = 0;
-
-static ssize_t s2ram_beep_show(struct kset *kset, char *buf)
-{
-	return sprintf(buf, "%d\n", s2ram_beep);
-}
-
-static ssize_t
-s2ram_beep_store(struct kset *kset, const char *buf, size_t n)
-{
-	int val;
-
-	if (sscanf(buf, "%d", &val) > 0) {
-		s2ram_beep = val;
-		return n;
-	}
-	return -EINVAL;
-}
-
-power_attr(s2ram_beep);
-
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -358,13 +337,11 @@ power_attr(pm_trace);
 static struct attribute * g[] = {
 	&state_attr.attr,
 	&pm_trace_attr.attr,
-	&s2ram_beep_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
-	&s2ram_beep_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 30ee462..40d616b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -569,7 +569,7 @@ #ifdef CONFIG_ACPI_SLEEP
 	{
 		.ctl_name	= KERN_ACPI_VIDEO_FLAGS,
 		.procname	= "acpi_video_flags",
-		.data		= &acpi_video_flags,
+		.data		= &acpi_realmode_flags,
 		.maxlen		= sizeof (unsigned long),
 		.mode		= 0644,
 		.proc_handler	= &proc_doulongvec_minmax,


(s2ram patch for testing follows)

Index: s2ram.c
===================================================================
RCS file: /cvsroot/suspend/suspend/s2ram.c,v
retrieving revision 1.46
diff -u -u -r1.46 s2ram.c
--- s2ram.c	10 Jan 2007 08:04:55 -0000	1.46
+++ s2ram.c	4 Jul 2007 21:28:22 -0000
@@ -35,13 +35,14 @@
 /* flags for the whitelist */
 #define S3_BIOS     0x01	/* machine needs acpi_sleep=s3_bios */
 #define S3_MODE     0x02	/* machine needs acpi_sleep=s3_mode */
-#define VBE_SAVE    0x04	/* machine needs "vbetool save / restore" */
-#define VBE_POST    0x08	/* machine needs "vbetool post" */
-#define RADEON_OFF  0x10	/* machine needs "radeontool light off" */
-#define UNSURE      0x20	/* unverified entries from acpi-support 0.59 */
-#define NOFB        0x40	/* must not use a frame buffer */
-#define VBE_MODE    0x80	/* machine needs "vbetool vbemode get / set" */
-#define PCI_SAVE   0x100	/* we need to save the VGA PCI registers */
+#define S3_BEEP     0x04	/* debugging beep */
+#define VBE_SAVE    0x08	/* machine needs "vbetool save / restore" */
+#define VBE_POST    0x10	/* machine needs "vbetool post" */
+#define RADEON_OFF  0x20	/* machine needs "radeontool light off" */
+#define UNSURE      0x40	/* unverified entries from acpi-support 0.59 */
+#define NOFB        0x80	/* must not use a frame buffer */
+#define VBE_MODE    0x100	/* machine needs "vbetool vbemode get / set" */
+#define PCI_SAVE   0x200	/* we need to save the VGA PCI registers */
 
 #include "whitelist.c"
 
@@ -188,8 +189,8 @@
 	int ret = 0;
 
 	/* 0 means: don't touch what was set on kernel commandline */
-	if (flags & (S3_BIOS | S3_MODE))
-		ret = set_acpi_video_mode(flags & (S3_BIOS | S3_MODE));
+	if (flags & (S3_BIOS | S3_MODE | S3_BEEP))
+		ret = set_acpi_video_mode(flags & (S3_BIOS | S3_MODE | S3_BEEP));
 
 	if (ret)
 		return ret;
@@ -368,7 +369,7 @@
 			flags |= RADEON_OFF;
 			break;
 		case 'a':
-			flags |= (atoi(optarg) & (S3_BIOS | S3_MODE));
+			flags |= (atoi(optarg) & (S3_BIOS | S3_MODE | S3_BEEP));
 			break;
 		case 'v':
 			flags |= PCI_SAVE;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

Hi,

On Wednesday, 4 July 2007 23:29, Pavel Machek wrote:
> Hi!
> 
> > > > > > Sorry, but I can't resist the opportunity to say "Send a patch!" :)
> > > > > > 
> > > > > > Seriously, though, I'd prefer not to. If we rename that acpi video flags 
> > > > > > variable (I assume this is what you're thinking of), we only create cause for 
> > > > > > confusion. A variable should for debugging or for controlling quirks, not for 
> > > > > > both at the same time.
> > > > > 
> > > > > Cause for confusion? We are currently using 2 bits of that variable,
> > > > > and we want to add one more bit. I seriously doubt that can confuse
> > > > > anyone.
> > > > 
> > > > Well, indeed it would be more elegant to rename the existing flags variable
> > > > and use another bit out of it, but I personally don't think it's _that_
> > > > important.  At least, I don't think we should block the patch
> > > > because of that.
> > > 
> > > It is not _that_ important.
> > > 
> > > > BTW, has anyone confirmed that it works on i386?
> > > 
> > > If you have patch somewhere nearby, I can test it on i386 and make it
> > > use just one flags variable.
> > 
> > http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc6/patches/28-Optional-Beeping-During-Resume-From-RAM.patch
> > 
> 
> I can confirm that (Nigel's patch) works on i386; but it causes a warning:
> 
> kernel/power/main.c: In function 's2ram_beep_show':
> kernel/power/main.c:319: warning: format '%d' expects type 'int', but argument 3 has type 'long unsigned int'

Yes, I've fixed the warning in the 2.6.22-rc7 series, but that's not a big
deal.

> Here's the version that uses just one variable, relative to Nigel's
> patch. Hmm, and it also closes nasty trap for the user in
> acpi_sleep_setup; order of parameters actually mattered there,
> acpi_sleep=s3_bios,s3_mode doing something different from
> acpi_sleep=s3_mode,s3_bios .  It actually works here.
> 
> Will you fold it into  patch28, or should I create a changelog?

Hmm, I think we should keep the record straight.  Please add a changelog
if that's not a problem.

> Testing welcome :-).
> 								Pavel
> 
> diff --git a/arch/i386/kernel/acpi/sleep.c b/arch/i386/kernel/acpi/sleep.c
> index 4ee8357..5b67866 100644
> --- a/arch/i386/kernel/acpi/sleep.c
> +++ b/arch/i386/kernel/acpi/sleep.c
> @@ -14,7 +14,7 @@ #include <asm/smp.h>
>  
>  /* address in low memory of the wakeup routine. */
>  unsigned long acpi_wakeup_address = 0;
> -unsigned long acpi_video_flags;
> +unsigned long acpi_realmode_flags;
>  extern char wakeup_start, wakeup_end;
>  
>  extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long));
> @@ -68,9 +68,11 @@ static int __init acpi_sleep_setup(char 
>  {
>  	while ((str != NULL) && (*str != '\0')) {
>  		if (strncmp(str, "s3_bios", 7) == 0)
> -			acpi_video_flags = 1;
> +			acpi_realmode_flags |= 1;
>  		if (strncmp(str, "s3_mode", 7) == 0)
> -			acpi_video_flags |= 2;
> +			acpi_realmode_flags |= 2;
> +		if (strncmp(str, "s3_beep", 7) == 0)
> +			acpi_realmode_flags |= 4;
>  		str = strchr(str, ',');
>  		if (str != NULL)
>  			str += strspn(str, ", \t");
> @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
>  
>  __setup("acpi_sleep=", acpi_sleep_setup);
>  
> +/* Ouch, we want to delete this. We already have better version in userspace, in 
> +   s2ram from suspend.sf.net project */
>  static __init int reset_videomode_after_s3(struct dmi_system_id *d)
>  {
> -	acpi_video_flags |= 2;
> +	acpi_realmode_flags |= 2;
>  	return 0;
>  }
>  
> diff --git a/arch/i386/kernel/acpi/wakeup.S b/arch/i386/kernel/acpi/wakeup.S
> index 68e58f1..443fe85 100644
> --- a/arch/i386/kernel/acpi/wakeup.S
> +++ b/arch/i386/kernel/acpi/wakeup.S
> @@ -50,7 +50,7 @@ #	BEEP
>  	movw	%ax, %ds					# Make ds:0 point to wakeup_start
>  	movw	%ax, %ss
>  
> -	testl   $1, beep_flags - wakeup_code
> +	testl   $4, realmode_flags - wakeup_code
>  	jz      1f
>  	BEEP
>  1:
> @@ -64,7 +64,7 @@ #	BEEP
>  	cmpl	$0x12345678, %eax
>  	jne	bogus_real_magic
>  
> -	testl	$1, video_flags - wakeup_code
> +	testl	$1, realmode_flags - wakeup_code
>  	jz	1f
>  	lcall   $0xc000,$3
>  	movw	%cs, %ax
> @@ -72,7 +72,7 @@ #	BEEP
>  	movw	%ax, %ss
>  1:
>  
> -	testl	$2, video_flags - wakeup_code
> +	testl	$2, realmode_flags - wakeup_code
>  	jz	1f
>  	mov	video_mode - wakeup_code, %ax
>  	call	mode_set
> @@ -111,11 +111,11 @@ #	BEEP
>  	cmpl	$0x12345678, %eax
>  	jne	bogus_real_magic
>  
> -	testl   $2, beep_flags - wakeup_code
> +	testl   $8, realmode_flags - wakeup_code
>  	jz      1f
>  	BEEP
>  1:
> -	ljmpl	$__KERNEL_CS,$wakeup_pmode_return
> +	ljmpl	$__KERNEL_CS, $wakeup_pmode_return
>  
>  real_save_gdt:	.word 0
>  		.long 0
> @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
>  real_save_cr4:	.long 0
>  real_magic:	.long 0
>  video_mode:	.long 0
> -video_flags:	.long 0
> +realmode_flags:	.long 0
>  beep_flags:	.long 0
>  real_efer_save_restore:	.long 0
>  real_save_efer_edx: 	.long 0
> @@ -288,10 +288,8 @@ ENTRY(acpi_copy_wakeup_routine)
>  
>  	movl	saved_videomode, %edx
>  	movl	%edx, video_mode - wakeup_start (%eax)
> -	movl	acpi_video_flags, %edx
> -	movl	%edx, video_flags - wakeup_start (%eax)
> -	movl	s2ram_beep, %edx
> -	movl	%edx, beep_flags - wakeup_start (%eax)
> +	movl	acpi_realmode_flags, %edx
> +	movl	%edx, realmode_flags - wakeup_start (%eax)
>  	movl	$0x12345678, real_magic - wakeup_start (%eax)
>  	movl	$0x12345678, saved_magic
>  	popl	%ebx
> diff --git a/arch/x86_64/kernel/acpi/sleep.c b/arch/x86_64/kernel/acpi/sleep.c
> index 195b703..4277f2b 100644
> --- a/arch/x86_64/kernel/acpi/sleep.c
> +++ b/arch/x86_64/kernel/acpi/sleep.c
> @@ -55,7 +55,7 @@ #ifdef CONFIG_ACPI_SLEEP
>  
>  /* address in low memory of the wakeup routine. */
>  unsigned long acpi_wakeup_address = 0;
> -unsigned long acpi_video_flags;
> +unsigned long acpi_realmode_flags;
>  extern char wakeup_start, wakeup_end;
>  
>  extern unsigned long acpi_copy_wakeup_routine(unsigned long);
> @@ -103,9 +103,11 @@ static int __init acpi_sleep_setup(char 
>  {
>  	while ((str != NULL) && (*str != '\0')) {
>  		if (strncmp(str, "s3_bios", 7) == 0)
> -			acpi_video_flags = 1;
> +			acpi_realmode_flags |= 1;
>  		if (strncmp(str, "s3_mode", 7) == 0)
> -			acpi_video_flags |= 2;
> +			acpi_realmode_flags |= 2;
> +		if (strncmp(str, "s3_beep", 7) == 0)
> +			acpi_realmode_flags |= 4;
>  		str = strchr(str, ',');
>  		if (str != NULL)
>  			str += strspn(str, ", \t");
> diff --git a/arch/x86_64/kernel/acpi/wakeup.S b/arch/x86_64/kernel/acpi/wakeup.S
> index ed63d18..13f1480 100644
> --- a/arch/x86_64/kernel/acpi/wakeup.S
> +++ b/arch/x86_64/kernel/acpi/wakeup.S
> @@ -50,7 +50,7 @@ # Running in *copy* of this code, somewh
>  	movw	%ax, %ss
>  
>  	# Data segment must be set up before we can see whether to beep.
> -	testl   $1, beep_flags - wakeup_code
> +	testl   $4, realmode_flags - wakeup_code
>  	jz      1f
>  	BEEP
>  1:
> @@ -70,7 +70,7 @@ # Running in *copy* of this code, somewh
>  	testl	%eax, %eax
>  	jnz	no_longmode
>  
> -	testl	$1, video_flags - wakeup_code
> +	testl	$1, realmode_flags - wakeup_code
>  	jz	1f
>  	lcall   $0xc000,$3
>  	movw	%cs, %ax
> @@ -78,7 +78,7 @@ # Running in *copy* of this code, somewh
>  	movw	%ax, %ss
>  1:
>  
> -	testl	$2, video_flags - wakeup_code
> +	testl	$2, realmode_flags - wakeup_code
>  	jz	1f
>  	mov	video_mode - wakeup_code, %ax
>  	call	mode_seta
> @@ -251,9 +251,8 @@ gdt_48a:
>  	.long   gdta - wakeup_code              # gdt base (relocated in later)
>  	
>  real_magic:	.quad 0
> -beep_flags:	.quad 0
>  video_mode:	.quad 0
> -video_flags:	.quad 0
> +realmode_flags:	.quad 0
>  
>  .code16
>  bogus_real_magic:
> @@ -367,12 +366,10 @@ ENTRY(acpi_copy_wakeup_routine)
>  	pushq	%rax
>  	pushq	%rdx
>  
> -	movl	s2ram_beep, %edx
> -	movl	%edx, beep_flags - wakeup_start (,%rdi)
>  	movl	saved_video_mode, %edx
>  	movl	%edx, video_mode - wakeup_start (,%rdi)
> -	movl	acpi_video_flags, %edx
> -	movl	%edx, video_flags - wakeup_start (,%rdi)
> +	movl	acpi_realmode_flags, %edx
> +	movl	%edx, realmode_flags - wakeup_start (,%rdi)
>  	movq	$0x12345678, real_magic - wakeup_start (,%rdi)
>  	movq	$0x123456789abcdef0, %rdx
>  	movq	%rdx, saved_magic
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index f26df82..9287d89 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -122,8 +122,7 @@ extern struct acpi_mcfg_allocation *pci_
>  extern int pci_mmcfg_config_num;
>  
>  extern int sbf_port;
> -extern unsigned long acpi_video_flags;
> -extern unsigned long s2ram_beep;
> +extern unsigned long acpi_realmode_flags;
>  
>  #else	/* !CONFIG_ACPI */
>  
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 516c24e..fc45ed2 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -312,27 +312,6 @@ static ssize_t state_store(struct kset *
>  
>  power_attr(state);
>  
> -unsigned long s2ram_beep = 0;
> -
> -static ssize_t s2ram_beep_show(struct kset *kset, char *buf)
> -{
> -	return sprintf(buf, "%d\n", s2ram_beep);
> -}
> -
> -static ssize_t
> -s2ram_beep_store(struct kset *kset, const char *buf, size_t n)
> -{
> -	int val;
> -
> -	if (sscanf(buf, "%d", &val) > 0) {
> -		s2ram_beep = val;
> -		return n;
> -	}
> -	return -EINVAL;
> -}
> -
> -power_attr(s2ram_beep);
> -
>  #ifdef CONFIG_PM_TRACE
>  int pm_trace_enabled;
>  
> @@ -358,13 +337,11 @@ power_attr(pm_trace);
>  static struct attribute * g[] = {
>  	&state_attr.attr,
>  	&pm_trace_attr.attr,
> -	&s2ram_beep_attr.attr,
>  	NULL,
>  };
>  #else
>  static struct attribute * g[] = {
>  	&state_attr.attr,
> -	&s2ram_beep_attr.attr,
>  	NULL,
>  };
>  #endif /* CONFIG_PM_TRACE */
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 30ee462..40d616b 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -569,7 +569,7 @@ #ifdef CONFIG_ACPI_SLEEP
>  	{
>  		.ctl_name	= KERN_ACPI_VIDEO_FLAGS,
>  		.procname	= "acpi_video_flags",
> -		.data		= &acpi_video_flags,
> +		.data		= &acpi_realmode_flags,
>  		.maxlen		= sizeof (unsigned long),
>  		.mode		= 0644,
>  		.proc_handler	= &proc_doulongvec_minmax,
> 
> 
> (s2ram patch for testing follows)
> 
> Index: s2ram.c
> ===================================================================
> RCS file: /cvsroot/suspend/suspend/s2ram.c,v
> retrieving revision 1.46
> diff -u -u -r1.46 s2ram.c
> --- s2ram.c	10 Jan 2007 08:04:55 -0000	1.46
> +++ s2ram.c	4 Jul 2007 21:28:22 -0000
> @@ -35,13 +35,14 @@
>  /* flags for the whitelist */
>  #define S3_BIOS     0x01	/* machine needs acpi_sleep=s3_bios */
>  #define S3_MODE     0x02	/* machine needs acpi_sleep=s3_mode */
> -#define VBE_SAVE    0x04	/* machine needs "vbetool save / restore" */
> -#define VBE_POST    0x08	/* machine needs "vbetool post" */
> -#define RADEON_OFF  0x10	/* machine needs "radeontool light off" */
> -#define UNSURE      0x20	/* unverified entries from acpi-support 0.59 */
> -#define NOFB        0x40	/* must not use a frame buffer */
> -#define VBE_MODE    0x80	/* machine needs "vbetool vbemode get / set" */
> -#define PCI_SAVE   0x100	/* we need to save the VGA PCI registers */
> +#define S3_BEEP     0x04	/* debugging beep */
> +#define VBE_SAVE    0x08	/* machine needs "vbetool save / restore" */
> +#define VBE_POST    0x10	/* machine needs "vbetool post" */
> +#define RADEON_OFF  0x20	/* machine needs "radeontool light off" */
> +#define UNSURE      0x40	/* unverified entries from acpi-support 0.59 */
> +#define NOFB        0x80	/* must not use a frame buffer */
> +#define VBE_MODE    0x100	/* machine needs "vbetool vbemode get / set" */
> +#define PCI_SAVE   0x200	/* we need to save the VGA PCI registers */
>  
>  #include "whitelist.c"
>  
> @@ -188,8 +189,8 @@
>  	int ret = 0;
>  
>  	/* 0 means: don't touch what was set on kernel commandline */
> -	if (flags & (S3_BIOS | S3_MODE))
> -		ret = set_acpi_video_mode(flags & (S3_BIOS | S3_MODE));
> +	if (flags & (S3_BIOS | S3_MODE | S3_BEEP))
> +		ret = set_acpi_video_mode(flags & (S3_BIOS | S3_MODE | S3_BEEP));
>  
>  	if (ret)
>  		return ret;
> @@ -368,7 +369,7 @@
>  			flags |= RADEON_OFF;
>  			break;
>  		case 'a':
> -			flags |= (atoi(optarg) & (S3_BIOS | S3_MODE));
> +			flags |= (atoi(optarg) & (S3_BIOS | S3_MODE | S3_BEEP));
>  			break;
>  		case 'v':
>  			flags |= PCI_SAVE;
> 

-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

Hi.

On Thursday 05 July 2007 07:29:07 Pavel Machek wrote:
> Here's the version that uses just one variable, relative to Nigel's
> patch. Hmm, and it also closes nasty trap for the user in
> acpi_sleep_setup; order of parameters actually mattered there,
> acpi_sleep=s3_bios,s3_mode doing something different from
> acpi_sleep=s3_mode,s3_bios .  It actually works here.

Good catch.

> Will you fold it into  patch28, or should I create a changelog?
> Testing welcome :-).

I really dislike the overloading of acpi video flags in this way. You yourself 
have said in the past that new functionality should use /sys instead 
of /proc, and here you are overloading an existing variable for the sake of 
expediency. Please, stick with a new /sys/power entry.

Documentation is also an issue. Your patch should update the kernel_parameters 
file so users can know how to get the beeping to happen. It would be nice if 
it mentioned the proc entry too.

[...]

> @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
>  
>  __setup("acpi_sleep=", acpi_sleep_setup);
>  
> +/* Ouch, we want to delete this. We already have better version in 
userspace, in 
> +   s2ram from suspend.sf.net project */

Do we? This version has advantages in not requiring any userspace app and in 
being able to work even if you can't yet get as far as having userspace 
working. [Reads more] Oh, do you mean a better way of setting this parameter 
(ie requiring the userspace app and then having it echo $VALUE 
> /proc/sys/kernel/acpi_video_flags? Considering we're not going to suspend 
to ram during init scripts, we have a way of setting/resetting it post-boot 
with or without s2m and the flag doesn't therefore need to be command line 
option, I can go with that. Does it need a patch to mark it as deprecated?

[...]

> @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
>  real_save_cr4:	.long 0
>  real_magic:	.long 0
>  video_mode:	.long 0
> -video_flags:	.long 0
> +realmode_flags:	.long 0
>  beep_flags:	.long 0
>  real_efer_save_restore:	.long 0
>  real_save_efer_edx: 	.long 0

Beep_flags should be removed too if you're sticking with /proc.

Regards,

Nigel
-- 
See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > Here's the version that uses just one variable, relative to Nigel's
> > patch. Hmm, and it also closes nasty trap for the user in
> > acpi_sleep_setup; order of parameters actually mattered there,
> > acpi_sleep=s3_bios,s3_mode doing something different from
> > acpi_sleep=s3_mode,s3_bios .  It actually works here.
> > 
> > Will you fold it into  patch28, or should I create a changelog?
> 
> Hmm, I think we should keep the record straight.  Please add a changelog
> if that's not a problem.

(I also added documentation and removed the superfluous beep_flags).

--- 

Move "debug during resume from s2ram" into the variable we already use
for real-mode flags to simplify code. It also closes nasty trap for
the user in acpi_sleep_setup; order of parameters actually mattered there,
acpi_sleep=s3_bios,s3_mode doing something different from
acpi_sleep=s3_mode,s3_bios.

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/arch/i386/kernel/acpi/sleep.c b/arch/i386/kernel/acpi/sleep.c
index 4ee8357..5b67866 100644
--- a/arch/i386/kernel/acpi/sleep.c
+++ b/arch/i386/kernel/acpi/sleep.c
@@ -14,7 +14,7 @@ #include <asm/smp.h>
 
 /* address in low memory of the wakeup routine. */
 unsigned long acpi_wakeup_address = 0;
-unsigned long acpi_video_flags;
+unsigned long acpi_realmode_flags;
 extern char wakeup_start, wakeup_end;
 
 extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long));
@@ -68,9 +68,11 @@ static int __init acpi_sleep_setup(char 
 {
 	while ((str != NULL) && (*str != '\0')) {
 		if (strncmp(str, "s3_bios", 7) == 0)
-			acpi_video_flags = 1;
+			acpi_realmode_flags |= 1;
 		if (strncmp(str, "s3_mode", 7) == 0)
-			acpi_video_flags |= 2;
+			acpi_realmode_flags |= 2;
+		if (strncmp(str, "s3_beep", 7) == 0)
+			acpi_realmode_flags |= 4;
 		str = strchr(str, ',');
 		if (str != NULL)
 			str += strspn(str, ", \t");
@@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
 
 __setup("acpi_sleep=", acpi_sleep_setup);
 
+/* Ouch, we want to delete this. We already have better version in userspace, in 
+   s2ram from suspend.sf.net project */
 static __init int reset_videomode_after_s3(struct dmi_system_id *d)
 {
-	acpi_video_flags |= 2;
+	acpi_realmode_flags |= 2;
 	return 0;
 }
 
diff --git a/arch/i386/kernel/acpi/wakeup.S b/arch/i386/kernel/acpi/wakeup.S
index 68e58f1..443fe85 100644
--- a/arch/i386/kernel/acpi/wakeup.S
+++ b/arch/i386/kernel/acpi/wakeup.S
@@ -50,7 +50,7 @@ #	BEEP
 	movw	%ax, %ds					# Make ds:0 point to wakeup_start
 	movw	%ax, %ss
 
-	testl   $1, beep_flags - wakeup_code
+	testl   $4, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
@@ -64,7 +64,7 @@ #	BEEP
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
-	testl	$1, video_flags - wakeup_code
+	testl	$1, realmode_flags - wakeup_code
 	jz	1f
 	lcall   $0xc000,$3
 	movw	%cs, %ax
@@ -72,7 +72,7 @@ #	BEEP
 	movw	%ax, %ss
 1:
 
-	testl	$2, video_flags - wakeup_code
+	testl	$2, realmode_flags - wakeup_code
 	jz	1f
 	mov	video_mode - wakeup_code, %ax
 	call	mode_set
@@ -111,11 +111,11 @@ #	BEEP
 	cmpl	$0x12345678, %eax
 	jne	bogus_real_magic
 
-	testl   $2, beep_flags - wakeup_code
+	testl   $8, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
-	ljmpl	$__KERNEL_CS,$wakeup_pmode_return
+	ljmpl	$__KERNEL_CS, $wakeup_pmode_return
 
 real_save_gdt:	.word 0
 		.long 0
@@ -124,7 +124,7 @@ real_save_cr3:	.long 0
 real_save_cr4:	.long 0
 real_magic:	.long 0
 video_mode:	.long 0
-video_flags:	.long 0
+realmode_flags:	.long 0
 beep_flags:	.long 0
 real_efer_save_restore:	.long 0
 real_save_efer_edx: 	.long 0
@@ -288,10 +288,8 @@ ENTRY(acpi_copy_wakeup_routine)
 
 	movl	saved_videomode, %edx
 	movl	%edx, video_mode - wakeup_start (%eax)
-	movl	acpi_video_flags, %edx
-	movl	%edx, video_flags - wakeup_start (%eax)
-	movl	s2ram_beep, %edx
-	movl	%edx, beep_flags - wakeup_start (%eax)
+	movl	acpi_realmode_flags, %edx
+	movl	%edx, realmode_flags - wakeup_start (%eax)
 	movl	$0x12345678, real_magic - wakeup_start (%eax)
 	movl	$0x12345678, saved_magic
 	popl	%ebx
diff --git a/arch/x86_64/kernel/acpi/sleep.c b/arch/x86_64/kernel/acpi/sleep.c
index 195b703..4277f2b 100644
--- a/arch/x86_64/kernel/acpi/sleep.c
+++ b/arch/x86_64/kernel/acpi/sleep.c
@@ -55,7 +55,7 @@ #ifdef CONFIG_ACPI_SLEEP
 
 /* address in low memory of the wakeup routine. */
 unsigned long acpi_wakeup_address = 0;
-unsigned long acpi_video_flags;
+unsigned long acpi_realmode_flags;
 extern char wakeup_start, wakeup_end;
 
 extern unsigned long acpi_copy_wakeup_routine(unsigned long);
@@ -103,9 +103,11 @@ static int __init acpi_sleep_setup(char 
 {
 	while ((str != NULL) && (*str != '\0')) {
 		if (strncmp(str, "s3_bios", 7) == 0)
-			acpi_video_flags = 1;
+			acpi_realmode_flags |= 1;
 		if (strncmp(str, "s3_mode", 7) == 0)
-			acpi_video_flags |= 2;
+			acpi_realmode_flags |= 2;
+		if (strncmp(str, "s3_beep", 7) == 0)
+			acpi_realmode_flags |= 4;
 		str = strchr(str, ',');
 		if (str != NULL)
 			str += strspn(str, ", \t");
diff --git a/arch/x86_64/kernel/acpi/wakeup.S b/arch/x86_64/kernel/acpi/wakeup.S
index ed63d18..13f1480 100644
--- a/arch/x86_64/kernel/acpi/wakeup.S
+++ b/arch/x86_64/kernel/acpi/wakeup.S
@@ -50,7 +50,7 @@ # Running in *copy* of this code, somewh
 	movw	%ax, %ss
 
 	# Data segment must be set up before we can see whether to beep.
-	testl   $1, beep_flags - wakeup_code
+	testl   $4, realmode_flags - wakeup_code
 	jz      1f
 	BEEP
 1:
@@ -70,7 +70,7 @@ # Running in *copy* of this code, somewh
 	testl	%eax, %eax
 	jnz	no_longmode
 
-	testl	$1, video_flags - wakeup_code
+	testl	$1, realmode_flags - wakeup_code
 	jz	1f
 	lcall   $0xc000,$3
 	movw	%cs, %ax
@@ -78,7 +78,7 @@ # Running in *copy* of this code, somewh
 	movw	%ax, %ss
 1:
 
-	testl	$2, video_flags - wakeup_code
+	testl	$2, realmode_flags - wakeup_code
 	jz	1f
 	mov	video_mode - wakeup_code, %ax
 	call	mode_seta
@@ -251,9 +251,8 @@ gdt_48a:
 	.long   gdta - wakeup_code              # gdt base (relocated in later)
 	
 real_magic:	.quad 0
-beep_flags:	.quad 0
 video_mode:	.quad 0
-video_flags:	.quad 0
+realmode_flags:	.quad 0
 
 .code16
 bogus_real_magic:
@@ -367,12 +366,10 @@ ENTRY(acpi_copy_wakeup_routine)
 	pushq	%rax
 	pushq	%rdx
 
-	movl	s2ram_beep, %edx
-	movl	%edx, beep_flags - wakeup_start (,%rdi)
 	movl	saved_video_mode, %edx
 	movl	%edx, video_mode - wakeup_start (,%rdi)
-	movl	acpi_video_flags, %edx
-	movl	%edx, video_flags - wakeup_start (,%rdi)
+	movl	acpi_realmode_flags, %edx
+	movl	%edx, realmode_flags - wakeup_start (,%rdi)
 	movq	$0x12345678, real_magic - wakeup_start (,%rdi)
 	movq	$0x123456789abcdef0, %rdx
 	movq	%rdx, saved_magic
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f26df82..9287d89 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -122,8 +122,7 @@ extern struct acpi_mcfg_allocation *pci_
 extern int pci_mmcfg_config_num;
 
 extern int sbf_port;
-extern unsigned long acpi_video_flags;
-extern unsigned long s2ram_beep;
+extern unsigned long acpi_realmode_flags;
 
 #else	/* !CONFIG_ACPI */
 
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 516c24e..fc45ed2 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -312,27 +312,6 @@ static ssize_t state_store(struct kset *
 
 power_attr(state);
 
-unsigned long s2ram_beep = 0;
-
-static ssize_t s2ram_beep_show(struct kset *kset, char *buf)
-{
-	return sprintf(buf, "%d\n", s2ram_beep);
-}
-
-static ssize_t
-s2ram_beep_store(struct kset *kset, const char *buf, size_t n)
-{
-	int val;
-
-	if (sscanf(buf, "%d", &val) > 0) {
-		s2ram_beep = val;
-		return n;
-	}
-	return -EINVAL;
-}
-
-power_attr(s2ram_beep);
-
 #ifdef CONFIG_PM_TRACE
 int pm_trace_enabled;
 
@@ -358,13 +337,11 @@ power_attr(pm_trace);
 static struct attribute * g[] = {
 	&state_attr.attr,
 	&pm_trace_attr.attr,
-	&s2ram_beep_attr.attr,
 	NULL,
 };
 #else
 static struct attribute * g[] = {
 	&state_attr.attr,
-	&s2ram_beep_attr.attr,
 	NULL,
 };
 #endif /* CONFIG_PM_TRACE */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 30ee462..40d616b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -569,7 +569,7 @@ #ifdef CONFIG_ACPI_SLEEP
 	{
 		.ctl_name	= KERN_ACPI_VIDEO_FLAGS,
 		.procname	= "acpi_video_flags",
-		.data		= &acpi_video_flags,
+		.data		= &acpi_realmode_flags,
 		.maxlen		= sizeof (unsigned long),
 		.mode		= 0644,
 		.proc_handler	= &proc_doulongvec_minmax,



									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> Documentation is also an issue. Your patch should update the kernel_parameters 
> file so users can know how to get the beeping to happen. It would be nice if 
> it mentioned the proc entry too.

Fixed the docs.

> > @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
> >  
> >  __setup("acpi_sleep=", acpi_sleep_setup);
> >  
> > +/* Ouch, we want to delete this. We already have better version in 
> userspace, in 
> > +   s2ram from suspend.sf.net project */
> 
> Do we? This version has advantages in not requiring any userspace app and in 
> being able to work even if you can't yet get as far as having

Take a look at the file. It has whitelist with just one entry, too
bad.

> > @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
> >  real_save_cr4:	.long 0
> >  real_magic:	.long 0
> >  video_mode:	.long 0
> > -video_flags:	.long 0
> > +realmode_flags:	.long 0
> >  beep_flags:	.long 0
> >  real_efer_save_restore:	.long 0
> >  real_save_efer_edx: 	.long 0
> 
> Beep_flags should be removed too if you're sticking with /proc.

Fixed.
										Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

Hi.

On Thursday 05 July 2007 08:48:59 Pavel Machek wrote:
> Hi!
> 
> > Documentation is also an issue. Your patch should update the 
kernel_parameters 
> > file so users can know how to get the beeping to happen. It would be nice 
if 
> > it mentioned the proc entry too.
> 
> Fixed the docs.

Ta.
 
> > > @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
> > >  
> > >  __setup("acpi_sleep=", acpi_sleep_setup);
> > >  
> > > +/* Ouch, we want to delete this. We already have better version in 
> > userspace, in 
> > > +   s2ram from suspend.sf.net project */
> > 
> > Do we? This version has advantages in not requiring any userspace app and 
in 
> > being able to work even if you can't yet get as far as having
> 
> Take a look at the file. It has whitelist with just one entry, too
> bad.

The contents of the whitelist are irrelevant. My laptop needs this 
functionality, but I haven't bothered to send you a whitelist entry, in part 
because I don't use s2ram.

Regardless of that, if you had read the whole comment (you've deleted half of 
it), you would have noticed that I ended up changing my mind and instead 
saying "Why not just delete the __setup now, or at least put it in the 
deprecated file?"

> > > @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
> > >  real_save_cr4:	.long 0
> > >  real_magic:	.long 0
> > >  video_mode:	.long 0
> > > -video_flags:	.long 0
> > > +realmode_flags:	.long 0
> > >  beep_flags:	.long 0
> > >  real_efer_save_restore:	.long 0
> > >  real_save_efer_edx: 	.long 0
> > 
> > Beep_flags should be removed too if you're sticking with /proc.
> 
> Fixed.

Ta.  But you didn't answer the question - why /proc and not sysfs?

Regards,

Nigel
-- 
See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > > > @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
> > > >  
> > > >  __setup("acpi_sleep=", acpi_sleep_setup);
> > > >  
> > > > +/* Ouch, we want to delete this. We already have better version in 
> > > userspace, in 
> > > > +   s2ram from suspend.sf.net project */
> > > 
> > > Do we? This version has advantages in not requiring any userspace app and 
> in 
> > > being able to work even if you can't yet get as far as having
> > 
> > Take a look at the file. It has whitelist with just one entry, too
> > bad.
> 
> The contents of the whitelist are irrelevant. My laptop needs this 
> functionality, but I haven't bothered to send you a whitelist entry, in part 
> because I don't use s2ram.
> 
> Regardless of that, if you had read the whole comment (you've deleted half of 
> it), you would have noticed that I ended up changing my mind and instead 
> saying "Why not just delete the __setup now, or at least put it in the 
> deprecated file?"

That should be  certainly done in separate patch, right? It is on my
todolist somewhere now.

> > > > @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
> > > >  real_save_cr4:	.long 0
> > > >  real_magic:	.long 0
> > > >  video_mode:	.long 0
> > > > -video_flags:	.long 0
> > > > +realmode_flags:	.long 0
> > > >  beep_flags:	.long 0
> > > >  real_efer_save_restore:	.long 0
> > > >  real_save_efer_edx: 	.long 0
> > > 
> > > Beep_flags should be removed too if you're sticking with /proc.
> > 
> > Fixed.
> 
> Ta.  But you didn't answer the question - why /proc and not sysfs?

Do you seriously advocate setting two bits of one variable from /proc,
and one more bit from /sys?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

Hi.

On Thursday 05 July 2007 09:01:09 Pavel Machek wrote:
> Hi!
> 
> > > > > @@ -80,9 +82,11 @@ static int __init acpi_sleep_setup(char 
> > > > >  
> > > > >  __setup("acpi_sleep=", acpi_sleep_setup);
> > > > >  
> > > > > +/* Ouch, we want to delete this. We already have better version in 
> > > > userspace, in 
> > > > > +   s2ram from suspend.sf.net project */
> > > > 
> > > > Do we? This version has advantages in not requiring any userspace app 
and 
> > in 
> > > > being able to work even if you can't yet get as far as having
> > > 
> > > Take a look at the file. It has whitelist with just one entry, too
> > > bad.
> > 
> > The contents of the whitelist are irrelevant. My laptop needs this 
> > functionality, but I haven't bothered to send you a whitelist entry, in 
part 
> > because I don't use s2ram.
> > 
> > Regardless of that, if you had read the whole comment (you've deleted half 
of 
> > it), you would have noticed that I ended up changing my mind and instead 
> > saying "Why not just delete the __setup now, or at least put it in the 
> > deprecated file?"
> 
> That should be  certainly done in separate patch, right? It is on my
> todolist somewhere now.

Yeah, agree.

> > > > > @@ -124,7 +124,7 @@ real_save_cr3:	.long 0
> > > > >  real_save_cr4:	.long 0
> > > > >  real_magic:	.long 0
> > > > >  video_mode:	.long 0
> > > > > -video_flags:	.long 0
> > > > > +realmode_flags:	.long 0
> > > > >  beep_flags:	.long 0
> > > > >  real_efer_save_restore:	.long 0
> > > > >  real_save_efer_edx: 	.long 0
> > > > 
> > > > Beep_flags should be removed too if you're sticking with /proc.
> > > 
> > > Fixed.
> > 
> > Ta.  But you didn't answer the question - why /proc and not sysfs?
> 
> Do you seriously advocate setting two bits of one variable from /proc,
> and one more bit from /sys?

That's partly why I had a separate variable - retaining proc only because it's 
existing functionality, using sysfs for the new code. Remember, too, that 
they're really distinct functionality. The only thing they share is that 
they're both used in real mode.

Regards,

Nigel
-- 
Nigel, Michelle and Alisdair Cunningham
5 Mitchell Street
Cobden 3266
Victoria, Australia

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

> > > > > Beep_flags should be removed too if you're sticking with /proc.
> > > > 
> > > > Fixed.
> > > 
> > > Ta.  But you didn't answer the question - why /proc and not sysfs?
> > 
> > Do you seriously advocate setting two bits of one variable from /proc,
> > and one more bit from /sys?
> 
> That's partly why I had a separate variable - retaining proc only because it's 
> existing functionality, using sysfs for the new code. Remember, too, that 

/proc is not deprecated _that_ much, and notice that this is sysctl,
not regular procfs code.

Yes, I see why you did it that way, but I also think you overdisgned
it a bit. 
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

On Thursday, 5 July 2007 01:25, Pavel Machek wrote:
> 
> > > > > > Beep_flags should be removed too if you're sticking with /proc.
> > > > > 
> > > > > Fixed.
> > > > 
> > > > Ta.  But you didn't answer the question - why /proc and not sysfs?
> > > 
> > > Do you seriously advocate setting two bits of one variable from /proc,
> > > and one more bit from /sys?
> > 
> > That's partly why I had a separate variable - retaining proc only because it's 
> > existing functionality, using sysfs for the new code. Remember, too, that 
> 
> /proc is not deprecated _that_ much, and notice that this is sysctl,
> not regular procfs code.
> 
> Yes, I see why you did it that way, but I also think you overdisgned
> it a bit. 

Hmm, what about adding a second interface to acpi_realmode_flags in sysfs, in
a separate patch, and scheduling the old one, in /proc, for removal?

Greetings,
Rafael 


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Rafael J. Wysocki]

On Thursday, 5 July 2007 00:46, Pavel Machek wrote:
> Hi!
> 
> > > Here's the version that uses just one variable, relative to Nigel's
> > > patch. Hmm, and it also closes nasty trap for the user in
> > > acpi_sleep_setup; order of parameters actually mattered there,
> > > acpi_sleep=s3_bios,s3_mode doing something different from
> > > acpi_sleep=s3_mode,s3_bios .  It actually works here.
> > > 
> > > Will you fold it into  patch28, or should I create a changelog?
> > 
> > Hmm, I think we should keep the record straight.  Please add a changelog
> > if that's not a problem.
> 
> (I also added documentation and removed the superfluous beep_flags).

OK, thanks.

Added to the patchset as

http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc7/patches/50-Optional-Beeping-During-Resume-From-Suspend-To-Ram-tidy.patch

I think both patches are -mm-ready, aren't they?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

Hi!

> > > > Here's the version that uses just one variable, relative to Nigel's
> > > > patch. Hmm, and it also closes nasty trap for the user in
> > > > acpi_sleep_setup; order of parameters actually mattered there,
> > > > acpi_sleep=s3_bios,s3_mode doing something different from
> > > > acpi_sleep=s3_mode,s3_bios .  It actually works here.
> > > > 
> > > > Will you fold it into  patch28, or should I create a changelog?
> > > 
> > > Hmm, I think we should keep the record straight.  Please add a changelog
> > > if that's not a problem.
> > 
> > (I also added documentation and removed the superfluous beep_flags).
> 
> OK, thanks.
> 
> Added to the patchset as
> 
> http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc7/patches/50-Optional-Beeping-During-Resume-From-Suspend-To-Ram-tidy.patch
> 
> I think both patches are -mm-ready, aren't they?

I'd say so.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.

[Pavel Machek]

On Thu 2007-07-05 20:43:44, Rafael J. Wysocki wrote:
> On Thursday, 5 July 2007 01:25, Pavel Machek wrote:
> > 
> > > > > > > Beep_flags should be removed too if you're sticking with /proc.
> > > > > > 
> > > > > > Fixed.
> > > > > 
> > > > > Ta.  But you didn't answer the question - why /proc and not sysfs?
> > > > 
> > > > Do you seriously advocate setting two bits of one variable from /proc,
> > > > and one more bit from /sys?
> > > 
> > > That's partly why I had a separate variable - retaining proc only because it's 
> > > existing functionality, using sysfs for the new code. Remember, too, that 
> > 
> > /proc is not deprecated _that_ much, and notice that this is sysctl,
> > not regular procfs code.
> > 
> > Yes, I see why you did it that way, but I also think you overdisgned
> > it a bit. 
> 
> Hmm, what about adding a second interface to acpi_realmode_flags in sysfs, in
> a separate patch, and scheduling the old one, in /proc, for removal?

Well.. it is sysctl... so we already have separate interface,
sysctl(). Yes, sysctl will probably move to /sys one day, but that is
probably bigger project than just suspend.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html