Re: [External] Re: [PATCH v2 3/3] platform/x86: thinkpad_acpi: Add platform profile support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi


2020. november 16., hétfő 0:22 keltezéssel, Mark Pearson írta:

> [...]
> >> +static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode)
> >> +{
> >> +	switch (profile) {
> >> +	case profile_low:
> >> +		*perfmode = DYTC_MODE_QUIET;
> >> +		break;
> >> +	case profile_balance:
> >> +		*perfmode = DYTC_MODE_BALANCE;
> >> +		break;
> >> +	case profile_perform:
> >> +		*perfmode = DYTC_MODE_PERFORM;
> >> +		break;
> >> +	default: /* Unknown profile */
> >> +		return -EOPNOTSUPP;
> >
> > I personally think EINVAL would be better here,
> > just like in `convert_dytc_to_profile()`.
> I liked how this worked when testing.
> If you put in an invalid profile name then platform_profile returned
> EINVAL but if you got this far you'd provided a valid profile setting
> that this driver doesn't support and the not supported message seemed
> clearer. As an example 'cool' is used on HP platforms but not Lenovo.
> I'd like to leave this one as it is please.
> >

I have just realized that the platform profile module does not check if
the profile the user wants to set is supported by the handler. As I've
mentioned in my other email, I think that should be checked. You could return
EOPNOTSUPP there (in the platform profile module). After reading the explanation
why you want to use EOPNOTSUPP, I believe it makes sense and it's fine.


> >
> >> +	}
> >> +	return 0;
> >> +}
> [...]


Regards,
Barnabás Pőcze




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux