On 12/19/2022 12:10 PM, Perry Yuan wrote:
--------------------->8-----------------------------
>  
> +/**
> + * enum amd_pstate_mode - driver working mode of amd pstate
> + */
> +
> +enum amd_pstate_mode {
> +	/** @AMD_PSTATE_DISABLE: Driver mode is disabled */
> +	AMD_PSTATE_DISABLE = 0,
> +
> +	/** @AMD_PSTATE_PASSIVE: Drier mode is passive mode */
> +	AMD_PSTATE_PASSIVE = 1,
> +
> +	/** @AMD_PSTATE_ACTIVE: Driver mode is active mode */
> +	AMD_PSTATE_ACTIVE = 2,
> +
> +	/** @AMD_PSTATE_GUIDE: Driver mode is guided mode */
> +	AMD_PSTATE_GUIDE = 3,
> +
> +	/** @AMD_PSTATE_MAX */
> +	AMD_PSTATE_MAX = 4,
> +};

IMO the above enum is self explanatory we don't need to annotate.

what about below?

/**
 * enum amd_pstate_mode - driver working mode
 * All supported modes are explained in kernel-parameters.txt
 */
enum amd_pstate_mode {
	AMD_PSTATE_DISABLE = 0,
	AMD_PSTATE_PASSIVE,
	AMD_PSTATE_ACTIVE,
	AMD_PSTATE_MAX,
};

Plz remove GUIDED mode here because it allows user to pass "amd_pstate=guided"
in kernel cmdline. Therefore it breaks the driver flow without guided patches.
I can update the enum in my guided patch.

> +
> +static const char * const amd_pstate_mode_string[] = {
> +	[AMD_PSTATE_DISABLE]     = "disable",
> +	[AMD_PSTATE_PASSIVE]     = "passive",
> +	[AMD_PSTATE_ACTIVE]      = "active",
> +	[AMD_PSTATE_GUIDE]      = "guide",
> +	NULL,
> +};
> +
>  #endif /* _LINUX_AMD_PSTATE_H */

-- 
Thanks & Regards,
Wyes