Re: [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Wed, Feb 06, 2008 at 01:33:25AM -0500, Len Brown wrote:
> Thanks for the reply, Greg.
> 
> If these counters are not always available by default, then
> I've lost most of their value for supporting systems in the field.
> 
> Plus, I think I can live with just the individual sysfs files,
> given Bjorn's "grep . *" tip.
> 
> So if I follow the rules, do you think it is okay to put
> these stats in sysfs?  In the hopes the answer is "yes",
> I'll reply with a refreshed patch

Sure, one value per file is fine.  I'll be glad to review a patch like
that, with a Documentation/ABI/ entry :)

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
Thanks for the reply, Greg.

If these counters are not always available by default, then
I've lost most of their value for supporting systems in the field.

Plus, I think I can live with just the individual sysfs files,
given Bjorn's "grep . *" tip.

So if I follow the rules, do you think it is okay to put
these stats in sysfs?  In the hopes the answer is "yes",
I'll reply with a refreshed patch

cheers,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Tue, Feb 05, 2008 at 08:58:50PM -0500, Len Brown wrote:
> On Tuesday 05 February 2008 18:18, Greg KH wrote:
> > On Tue, Feb 05, 2008 at 06:12:09PM -0500, Len Brown wrote:
> > > On Tuesday 05 February 2008 17:18, Greg KH wrote:
> > > > On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> > > > > # cat /sys/firmware/acpi/interrupts/summary
> > > > > pm_timer 0
> > > > > glbl_lock0
> > > > > power_btn0
> > > > > sleep_btn0
> > > > > rtc  0
> > > > > gpe000
> > > ...
> > > > > gpe1F0
> > > > > gpe_hi0
> > > > > gpe_total   63
> > > > > acpi_irq63
> > > > 
> > > > Eeek!  Why?  What's wrong with individual files here?
> > > 
> > > My expectation is that this is a shell interface for debugging,
> > > not an API for programs.  ala /proc/interrupts.
> > 
> > Great, then use debugfs for it.  Please, don't put debug stuff like this
> > in sysfs, that's not what it is there for.  You can do whatever you want
> > in debugfs :)
> 
> Can you point to a model of good behaviour that I can copy?

Any user of the debugfs api you could copy for this.

> note that I want this information to be available on every system,
> just like /proc/interrupts is.

Ah, then /proc perhaps?

> /proc/ has seqfile support, is there a reason I shouldn't use it?
> I'd banned additional files from /proc/acpi for a long time
> since the directory layout was ill-conceived.  But maybe I
> should re-consider the headlong rush to use sysfs?

One of the main problems of /proc was that the files were not
documented, or that the format would change between versions, or that
they were different on different arches.

For something like this, yes, maybe you do need to use proc.  It can
handle almost infinite length files, and just make sure you document it
well.

But I would just stick with debugfs, all distros enable it when
shipping, you just have to ask them to mount it by hand usually:
mount -t debugfs none /sys/kernel/debug/

Either way, I wouldn't recommend sysfs for this.

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 18:18, Greg KH wrote:
> On Tue, Feb 05, 2008 at 06:12:09PM -0500, Len Brown wrote:
> > On Tuesday 05 February 2008 17:18, Greg KH wrote:
> > > On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> > > > # cat /sys/firmware/acpi/interrupts/summary
> > > > pm_timer 0
> > > > glbl_lock0
> > > > power_btn0
> > > > sleep_btn0
> > > > rtc  0
> > > > gpe000
> > ...
> > > > gpe1F0
> > > > gpe_hi0
> > > > gpe_total   63
> > > > acpi_irq63
> > > 
> > > Eeek!  Why?  What's wrong with individual files here?
> > 
> > My expectation is that this is a shell interface for debugging,
> > not an API for programs.  ala /proc/interrupts.
> 
> Great, then use debugfs for it.  Please, don't put debug stuff like this
> in sysfs, that's not what it is there for.  You can do whatever you want
> in debugfs :)

Can you point to a model of good behaviour that I can copy?

note that I want this information to be available on every system,
just like /proc/interrupts is.

/proc/ has seqfile support, is there a reason I shouldn't use it?
I'd banned additional files from /proc/acpi for a long time
since the directory layout was ill-conceived.  But maybe I
should re-consider the headlong rush to use sysfs?

