Re: [RFC PATCH v2] platform/x86: panasonic-laptop: Add platform_profile support

[Alex Yeo <alexyeo362@gmail.com>]

Thank you for taking the time to do a code review. I have made the 
specified changes for v3.

v3: 
https://lore.kernel.org/platform-driver-x86/20261007190446.38862-1-alexyeo362@gmail.com/

>> @@ -130,9 +134,12 @@
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/platform_profile.h>
>> +#include <linux/printk.h>
>>   #include <linux/seq_file.h>
>>   #include <linux/serio.h>
>>   #include <linux/slab.h>
>> +#include <linux/sysfs.h>
> 
> Thanks for the update.
> 
> I no longer recall if I suggested adding sysfs.h in the earlier review
> but I cannot now seem to locate anything that would be directly related to
> sysfs in this patch.
> 

Removed as no code uses that directly. Still builds with W=1 C=1.

>> +static int pcc_profile_platform_profile_probe(void *drvdata, unsigned long *choices)
>> +{
>> +	enum platform_profile_option pp_opt;
>> +	struct pcc_profile *pcc = drvdata;
>> +
>> +	for (pp_opt = 0; pp_opt < ARRAY_SIZE(pcc->quirks->platform_profiles); pp_opt++) {
>> +		const struct pcc_platform_profile *profile =
>> +			&pcc->quirks->platform_profiles[pp_opt];
>> +
>> +		if (profile->fan_mode != PCC_FAN_MODE_UNSET &&
>> +		    profile->tdp_mode != PCC_TDP_MODE_UNSET) {
>> +			set_bit(pp_opt, choices);
>> +		} else if (WARN_ON(profile->fan_mode != PCC_FAN_MODE_UNSET ||
>> +				   profile->tdp_mode != PCC_TDP_MODE_UNSET)) {
>> +			/* Both TDP and fan mode should be set */
>> +			return -EINVAL;
>> +		}
> 
> First check the error and return.
> 
> Then check double UNSET case and use continue.
> 
> Lastly, else is no longer needed for the set_bit() in the remaining cases.
> 

The code was refactored to fit that flow.
For the WARN_ON error check (first if statement), it follows the XOR 
pattern so != was used.

Thanks