Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
> > > > > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
pgpHMqUeXvRvr.pgp
Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.
pgpMtfWDsge3O.pgp
Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 <[EMAIL PROTECTED]>
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
/* 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
callmode_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)
movlsaved_videomode, %edx
movl%edx, video_mode - wakeup_start (%eax)
- movlacpi_video_flags, %edx
- movl%edx, video_flags - wakeup_start (%eax)
- movls2ram_beep, %edx
- movl%edx, beep_flags - wakeup_start (%eax)
+ movlacpi_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
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.
pgp4fmq32SK4d.pgp
Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
>
> /* 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
> callmode_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
>
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
/* 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
callmode_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:
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
callmode_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)
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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
callmode_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:
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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. pgp4fmq32SK4d.pgp Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 [EMAIL PROTECTED]
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
callmode_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)
movlsaved_videomode, %edx
movl%edx, video_mode - wakeup_start (%eax)
- movlacpi_video_flags, %edx
- movl%edx, video_flags - wakeup_start (%eax)
- movls2ram_beep, %edx
- movl%edx, beep_flags - wakeup_start (%eax)
+ movlacpi_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;
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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. pgpMtfWDsge3O.pgp Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 pgpHMqUeXvRvr.pgp Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 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) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 / RD 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) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 > > +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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 pgpS22bczXZSF.pgp Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 > +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 > 21:14:49.0 +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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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 pgpS22bczXZSF.pgp Description: PGP signature
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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) movlacpi_video_flags, %edx movl%edx, video_flags - wakeup_start (%eax) + movls2ram_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.0 +1000 +++ 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +1000 @@ -16,6 +16,21 @@ # cs = 0x1234, eip = 0x05 # +#define BEEP \ + inb $97, %al; \ + outb%al, $0x80; \ + movb$3, %al;\ +
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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.0 +1000 +++ 970-str-beep.patch-new/arch/i386/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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) movlacpi_video_flags, %edx movl%edx, video_flags - wakeup_start (%eax) + movls2ram_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.0 +1000 +++ 970-str-beep.patch-new/arch/x86_64/kernel/acpi/wakeup.S 2007-06-19 21:14:49.0 +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; \ +
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optional Beeping During Resume From Suspend To Ram.
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