thanks,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 19:44, Bjorn Helgaas wrote:
> On Tuesday 05 February 2008 04:12:09 pm Len Brown wrote:
> > is there
> > a version of cat that prints the file name before
> > the contents of each file?
> 
> I use "grep . *" for this sort of thing.

good tip, bjorn, thanks!

/sys/devices/system/cpu/cpu0/cpufreq # grep . *
affected_cpus:0
cpuinfo_cur_freq:1596000
cpuinfo_max_freq:266
cpuinfo_min_freq:1596000
scaling_available_frequencies:266 2128000 1596000
scaling_available_governors:conservative ondemand userspace powersave 
performance
scaling_cur_freq:1596000
scaling_driver:centrino
scaling_governor:ondemand
scaling_max_freq:266
scaling_min_freq:1596000
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Bjorn Helgaas
On Tuesday 05 February 2008 04:12:09 pm Len Brown wrote:
> is there
> a version of cat that prints the file name before
> the contents of each file?

I use "grep . *" for this sort of thing.
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Tue, Feb 05, 2008 at 06:12:09PM -0500, Len Brown wrote:
> On Tuesday 05 February 2008 17:18, Greg KH wrote:
> > On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> > > # cat /sys/firmware/acpi/interrupts/summary
> > > pm_timer 0
> > > glbl_lock0
> > > power_btn0
> > > sleep_btn0
> > > rtc  0
> > > gpe000
> ...
> > > gpe1F0
> > > gpe_hi0
> > > gpe_total   63
> > > acpi_irq63
> > 
> > Eeek!  Why?  What's wrong with individual files here?
> 
> My expectation is that this is a shell interface for debugging,
> not an API for programs.  ala /proc/interrupts.

Great, then use debugfs for it.  Please, don't put debug stuff like this
in sysfs, that's not what it is there for.  You can do whatever you want
in debugfs :)

> if we have 40 individual files, each with a number in it,
> it is less convenient to cat the file and paste the results
> into an email or bug report and have the receiver easily
> see what what count goes with what file -- or is there
> a version of cat that prints the file name before
> the contents of each file?
> 
> I've seen 
> more * | cat
> but one has to wonder why an interface would be built
> to make something so simple not be simple.

Use debugfs please.

> > What's to ensure 
> > that you aren't going to overflow your buffer?
> 
> Good question.  What's to ensure we don't overflow it
> when I print any other random string?
> Who allocates it and how big is it?

The core does, and if you are putting something too big in it, you are
using the sysfs interface wrong :)

Simple, one value per file is what sysfs is for.  Use debugfs for
debugging stuff.

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 17:18, Greg KH wrote:
> On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> > # cat /sys/firmware/acpi/interrupts/summary
> > pm_timer 0
> > glbl_lock0
> > power_btn0
> > sleep_btn0
> > rtc  0
> > gpe000
...
> > gpe1F0
> > gpe_hi0
> > gpe_total   63
> > acpi_irq63
> 
> Eeek!  Why?  What's wrong with individual files here?

My expectation is that this is a shell interface for debugging,
not an API for programs.  ala /proc/interrupts.

if we have 40 individual files, each with a number in it,
it is less convenient to cat the file and paste the results
into an email or bug report and have the receiver easily
see what what count goes with what file -- or is there
a version of cat that prints the file name before
the contents of each file?

I've seen 
more * | cat
but one has to wonder why an interface would be built
to make something so simple not be simple.

> What's to ensure 
> that you aren't going to overflow your buffer?

Good question.  What's to ensure we don't overflow it
when I print any other random string?
Who allocates it and how big is it?

> There's a reason we 
> don't have the seq file interface for sysfs, to prevent this very kind
> of thing...

If sysfs can't handle it, I suppose I could instead
print the needed information to the console

