Re: [PATCH v3 1/1] asus-wmi: Add support for platform_profile

[Andy Shevchenko]

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?

    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_*()?

> +       } 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;

> +       }


-- 
With Best Regards,
Andy Shevchenko