Thanks for the fast answer,
On 1/14/25 5:20 PM, Ilpo Järvinen wrote:
>> Changes since v1:
>> - More clear description of 0xFF special effect when setting power limits
>> - Added structs for clearer naming of power limits and GPU power modes settings
>> - Retrieve and keep current GPU slowdown temp threshold (instead of hard coded)
>> - Removed platform_profile_victus_s_get(), re-using platform_profile_omen_get()
>> - Changed char variable types to u8 where it was more relevant
>> - Moved some comments
>> - Minor typo / alignment corrections
>
> I wrote a few comments to the v1 thread as the relevant discussion was
> there but they'll be relevant to a few places in this v2.
>> +static int hp_wmi_get_fan_count_userdefine_trigger(void)
>> +{
>> + u8 fan_data[4] = { 0 };
>
> {} is enough to initialize the entire array.
>> +static int hp_wmi_get_fan_speed_victus_s(int fan)
>> +{
>> + u8 fan_data[128] = { 0 };
>
> Ditto.
Understood, I just applied that.
While discussing these array initialisations to zero: I'm addressing what we finished discussing in the v1 thread by defining and using the following self explanatory named literals (instead of comments):
#define HP_FAN_SPEED_AUTOMATIC 0x00
#define HP_POWER_LIMIT_DEFAULT 0x00
#define HP_POWER_LIMIT_NO_CHANGE 0xFF
In this particular case, while technically the {} would work, I suppose it would still be preferable to change from:
u8 fan_speed[2] = { 0 }; /* Restores automatic speed */
to:
u8 fan_speed[2] = { HP_FAN_SPEED_AUTOMATIC }
>> +static int hp_wmi_fan_speed_max_reset(void)
>> +{
>> + int ret = hp_wmi_fan_speed_max_set(0);
>> +
>> + if (ret)
>
> To keep the call and its error handling together, please use this form:
>
> int ret;
>
> ret = hp_wmi_fan_speed_max_set(0);
> if (ret)
Understood, and applied.
Before sending the resulting v3, would you like me to apply this form change, alongside with { 0 } becoming {}, to some others already existing functions around? I prefer asking before doing so, because by default I'm not sure changing unrelated already existing lines is expected when sending a patch.
If not, I suppose this may be the matter of another future and unrelated patch submission.
Thanks again for the review work.
Best regards,
Julien ROBIN