> > +   /*
> > +* General Purpose Events
> > +*/
> > +   for (i = 0; i < number_of_gpes; i++) {
> > +   count += sprintf(buf + count, "gpe%02X %4d\n",
> > +   i, acpi_gpe_counters[i]);
> > +   }
> 
> Nope, no error checking, fun chance of blowing the memory buffer :(

you're right, there is rumour of a future machine
with more than 32 GPEs...

> Please change the interface to not do this.
> 
> Oh, and for every new sysfs file, you need a Documentation/ABI/
> addition, please also add that.

ok.

thanks,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> # cat /sys/firmware/acpi/interrupts/summary
> pm_timer 0
> glbl_lock0
> power_btn0
> sleep_btn0
> rtc  0
> gpe000
> gpe010
> gpe020
> gpe030
> gpe040
> gpe050
> gpe060
> gpe070
> gpe080
> gpe092
> gpe0A0
> gpe0B0
> gpe0C0
> gpe0D0
> gpe0E0
> gpe0F0
> gpe100
> gpe11   60
> gpe120
> gpe130
> gpe140
> gpe150
> gpe160
> gpe170
> gpe180
> gpe191
> gpe1A0
> gpe1B0
> gpe1C0
> gpe1D0
> gpe1E0
> gpe1F0
> gpe_hi0
> gpe_total   63
> acpi_irq63

Eeek!  Why?  What's wrong with individual files here?  What's to ensure
that you aren't going to overflow your buffer?  There's a reason we
don't have the seq file interface for sysfs, to prevent this very kind
of thing...


> + /*
> +  * General Purpose Events
> +  */
> + for (i = 0; i < number_of_gpes; i++) {
> + count += sprintf(buf + count, "gpe%02X %4d\n",
> + i, acpi_gpe_counters[i]);
> + }

Nope, no error checking, fun chance of blowing the memory buffer :(

Please change the interface to not do this.

Oh, and for every new sysfs file, you need a Documentation/ABI/
addition, please also add that.

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
 # cat /sys/firmware/acpi/interrupts/summary
 pm_timer 0
 glbl_lock0
 power_btn0
 sleep_btn0
 rtc  0
 gpe000
 gpe010
 gpe020
 gpe030
 gpe040
 gpe050
 gpe060
 gpe070
 gpe080
 gpe092
 gpe0A0
 gpe0B0
 gpe0C0
 gpe0D0
 gpe0E0
 gpe0F0
 gpe100
 gpe11   60
 gpe120
 gpe130
 gpe140
 gpe150
 gpe160
 gpe170
 gpe180
 gpe191
 gpe1A0
 gpe1B0
 gpe1C0
 gpe1D0
 gpe1E0
 gpe1F0
 gpe_hi0
 gpe_total   63
 acpi_irq63

Eeek!  Why?  What's wrong with individual files here?  What's to ensure
that you aren't going to overflow your buffer?  There's a reason we
don't have the seq file interface for sysfs, to prevent this very kind
of thing...


 + /*
 +  * General Purpose Events
 +  */
 + for (i = 0; i  number_of_gpes; i++) {
 + count += sprintf(buf + count, gpe%02X %4d\n,
 + i, acpi_gpe_counters[i]);
 + }

Nope, no error checking, fun chance of blowing the memory buffer :(

Please change the interface to not do this.

Oh, and for every new sysfs file, you need a Documentation/ABI/
addition, please also add that.

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 17:18, Greg KH wrote:
 On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
  # cat /sys/firmware/acpi/interrupts/summary
  pm_timer 0
  glbl_lock0
  power_btn0
  sleep_btn0
  rtc  0
  gpe000
...
  gpe1F0
  gpe_hi0
  gpe_total   63
  acpi_irq63
 
 Eeek!  Why?  What's wrong with individual files here?

My expectation is that this is a shell interface for debugging,
not an API for programs.  ala /proc/interrupts.

if we have 40 individual files, each with a number in it,
it is less convenient to cat the file and paste the results
into an email or bug report and have the receiver easily
see what what count goes with what file -- or is there
a version of cat that prints the file name before
the contents of each file?

I've seen 
more * | cat
but one has to wonder why an interface would be built
to make something so simple not be simple.

 What's to ensure 
 that you aren't going to overflow your buffer?

Good question.  What's to ensure we don't overflow it
when I print any other random string?
Who allocates it and how big is it?

 There's a reason we 
 don't have the seq file interface for sysfs, to prevent this very kind
 of thing...

If sysfs can't handle it, I suppose I could instead
print the needed information to the console

  +   /*
  +* General Purpose Events
  +*/
  +   for (i = 0; i  number_of_gpes; i++) {
  +   count += sprintf(buf + count, gpe%02X %4d\n,
  +   i, acpi_gpe_counters[i]);
  +   }
 
 Nope, no error checking, fun chance of blowing the memory buffer :(

you're right, there is rumour of a future machine
with more than 32 GPEs...

 Please change the interface to not do this.
 
 Oh, and for every new sysfs file, you need a Documentation/ABI/
 addition, please also add that.

ok.

thanks,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Bjorn Helgaas
On Tuesday 05 February 2008 04:12:09 pm Len Brown wrote:
 is there
 a version of cat that prints the file name before
 the contents of each file?

I use grep . * for this sort of thing.
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 19:44, Bjorn Helgaas wrote:
 On Tuesday 05 February 2008 04:12:09 pm Len Brown wrote:
  is there
  a version of cat that prints the file name before
  the contents of each file?
 
 I use grep . * for this sort of thing.

good tip, bjorn, thanks!

/sys/devices/system/cpu/cpu0/cpufreq # grep . *
affected_cpus:0
cpuinfo_cur_freq:1596000
cpuinfo_max_freq:266
cpuinfo_min_freq:1596000
scaling_available_frequencies:266 2128000 1596000
scaling_available_governors:conservative ondemand userspace powersave 
performance
scaling_cur_freq:1596000
scaling_driver:centrino
scaling_governor:ondemand
scaling_max_freq:266
scaling_min_freq:1596000
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
On Tuesday 05 February 2008 18:18, Greg KH wrote:
 On Tue, Feb 05, 2008 at 06:12:09PM -0500, Len Brown wrote:
  On Tuesday 05 February 2008 17:18, Greg KH wrote:
   On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
# cat /sys/firmware/acpi/interrupts/summary
pm_timer 0
glbl_lock0
power_btn0
sleep_btn0
rtc  0
gpe000
  ...
gpe1F0
gpe_hi0
gpe_total   63
acpi_irq63
   
   Eeek!  Why?  What's wrong with individual files here?
  
  My expectation is that this is a shell interface for debugging,
  not an API for programs.  ala /proc/interrupts.
 
 Great, then use debugfs for it.  Please, don't put debug stuff like this
 in sysfs, that's not what it is there for.  You can do whatever you want
 in debugfs :)

Can you point to a model of good behaviour that I can copy?

note that I want this information to be available on every system,
just like /proc/interrupts is.

/proc/ has seqfile support, is there a reason I shouldn't use it?
I'd banned additional files from /proc/acpi for a long time
since the directory layout was ill-conceived.  But maybe I
should re-consider the headlong rush to use sysfs?

thanks,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Tue, Feb 05, 2008 at 08:58:50PM -0500, Len Brown wrote:
 On Tuesday 05 February 2008 18:18, Greg KH wrote:
  On Tue, Feb 05, 2008 at 06:12:09PM -0500, Len Brown wrote:
   On Tuesday 05 February 2008 17:18, Greg KH wrote:
On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
 # cat /sys/firmware/acpi/interrupts/summary
 pm_timer 0
 glbl_lock0
 power_btn0
 sleep_btn0
 rtc  0
 gpe000
   ...
 gpe1F0
 gpe_hi0
 gpe_total   63
 acpi_irq63

Eeek!  Why?  What's wrong with individual files here?
   
   My expectation is that this is a shell interface for debugging,
   not an API for programs.  ala /proc/interrupts.
  
  Great, then use debugfs for it.  Please, don't put debug stuff like this
  in sysfs, that's not what it is there for.  You can do whatever you want
  in debugfs :)
 
 Can you point to a model of good behaviour that I can copy?

Any user of the debugfs api you could copy for this.

 note that I want this information to be available on every system,
 just like /proc/interrupts is.

Ah, then /proc perhaps?

 /proc/ has seqfile support, is there a reason I shouldn't use it?
 I'd banned additional files from /proc/acpi for a long time
 since the directory layout was ill-conceived.  But maybe I
 should re-consider the headlong rush to use sysfs?

One of the main problems of /proc was that the files were not
documented, or that the format would change between versions, or that
they were different on different arches.

For something like this, yes, maybe you do need to use proc.  It can
handle almost infinite length files, and just make sure you document it
well.

But I would just stick with debugfs, all distros enable it when
shipping, you just have to ask them to mount it by hand usually:
mount -t debugfs none /sys/kernel/debug/

Either way, I wouldn't recommend sysfs for this.

thanks,

greg k-h
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Len Brown
Thanks for the reply, Greg.

If these counters are not always available by default, then
I've lost most of their value for supporting systems in the field.

Plus, I think I can live with just the individual sysfs files,
given Bjorn's grep . * tip.

So if I follow the rules, do you think it is okay to put
these stats in sysfs?  In the hopes the answer is yes,
I'll reply with a refreshed patch

cheers,
-Len
--
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 for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-05 Thread Greg KH
On Wed, Feb 06, 2008 at 01:33:25AM -0500, Len Brown wrote:
 Thanks for the reply, Greg.
 
 If these counters are not always available by default, then
 I've lost most of their value for supporting systems in the field.
 
 Plus, I think I can live with just the individual sysfs files,
 given Bjorn's grep . * tip.
 
 So if I follow the rules, do you think it is okay to put
 these stats in sysfs?  In the hopes the answer is yes,
 I'll reply with a refreshed patch

Sure, one value per file is fine.  I'll be glad to review a patch like
that, with a Documentation/ABI/ entry :)

thanks,

greg k-h
--
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/


[PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-04 Thread Len Brown
From: Len Brown <[EMAIL PROTECTED]>

Here is the ACPI GPE statistics patch, forward ported to grok 2.6.25's kobject 
changes.
(Greg, let me know if I was able to unleash the inner beauty of the kobj 
interface)
(yes, I know you don't like sysfs files with more than 1 value,
 but I violate that rule for only one of the 33 files:-)

thanks,
-Len
--

# ls /sys/firmware/acpi/interrupts/
gpe00  gpe02  gpe04  gpe06  gpe08  gpe0A  gpe0C  gpe0E  gpe10  gpe12  gpe14  
gpe16  gpe18  gpe1A  gpe1C  gpe1E  summary
gpe01  gpe03  gpe05  gpe07  gpe09  gpe0B  gpe0D  gpe0F  gpe11  gpe13  gpe15  
gpe17  gpe19  gpe1B  gpe1D  gpe1F

# cat /sys/firmware/acpi/interrupts/summary
pm_timer 0
glbl_lock0
power_btn0
sleep_btn0
rtc  0
gpe000
gpe010
gpe020
gpe030
gpe040
gpe050
gpe060
gpe070
gpe080
gpe092
gpe0A0
gpe0B0
gpe0C0
gpe0D0
gpe0E0
gpe0F0
gpe100
gpe11   60
gpe120
gpe130
gpe140
gpe150
gpe160
gpe170
gpe180
gpe191
gpe1A0
gpe1B0
gpe1C0
gpe1D0
gpe1E0
gpe1F0
gpe_hi0
gpe_total   63
acpi_irq63

Inspired-by-original-patch-by: Luming Yu <[EMAIL PROTECTED]>
Signed-off-by: Len Brown <[EMAIL PROTECTED]>
---
 drivers/acpi/events/evevent.c |2 +-
 drivers/acpi/events/evgpe.c   |2 +
 drivers/acpi/osl.c|   12 ++-
 drivers/acpi/system.c |  218 +
 drivers/acpi/utilities/utglobal.c |2 +
 include/acpi/acglobal.h   |1 +
 include/acpi/aclocal.h|1 +
 include/acpi/acpiosxf.h   |2 +
 include/linux/acpi.h  |2 +
 9 files changed, 240 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/events/evevent.c b/drivers/acpi/events/evevent.c
index e412878..18514bf 100644
--- a/drivers/acpi/events/evevent.c
+++ b/drivers/acpi/events/evevent.c
@@ -259,7 +259,7 @@ u32 acpi_ev_fixed_event_detect(void)
enable_bit_mask)) {
 
/* Found an active (signalled) event */
-
+   acpi_fixed_event_count[i]++;
int_status |= acpi_ev_fixed_event_dispatch((u32) i);
}
}
diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
index e22f4a9..515128f 100644
--- a/drivers/acpi/events/evgpe.c
+++ b/drivers/acpi/events/evgpe.c
@@ -620,6 +620,8 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_event_info 
*gpe_event_info, u32 gpe_number)
 
