Re: [PATCH 2/5] platform/x86: think-lmi: Remove unnecessary parens

["Mark Pearson" <mpearson-lenovo@squebb.ca>]

On Tue, Dec 23, 2025, at 2:23 PM, Benjamin Philip wrote:
> This commit removes any unnecessary parentheses as flagged by
> checkpatch in the following check:
>
> CHECK: Unnecessary parentheses around '...'
>

Shouldn't you mention that you also corrected pieces of code alignment here too?

> Signed-off-by: Benjamin Philip <benjamin.philip495@gmail.com>
> ---
>  drivers/platform/x86/lenovo/think-lmi.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/think-lmi.c
> b/drivers/platform/x86/lenovo/think-lmi.c
> index 540b472b1bf3..1fac8986d077 100644
> --- a/drivers/platform/x86/lenovo/think-lmi.c
> +++ b/drivers/platform/x86/lenovo/think-lmi.c
> @@ -440,7 +440,7 @@ static ssize_t current_password_store(struct kobject *kobj,
>
>  	pwdlen = strlen(buf);
>  	/* pwdlen == 0 is allowed to clear the password */
> -	if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen)))
> +	if (pwdlen && (pwdlen < setting->minlen || pwdlen > setting->maxlen))
>  		return -EINVAL;
>
>  	strscpy(setting->password, buf, setting->maxlen);
> @@ -476,7 +476,7 @@ static ssize_t new_password_store(struct kobject *kobj,
>
>  	pwdlen = strlen(new_pwd);
>  	/* pwdlen == 0 is allowed to clear the password */
> -	if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen))) {
> +	if (pwdlen && (pwdlen < setting->minlen || pwdlen > setting->maxlen)) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -859,7 +859,7 @@ static ssize_t certificate_store(struct kobject *kobj,
>  			signature = setting->signature;
>  	} else { /* Cert install */
>  		/* Check if SMC and SVC already installed */
> -		if ((setting == tlmi_priv.pwd_system) &&
> tlmi_priv.pwd_admin->cert_installed) {
> +		if (setting == tlmi_priv.pwd_system && tlmi_priv.pwd_admin->cert_installed) {
>  			/* This gets treated as a cert update */
>  			install_mode = TLMI_CERT_UPDATE;
>  			signature = tlmi_priv.pwd_admin->signature;
> @@ -881,7 +881,7 @@ static ssize_t certificate_store(struct kobject *kobj,
>  	} else {
>  		/* This is a fresh install */
>  		/* To set admin cert, a password must be enabled */
> -		if ((setting == tlmi_priv.pwd_admin) &&
> +		if (setting == tlmi_priv.pwd_admin &&
>  		    (!setting->pwd_enabled || !setting->password[0])) {
>  			kfree(new_cert);
>  			return -EACCES;
> @@ -965,13 +965,13 @@ static ssize_t save_signature_store(struct kobject *kobj,
>  static struct kobj_attribute auth_save_signature = __ATTR_WO(save_signature);
>
>  static umode_t auth_attr_is_visible(struct kobject *kobj,
> -					     struct attribute *attr, int n)
> +				    struct attribute *attr, int n)
>  {
>  	struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj);
>
>  	/* We only want to display level and index settings on HDD/NVMe */
>  	if (attr == &auth_index.attr || attr == &auth_level.attr) {
> -		if ((setting == tlmi_priv.pwd_hdd) || (setting == tlmi_priv.pwd_nvme))
> +		if (setting == tlmi_priv.pwd_hdd || setting == tlmi_priv.pwd_nvme)
>  			return attr->mode;
>  		return 0;
>  	}
> @@ -985,8 +985,8 @@ static umode_t auth_attr_is_visible(struct kobject *kobj,
>  		if (tlmi_priv.certificate_support) {
>  			if (setting == tlmi_priv.pwd_admin)
>  				return attr->mode;
> -			if ((tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) &&
> -			    (setting == tlmi_priv.pwd_system))
> +			if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT &&
> +			    setting == tlmi_priv.pwd_system)
>  				return attr->mode;
>  		}
>  		return 0;
> @@ -1216,13 +1216,13 @@ static struct kobj_attribute attr_current_val
> = __ATTR_RW_MODE(current_value, 06
>
>  static struct kobj_attribute attr_type = __ATTR_RO(type);
>
> -static umode_t attr_is_visible(struct kobject *kobj,
> -					     struct attribute *attr, int n)
> +static umode_t attr_is_visible(struct kobject *kobj, struct attribute *attr,
> +			       int n)
>  {
>  	struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj);
>
>  	/* We don't want to display possible_values attributes if not available */
> -	if ((attr == &attr_possible_values.attr) && (!setting->possible_values))
> +	if (attr == &attr_possible_values.attr && !setting->possible_values)
>  		return 0;
>
>  	return attr->mode;
> -- 
> 2.52.0

I prefer the brackets as they were - for me it's clearer and removing them adds little value.
But the changes are fine, and I guess it's preferred overall by kernel community, so shrug.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>