[Toybox] [PATCH] acpi: change when to recurse

[Isaac Dunham]

On Mon, Oct 06, 2014 at 12:36:08AM -0500, Rob Landley wrote:
> On 10/05/14 23:13, Isaac Dunham wrote:
> > On Sun, Oct 05, 2014 at 04:28:21PM -0500, Rob Landley wrote:
> >> On 10/04/14 15:52, Isaac Dunham wrote:
> >>> acpi: never follow symlinks, always recurse in dirs.
> >>>
> >>> /sys/class contains several directories, each containing symlinks to
> >>> devices of a given type.
> >>> The symlinks are where we want to end recursion.
> >>> This makes for a simpler, cleaner, and more correct check, avoids needless
> >>> recursion, and shrinks the text segment by 25 bytes (at least here).
> >>
> >> I _totally_ got bit by this doing mdev back on busybox... repeatedly:
> >>
> >> http://lists.busybox.net/pipermail/busybox/2006-February/052297.html
> >> http://lists.busybox.net/pipermail/busybox/2008-July/066210.html
> >>
> >> I.E. things that were directories can become symlinks, and vice versa.
> >> I'm reluctant to trust it.
> > 
> > Ok, thanks for the warning.
> > Is there a simple function to check absolute depth?
> > (ie, how many dirs down from / the current dirtree entry is)
> 
> I just recurse on the node's parent pointer are count up until I hit the
> null at the top. (Haven't bothered to make a function for it, it's a
> simple for loop.)
> 
> This doesn't tell you how far you are from /, it tells you how far you
> are from the top directory you started recursing from. (Which could be a
> path arbitrarily deep from root, but is generally a known quantity.)

In other words, "No, but relative depth should be fine."
Thanks, I can probably make it work.

> > I *really* hate the first way I wrote; it's not even a magic number, it's
> > a disgusting hack that *will* break if it ever gets called outside 
> > /sys/class/power_supply/.  And it certainly can't be copied for if we
> > recurse through /sys/class/thermal and /sys/class/power_supply.
> 
> What are the actual rules? What are you looking for in the filesystem?