acpi_gpe_count++;
 
+   acpi_os_gpe_count(gpe_number);
+
/*
 * If edge-triggered, clear the GPE status bit now.  Note that
 * level-triggered events are cleared after the GPE is serviced.
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index e53fb51..1087efe 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -332,7 +332,15 @@ acpi_os_table_override(struct acpi_table_header * 
existing_table,
 
 static irqreturn_t acpi_irq(int irq, void *dev_id)
 {
-   return (*acpi_irq_handler) (acpi_irq_context) ? IRQ_HANDLED : IRQ_NONE;
+   u32 handled;
+
+   handled = (*acpi_irq_handler) (acpi_irq_context);
+
+   if (handled) {
+   acpi_irq_handled++;
+   return IRQ_HANDLED;
+   } else
+   return IRQ_NONE;
 }
 
 acpi_status
@@ -341,6 +349,8 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler 
handler,
 {
unsigned int irq;
 
+   acpi_irq_stats_init();
+
/*
 * Ignore the GSI from the core, and use the value in our copy of the
 * FADT. It may not be the same if an interrupt source override exists
diff --git a/drivers/acpi/system.c b/drivers/acpi/system.c
index 5ffe0ea..538d154 100644
--- a/drivers/acpi/system.c
+++ b/drivers/acpi/system.c
@@ -40,6 +40,8 @@ ACPI_MODULE_NAME("system");
 #define ACPI_SYSTEM_CLASS  "system"
 #define ACPI_SYSTEM_DEVICE_NAME"System"
 
+u32 acpi_irq_handled;
+
 /*
  * Make ACPICA version work as module param
  */
