Re: [PATCH v2 4/4] pwm: sysfs: Utilize an array for polarity strings

[Joe Perches]

On Fri, 2022-08-26 at 20:07 +0300, Andy Shevchenko wrote:
> Code is smaller and looks nicer if we combine polarity strings
> into an array.

It's less robust though as PWM_POLARITY_NORMAL and _INVERSED
are now required to be 0 and 1.  As the only 2 values in
an enum they are, but that's not really guaranteed unless
you read the enum definition.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> v2: added pwm_ prefix to the variable (Uwe), adjusted intendation (Uwe)
>  drivers/pwm/sysfs.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 767c4b19afb1..502167e44a3d 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -151,27 +151,23 @@ static ssize_t enable_store(struct device *child,
>  	return ret ? : size;
>  }
>  
> +static const char * const pwm_polarity_strings[] = {
> +	[PWM_POLARITY_NORMAL] = "normal",
> +	[PWM_POLARITY_INVERSED] = "inversed",
> +};
> +
>  static ssize_t polarity_show(struct device *child,
>  			     struct device_attribute *attr,
>  			     char *buf)
>  {
>  	const struct pwm_device *pwm = child_to_pwm_device(child);
> -	const char *polarity = "unknown";
>  	struct pwm_state state;
>  
>  	pwm_get_state(pwm, &state);
> +	if (state.polarity < 0 || state.polarity >= ARRAY_SIZE(pwm_polarity_strings))
> +		return sysfs_emit(buf, "unknown\n");
>  
> -	switch (state.polarity) {
> -	case PWM_POLARITY_NORMAL:
> -		polarity = "normal";
> -		break;
> -
> -	case PWM_POLARITY_INVERSED:
> -		polarity = "inversed";
> -		break;
> -	}
> -
> -	return sysfs_emit(buf, "%s\n", polarity);
> +	return sysfs_emit(buf, "%s\n", pwm_polarity_strings[state.polarity]);
>  }
>  
>  static ssize_t polarity_store(struct device *child,
> @@ -180,20 +176,16 @@ static ssize_t polarity_store(struct device *child,
>  {
>  	struct pwm_export *export = child_to_pwm_export(child);
>  	struct pwm_device *pwm = export->pwm;
> -	enum pwm_polarity polarity;
>  	struct pwm_state state;
>  	int ret;
>  
> -	if (sysfs_streq(buf, "normal"))
> -		polarity = PWM_POLARITY_NORMAL;
> -	else if (sysfs_streq(buf, "inversed"))
> -		polarity = PWM_POLARITY_INVERSED;
> -	else
> -		return -EINVAL;
> +	ret = sysfs_match_string(pwm_polarity_strings, buf);
> +	if (ret < 0)
> +		return ret;
>  
>  	mutex_lock(&export->lock);
>  	pwm_get_state(pwm, &state);
> -	state.polarity = polarity;
> +	state.polarity = ret;
>  	ret = pwm_apply_state(pwm, &state);
>  	mutex_unlock(&export->lock);
>