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

[Alex Yeo]

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@xxxxxxxxx/

> > @@ -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