Hi Andy, thanks for the feedback. All is addressed, and some inline
comment/reply. New version incoming pending pr_info() vs dev_info()
On Sat, Aug 14 2021 at 12:40:39 +0300, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
On Sat, Aug 14, 2021 at 7:33 AM Luke D. Jones <luke@xxxxxxxxxx> wrote:
Add initial support for platform_profile where the support is
based on availability of ASUS_THROTTLE_THERMAL_POLICY.
Because throttle_thermal_policy is used by platform_profile and is
writeable separately to platform_profile any userspace changes to
throttle_thermal_policy need to notify platform_profile.
In future throttle_thermal_policy sysfs should be removed so that
only one method controls the laptop power profile.
Some comments below.
...
+ /*
+ * Ensure that platform_profile updates userspace with the
change to ensure
+ * that platform_profile and throttle_thermal_policy_mode
are in sync
Missed period here and in other multi-line comments.
+ */
...
+ /* All possible toggles like throttle_thermal_policy here */
+ if (asus->throttle_thermal_policy_available) {
+ tp = asus->throttle_thermal_policy_mode;
+ } else {
+ return -1;
+ }
+
+ if (tp < 0)
+ return tp;
This will be better in a form
if (!..._available)
return -ENODATA; // what -1 means?
I wasn't sure what the best return was here. On your suggestion I've
changed to ENODATA
tp = ...;
if (tp < 0)
return tp;
...
+ /* All possible toggles like throttle_thermal_policy here */
+ if (!asus->throttle_thermal_policy_available) {
+ return -1;
See above.
+ }
...
+ if (asus->throttle_thermal_policy_available) {
+ pr_info("Using throttle_thermal_policy for
platform_profile support\n");
Why pr_*()?
That seemed to be the convention? I see there is also dev_info(), so
I've switched to that as it seems more appropriate.
+ } else {
+ /*
+ * Not an error if a component platform_profile
relies on is unavailable
+ * so early return, skipping the setup of
platform_profile.
+ */
+ return 0;
Do it other way around,
/*
* Comment
*/
if (!...)
return 0;
Thanks, I think I was transliterating thought process to code as I went
along.
All updated now.
+ }
--
With Best Regards,
Andy Shevchenko