@@ -166,6 +168,222 @@ static int acpi_system_sysfs_init(void)
return 0;
 }
 
+/*
+ * ACPI IRQ counters
+ *
+ * /sys/firmware/acpi/interrupts/
+ *   summary -- IRQ, Fixed Event, and GPE counters
+ *   gpeXX -- broken-out counters for up to 32 GPEs
+ */
+
+static u32 *acpi_gpe_counters;
+static u32 gpe_counter_high;   /* bucket for GPE's >= 32 */
+static u32 number_of_gpes;
+static struct attribute **all_attrs;
+
+static struct attribute_group interrupt_stats_attr_group = {
+   .name = "interrupts",
+};
+static struct kobj_attribute *gpe_attrs;
+
+static int count_num_gpes(void)
+{
+   int count = 0;
+   struct acpi_gpe_xrupt_info *gpe_xrupt_info;
+   struct acpi_gpe_block_info *gpe_block;
+   acpi_cpu_flags flags;
+
+   flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+
+   gpe_xrupt_info = acpi_gbl_gpe_xrupt_list_head;
+   while (gpe_xrupt_info) {
+   gpe_block = 

[PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters

2008-02-04 Thread Len Brown
From: Len Brown [EMAIL PROTECTED]

Here is the ACPI GPE statistics patch, forward ported to grok 2.6.25's kobject 
changes.
(Greg, let me know if I was able to unleash the inner beauty of the kobj 
interface)
(yes, I know you don't like sysfs files with more than 1 value,
 but I violate that rule for only one of the 33 files:-)

thanks,
-Len
--

# ls /sys/firmware/acpi/interrupts/
gpe00  gpe02  gpe04  gpe06  gpe08  gpe0A  gpe0C  gpe0E  gpe10  gpe12  gpe14  
gpe16  gpe18  gpe1A  gpe1C  gpe1E  summary
gpe01  gpe03  gpe05  gpe07  gpe09  gpe0B  gpe0D  gpe0F  gpe11  gpe13  gpe15  
gpe17  gpe19  gpe1B  gpe1D  gpe1F

# cat /sys/firmware/acpi/interrupts/summary
pm_timer 0
glbl_lock0
power_btn0
sleep_btn0
rtc  0
gpe000
gpe010
gpe020
gpe030
gpe040
gpe050
gpe060
gpe070
gpe080
gpe092
gpe0A0
gpe0B0
gpe0C0
gpe0D0
gpe0E0
gpe0F0
gpe100
gpe11   60
gpe120
gpe130
gpe140
gpe150
gpe160
gpe170
gpe180
gpe191
gpe1A0
gpe1B0
gpe1C0
gpe1D0
gpe1E0
gpe1F0
gpe_hi0
gpe_total   63
acpi_irq63

Inspired-by-original-patch-by: Luming Yu [EMAIL PROTECTED]
Signed-off-by: Len Brown [EMAIL PROTECTED]
---
 drivers/acpi/events/evevent.c |2 +-
 drivers/acpi/events/evgpe.c   |2 +
 drivers/acpi/osl.c|   12 ++-
 drivers/acpi/system.c |  218 +
 drivers/acpi/utilities/utglobal.c |2 +
 include/acpi/acglobal.h   |1 +
 include/acpi/aclocal.h|1 +
 include/acpi/acpiosxf.h   |2 +
 include/linux/acpi.h  |2 +
 9 files changed, 240 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/events/evevent.c b/drivers/acpi/events/evevent.c
index e412878..18514bf 100644
--- a/drivers/acpi/events/evevent.c
+++ b/drivers/acpi/events/evevent.c
@@ -259,7 +259,7 @@ u32 acpi_ev_fixed_event_detect(void)
enable_bit_mask)) {
 
/* Found an active (signalled) event */
-
+   acpi_fixed_event_count[i]++;
int_status |= acpi_ev_fixed_event_dispatch((u32) i);
}
}
diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
index e22f4a9..515128f 100644
--- a/drivers/acpi/events/evgpe.c
+++ b/drivers/acpi/events/evgpe.c
@@ -620,6 +620,8 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_event_info 
*gpe_event_info, u32 gpe_number)
 
