lkml.org 
[lkml]   [2024]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] platform/x86: dell-laptop: Implement platform_profile
On Tue, Apr 30, 2024, at 4:31 AM, Ilpo Järvinen wrote:
> On Mon, 29 Apr 2024, Lyndon Sanche wrote:
>> + */
>> +
>> +#define DELL_ACC_GET_FIELD GENMASK(19, 16)
>> +#define DELL_ACC_SET_FIELD GENMASK(11, 8)
>> +#define DELL_THERMAL_SUPPORTED GENMASK(3, 0)
>
> Please align these with tabs.
>

Agreed.

>> +enum thermal_mode_bits {
>> + DELL_BALANCED = BIT(0),
>> + DELL_COOL_BOTTOM = BIT(1),
>> + DELL_QUIET = BIT(2),
>> + DELL_PERFORMANCE = BIT(3),
>
> You need #include <linux/bits.h> for BIT().
>

Agreed.

>> + }
>> +}
>> +
>> +static int thermal_platform_profile_get(struct platform_profile_handler *pprof,
>> + enum platform_profile_option *profile)
>> +{
>> + int ret = thermal_get_mode();
>> +
>> + if (ret < 0)
>
> I think I already mentioned about this, change to:
>
> int ret;
>
> ret = thermal_get_mode();
> if (ret < 0)
>

I missed this.

>> + return ret;
>> +
>> + switch (ret) {
>> + case DELL_BALANCED:
>> + *profile = PLATFORM_PROFILE_BALANCED;
>> + break;
>> + case DELL_PERFORMANCE:
>> + *profile = PLATFORM_PROFILE_PERFORMANCE;
>> + break;
>> + case DELL_COOL_BOTTOM:
>> + *profile = PLATFORM_PROFILE_COOL;
>> + break;
>> + case DELL_QUIET:
>> + *profile = PLATFORM_PROFILE_QUIET;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int thermal_init(void)
>> +{
>> + int ret;
>> + int supported_modes;
>> +
>> + ret = thermal_get_supported_modes(&supported_modes);
>> + if (ret || !supported_modes)
>> + return 0;
>
> I think you should propagate the error code differently from nothing
> supported:
>
> if (ret < 0)
> return ret;
> if (!supported_modes)
> return 0;
>

Agreed.

Thank you for your feedback.

Lyndon

\
 
 \ /
  Last update: 2024-04-30 20:42    [W:0.157 / U:0.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site