Merge lp://staging/~manjo/ubuntu/trusty/systemd/HP-m800 into lp://staging/ubuntu/trusty/systemd

Requesting a review from Martin, who probably knows this bit better than I do. Is this safe to do, globally like this, or could it cause unintended weird consequences on other random platforms that happen to have something wired up to the same gpio?

Regarding the possibility of gpio_key.12 being used by other systems to map to some other trigger, I put in the check that the gpio_key.12 is associated with power control (keys=116). '116' is supposedly linux generic power control in DTS. There is no uniq idSystem or idVendor for device /dev/input/event0 as you can see from udevadm output, therefore I tried to use the best available combination as a safety check. It would have been nice if HP's fw had something like idSystem or idVendor attribute to /dev/input/event0 but that does not exist, in which case, this patch will enable power control for any system vendor (Other than the one the patch in intended for) that describes in DTS, the trigger gpio_key.12 as power control (116).

This looks ok to me as an SRU. But please file a bug report for it for SRU tracking/verification. Thanks!

Note that a rule can still refer to the general DMI data, like this:

SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform", KERNELS=="gpio_keys.12", ATTR{[dmi/id]sys_vendor}=="HP*", ATTR{[dmi/id]product_name}=="SuperCalc 100[234]*", ATTRS{keys}=="116", TAG+="power-switch"

So it is easily possible to limit such a rule to particular systems.

> Note that a rule can still refer to the general DMI data, like this:
>
> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
> KERNELS=="gpio_keys.12", ATTR{[dmi/id]sys_vendor}=="HP*",
> ATTR{[dmi/id]product_name}=="SuperCalc 100[234]*", ATTRS{keys}=="116", TAG
> +="power-switch"
>
> So it is easily possible to limit such a rule to particular systems.

The systems in question don't provide DMI data. Do you know of an equivalent with device-tree? I don't see any device-tree attributes available in sysfs - the best system identifier I see is /proc/device-tree/model.

dann frazier [2014-07-28 14:17 -0000]:
> The systems in question don't provide DMI data. Do you know of an
> equivalent with device-tree? I don't see any device-tree attributes
> available in sysfs - the best system identifier I see is
> /proc/device-tree/model.

Meh, too bad. I suppose an equivalent would be

  PROGRAM="/bin/grep FancyModel /proc/device-tree/model"

?

While having grep in a udev rule is ugly, I assume that would only trigger if the other conditions are met (something attached to that GPIO pin that we think we might want to play with), right? If so, I think it would be safer to do the device-tree model check than to just let the rule run free.

Sorry to be jumping in - Isn't this any help to isolate to the m800 cartridge ?
cat /proc/device-tree/model
HP ProLiant m800 Server Cartridge

Setting my review status back to "needs fixing" until you can test the grep rule suggested above, and this is working for both the systems you are interested in. Thanks!

> Setting my review status back to "needs fixing" until you can test the grep
> rule suggested above, and this is working for both the systems you are
> interested in. Thanks!

I think we are confusing two different platforms, What dannf is talking about is a different platform. The SRU I proposed is for HP-m800 server cartridges. There wont be a generic solution that will address soft shutdown for both platforms. On m800 platform gpio_keys.12 is associated with power control (attribute 116 which is generic power control attr for linux).

pitti/adam,

I have added a check to make sure the gpio_keys trigger are confined to m800 server cartridge by checking proc/device-tree/model. I think this solution is better than the PROGRAM='grep' that was suggested earlier.

Can you please review and accept? This fix was tested by me on m800 cartridge (c3n3) at canonical.

Thanks

For efficiency reasons I'd like to avoid calling the two shells and the cat on every platform. This can be done more efficiently with

SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform", KERNELS=="gpio_keys.12", ATTRS{keys}=="116", PROGRAM="/bin/cat /proc/device-tree/model", RESULT=="HP ProLiant m800*", TAG+="power-switch"

Then on any system which doesn't have a gpio_keys.12 there will be no additional program calls at all, and on that device it's a single cat (and if the file isn't pesent it'll just fail and RESULT will be empty).

Can you please double-check that?

== C3N3 ===

ubuntu@ubuntu:~$ cat /lib/udev/rules.d/70-power-switch.rules
# This file is part of systemd.
#
# systemd is free software; you can redistribute it and/or modify it
# under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation; either version 2.1 of the License, or
# (at your option) any later version.

ACTION=="remove", GOTO="power_switch_end"

SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform", KERNELS=="gpio_keys.12", ATTRS{keys}=="116", PROGRAM="/bin/cat /proc/device-tree/model", RESULT=="HP ProLiant m800 Server Cartridge", TAG+="power-switch"
SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="acpi", TAG+="power-switch"
SUBSYSTEM=="input", KERNEL=="event*", KERNELS=="thinkpad_acpi", TAG+="power-switch"

LABEL="power_switch_end"

ubuntu@ubuntu:~$ uname -a
Linux ubuntu 3.13.0-8-keystone #12-Ubuntu SMP Sat Jul 19 07:56:07 UTC 2014 armv7l armv7l armv7l GNU/Linux

ubuntu@ubuntu:~$ cat /proc/device-tree/model
HP ProLiant m800 Server Cartridgeubuntu@ubuntu:~$

== INITIATING SOFT SHUTDOWN FROM ILO ==
</>hpiLO-> set node power off shutdown c3n3

  c3: #Cartridge 3
    c3n3: #Node 3 Shutting node down gracefully

hpiLO->

== SYSTEM SHUTTING DOWN AS EXPECTED ==
ubuntu@ubuntu:~$ cat /proc/device-tree/model
HP ProLiant m800 Server Cartridgeubuntu@ubuntu:~$
Broadcast message from root@ubuntu
 (unknown) at 11:49 ...

The system is going down for power off NOW!
Connection to 192.168.17.24 closed by remote host.
Connection to 192.168.17.24 closed.

== CONFIRM WITH ILO THAT NODE IS POWERED DOWN ==
hpiLO-> show node list

Slot ID Proc Manufacturer Architecture Memory Power Health
---- ----- ---------------------- -------------------- ------ ----- ------
  3 c3n1 * ARM Architecture 8 GB Off OK
  3 c3n2 * ARM Architecture 8 GB On OK
  3 c3n3 * ARM Architecture 8 GB Off OK
  3 c3n4 * ARM Architecture 8 GB On OK

hpiLO->

Splendid!

I massaged that into a proper debian/patches patch and added a bug ref to the changelog. Committed to packaging git for utopic:

http://anonscm.debian.org/gitweb/?p=pkg-systemd/systemd.git;a=commitdiff;h=47c10f310

and also uploaded to trusty's SRU review queue. I'm closing this MP now as it won't be literally merged into the UDD branches.

Thanks!