The actual rules:
/sys/class/$CURRENT_CLASS/* should be the final nodes we visit.
 
<snip>
> >  -c  show cooling device status
> >  -t  show temperature
> > (-V  show everything we support)
> 
> Ok.
> 
> > All of them are useful to some degree for me.
> 
> Indeed.
> 
> >>>   recurse through /sys/class/thermal,
> >>>   read type, max_state, and cur_state,
> >>>   if max_state and cur_state were unreadable,
> >>>     printf("Cooling %d: %s no state information available\n", ctr++, type)
> >>>   else printf("Cooling %d: %s %d of %d\n",ctr++,type,cur_state, max_state)
> >>>
> >>> It would be rather difficult to do this in the same callback as -ab.
> >>
> >> Not knowing what -ab do, i think I'll have to re-reply to this message
> >> when I have time to dig through the code and see what the existing one
> >> does...
> > 
> > recurse through /sys/class/power_supply and read a few different files.
> 
> My LCD has a cooling device? Really?
> 
> Ok, it's got a max state of 9 and a cur state of 0. Meaning...?

/sys/class/thermal contains fans, and all the things that can *readily* be
throttled back to cut power (ie, LCDs and CPUs).

For a CPU, the CPU speed steps are numbered, with 0 as "slow as possible"
and max_state corresponding to the maximum speed.

Ostensibly, LCD max brightness is max_state; dimmest (or is it off?)
is cur_state=0.
In reality, many laptops are wired backwards, so max_state actually means
the dimmest state possible for them.

...Do you have anything in /sys/class/pwm?

> (Devices 0 and 1 are both "Processor" and cur_state 0, one has max state
> of 3 the other 10.)
> 
> So... yay thermal. That's easy to recurse on, but all we do is print out
> the numbers?

Believe it or not, that's all we do.
After all, any interpretation *will* be wrong somewhere.


> (And yes, give it its own dirtree callback function. If !parent then
> recurse following symlinks, otherwise openat() cur_state max_state and
> type relative to dirtree_parentfd().)

For temperature, we'll need to recurse through more of /sys/class;
my phone has a temperature sensor on its battery, and there's no telling
where else we might find one.
That means
  if (!tree->parent || !tree->parent->parent)
    return DIRTREE_RECURSE|DIRTREE_SYMFOLLOW;

> >>> The above method does ignore quite a few sensors;
> >>> there's no obvious way to get HDD temps besides these commands:
> >>>  smartctl -A -d ata /dev/sda |grep Temperature
> >>> # or (Hitachi-only, but no spin-up)
> >>>  hdparm -H /dev/sda
> >>
> >> strace, the breakfast of champions.
> > 
> > Yes, I've tried that and ltrace.
> > hdparm calls ioctl(fd, SG_IO, ...) with a very large struct that I haven't
> > figured out.
> 
> SG_IO sounds like it's sending its own scsi command. Possibly:
> 
> http://www.t10.org/lists/2op.htm
> 
> I can dig down into the kernel and try to reverse engineer it... Ah,
> there's docs:
> 
> http://tldp.org/HOWTO/SCSI-Generic-HOWTO/sg_io.html
> http://tldp.org/HOWTO/SCSI-Generic-HOWTO/pexample.html
> 
> Lots of docs:
> 
> http://sg.danny.cz/sg/sg_io.html
<snip> 
> There are standard scsi commands for basic status info, which are
> repeated in a bunch of places (such as the wikipedia page about scsi).

hdparm -H is supposedly a Hitachi-specific extension, which is 
preferable to the standard way because it doesn't spin up the disk to read
temperature.

I'm inclined to avoid reading the disk temperature via scsi commands.
It would make for a bit of a mess and could have "surprising" effects on
time you can run on battery, depending on the disk vendor.

Now if someone decided to write a driver that can export it to sysfs,
I would be fine with reading that.

> >>> Then also Thinkpads have multiple sensors, which are apparently named 
> >>> (in sysfs) temp*_input
> >>> The "official" acpi misses both the Thinkpad sensors and HDD sensors.
> >>
> >> If you have the testing environment, by all means let's get this right.
> > 
> >> Query: what are you trying to _do_?
> > 
> > Read temperatures (and perhaps cooling device state, but I can usually hear
> > that well enough...)
> 
> Let's start with the thermal one you listed first. I apparently have the
> stuff to test that here, and it seems really simple.
> 
> Thinkpads, I haven't got anymore. (About ten years ago I had more than
> one. But I haven't bought one since before they sold the line to lenovo.)

I have one:
<snip>
> > -"Caracal", a dual-core Thinkpad X100e suffering from chronic overheating
> > problems. Rarely use for prolonged periods. Runs Debian (Squeeze/ Sid
> > from when it meant ~Jessie); mainly used for CDE/Motif stuff.
> > (AMD Neo at 1.6 GHz; 2 gigs RAM)
<snip> 
> > So yes, I do have the environment for testing.
> 
> I actually blew a noticeable amount of money on a real laptop last year,
> system76 8x modernish processor (i5? i7? one of them. Probably not REAL
> 8x, I think it's hyper-threaded, but still) and 16 gigs ram.
> 
> Mild overheating problem, but it can build all the aboriginal targets
> (simultaneously, in parallel) in like 3 hours, as opposed to the 24
> hours my netbook takes.
> 
> That would actually be a fun thing to try thermal sensor stuff on,
> because the reason I know it has an overheating problem is dmesg says
> "thermal limit reached, processors throttled" when I leave it doing a 3
> hour build.

Much better than my Thinkpad.
Back in ~2009-2010 (late netbook craze era) AMD decided to put one of its
processors in the "mobile" market; it was higher power use than the Atom,
but it was (a) 64-bit, (b) AMD, (c) faster in terms of real performance,
and (d) had virtualization support.
Besides that, it came with a Radeon GPU, instead of Poulsbo.
So when I found that out that Lenovo had made a cheap 11.6" laptop with that,
I went for it.
And I went for the dual-core version.
The sole problem is that the fan wasn't quite enough for a single-core;
it would frequently shut down when I tried to do something like build
a kernel.
(If I would blow out the fan, lock it in "powersave" mode at 800 MHz,
set the fan to full-speed, and run it with make -j1 at nice 19 or 20,
it would "only" get to the mid-80 degree C range. Thermal shutdown is
92 C.)

> Hmmm...
> 
> $ ls /sys/class/thermal
> cooling_device0  cooling_device3  cooling_device6  thermal_zone0
> cooling_device1  cooling_device4  cooling_device7
> cooling_device2  cooling_device5  cooling_device8
> 
> $ ls /sys/class/thermal/thermal_zone0
> device  passive  power      temp               trip_point_0_type  uevent
> mode    policy   subsystem  trip_point_0_temp  type
 
thermal_zone0 is probably the fan.
Speaking of which, do you have a fan driver that allows setting fan speed?

> $ cat /sys/class/thermal/cooling_device*/type
> Processor
> Processor
> Processor
> Processor
> Processor
> Processor
> Processor
> Processor
> LCD
> 
> All the "Processor" entries are 0 3, and the LCD is 7 7. (For some
> reason the LCD's cur state and max state are the same, even though the
> lid's open and the display is off. Odd.)

See previous comment about backlights being wired backwards.

Incidentally...
I was thinking that I might need to use glob() in the callback for the
Thinkpad temperatures.
But it seems that I could just do 
  err = glob("/sys/class/*/*/temp", 0, NULL, &pglob);
  glob("/sys/class/*/*/temp*_input", err?GLOB_APPEND:0, NULL, &pglob);

And check if there are any matches.

Thanks,
Isaac Dunham

 1412634219.0