bug#26919: [PATCH] Add thermald and thermald-service-type (for cpu frequency scaling)

[Ludovic Courtès]

Hi!

Christopher Allan Webber <address@hidden> skribis:

> Trying again, since last time I accidentally sent to bug-guix... oops.
>
> Hello!  I got tired of my laptop crashing so much due to overheating, so
> here's a package for thermald as well as a service type.

Heheh.  :-)

> From 56c7019d58d4a064fe44e626a1396b62cbf808b0 Mon Sep 17 00:00:00 2001
> From: Christopher Allan Webber <address@hidden>
> Date: Sat, 13 May 2017 19:36:01 -0500
> Subject: [PATCH 1/2] gnu: Add thermald.
>
> * gnu/packages/admin.scm (thermald): New variable.

LGTM!

> From de2045cdc81ddef7b01b3445fb731a3252f5b4a8 Mon Sep 17 00:00:00 2001
> From: Christopher Allan Webber <address@hidden>
> Date: Sat, 13 May 2017 19:37:02 -0500
> Subject: [PATCH 2/2] services: Add 'thermald-service-type'.
>
> * gnu/services/admin.scm (<thermald-configuration>): New record type.
> (thermald-shepherd-service, thermald-service-type): New variables.
> * doc/guix.texi (Thermal Management): New section documenting thermald.

[...]

> address@hidden Thermal Management

Would it make sense to move it under “Power Management Services” rather
than “Miscellaneous Services”?

> +The @code{(gnu services admin)} module provides an interface to
> +thermald, a cpu frequency scaling service which helps prevent overheating.

s/cpu/CPU/ :-)

> address@hidden {Scheme Variable} thermald-service-type
> +This is the service type for thermald.

What about something like this, to give more context:

  … for @uref{https://01.org/linux-thermal-daemon/, thermald}, the Linux
  Thermal Daemon, which is responsible for controlling the thermal state
  of Intel processors and preventing overheating.

?

> address@hidden {Data Type} thermald-configuration
> +Data type representing the configuration of THERMALD-SERVICE.

s/THERMALD-SERVICE/@code{thermald-service-type}/

> +(define-record-type* <thermald-configuration>
> +  thermald-configuration make-thermald-configuration
> +  thermald-configuration?
> +  (ignore-cpuid-check? thermald-ignore-cpuid-check?
> +                       (default #f)))

Could you add a ‘thermald’ field, defaulting to ‘thermald’ (the package)…

> +(define (thermald-shepherd-service config)
> +  (list
> +   (shepherd-service
> +    (provision '(thermald))
> +    (documentation "Run thermald cpu frequency scaling.")
> +    (start #~(make-forkexec-constructor
> +              '(#$(file-append thermald "/sbin/thermald")

… and honor it here, and add it to guix.texi?

OK with these changes, thank you!

Do you think it’s something we should add to ‘%base-services’, or is it
too hardware-dependent?

Ludo’.