acpi_gpe_count++;
 
+   acpi_os_gpe_count(gpe_number);
+
/*
 * If edge-triggered, clear the GPE status bit now.  Note that
 * level-triggered events are cleared after the GPE is serviced.
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index e53fb51..1087efe 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -332,7 +332,15 @@ acpi_os_table_override(struct acpi_table_header * 
existing_table,
 
 static irqreturn_t acpi_irq(int irq, void *dev_id)
 {
-   return (*acpi_irq_handler) (acpi_irq_context) ? IRQ_HANDLED : IRQ_NONE;
+   u32 handled;
+
+   handled = (*acpi_irq_handler) (acpi_irq_context);
+
+   if (handled) {
+   acpi_irq_handled++;
+   return IRQ_HANDLED;
+   } else
+   return IRQ_NONE;
 }
 
 acpi_status
@@ -341,6 +349,8 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler 
handler,
 {
unsigned int irq;
 
+   acpi_irq_stats_init();
+
/*
 * Ignore the GSI from the core, and use the value in our copy of the
 * FADT. It may not be the same if an interrupt source override exists
diff --git a/drivers/acpi/system.c b/drivers/acpi/system.c
index 5ffe0ea..538d154 100644
--- a/drivers/acpi/system.c
+++ b/drivers/acpi/system.c
@@ -40,6 +40,8 @@ ACPI_MODULE_NAME(system);
 #define ACPI_SYSTEM_CLASS  system
 #define ACPI_SYSTEM_DEVICE_NAMESystem
 
+u32 acpi_irq_handled;
+
 /*
  * Make ACPICA version work as module param
  */
@@ -166,6 +168,222 @@ static int acpi_system_sysfs_init(void)
return 0;
 }
 
+/*
+ * ACPI IRQ counters
+ *
+ * /sys/firmware/acpi/interrupts/
+ *   summary -- IRQ, Fixed Event, and GPE counters
+ *   gpeXX -- broken-out counters for up to 32 GPEs
+ */
+
+static u32 *acpi_gpe_counters;
+static u32 gpe_counter_high;   /* bucket for GPE's = 32 */
+static u32 number_of_gpes;
+static struct attribute **all_attrs;
+
+static struct attribute_group interrupt_stats_attr_group = {
+   .name = interrupts,
+};
+static struct kobj_attribute *gpe_attrs;
+
+static int count_num_gpes(void)
+{
+   int count = 0;
+   struct acpi_gpe_xrupt_info *gpe_xrupt_info;
+   struct acpi_gpe_block_info *gpe_block;
+   acpi_cpu_flags flags;
+
+   flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+
+   gpe_xrupt_info = acpi_gbl_gpe_xrupt_list_head;
+   while (gpe_xrupt_info) {
+   gpe_block =