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
