Re: [PATCH 2/2] serial: mctrl_gpio: Support all GPIO suffixes (gpios vs gpio)

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Aug 08, 2019 at 03:25:43PM +0200, Stefan Roese wrote:
> This patch fixes a backward compatibility issue, when boards use the
> old style GPIO suffix "-gpio" instead of the new "-gpios". This
> potential problem has been introduced by commit d99482673f95 ("serial:
> mctrl_gpio: Check if GPIO property exisits before requesting it").
> 
> This patch now fixes this issue by iterating over all supported GPIO
> suffixes by using the newly introduced for_each_gpio_suffix() helper.
> 
> Also, the string buffer is now allocated on the stack to avoid the
> problem of allocation in a loop and its potential failure.

>  	for (i = 0; i < UART_GPIO_MAX; i++) {
>  		enum gpiod_flags flags;
> -		char *gpio_str;
> +		const char *suffix;
> +		char gpio_str[32];	/* 32 is max size of property name */

Hmm... don't we have some define for the maximum length of property?

Or maybe we can still continue using kasprintf() approach?

>  		bool present;
> +		int k;
> +
> +		/*
> +		 * Check if GPIO property exists and continue if not. Iterate
> +		 * over all supported GPIO suffixes (foo-gpios vs. foo-gpio).
> +		 */
> +		for_each_gpio_suffix(k, suffix) {
> +			snprintf(gpio_str, sizeof(gpio_str), "%s-%s",
> +				 mctrl_gpios_desc[i].name, suffix);
> +
> +			present = device_property_present(dev, gpio_str);
> +			if (present)
> +				break;
> +		}
>  
> -		/* Check if GPIO property exists and continue if not */
> -		gpio_str = kasprintf(GFP_KERNEL, "%s-gpios",
> -				     mctrl_gpios_desc[i].name);
> -		if (!gpio_str)
> -			continue;
> -

> -		present = device_property_present(dev, gpio_str);

Because there is no more explicit assignment of present outside of the loop,
the compiler may warn about uninitialized variable in use...

> -		kfree(gpio_str);

>  		if (!present)

...here.

>  			continue;

-- 
With Best Regards,
Andy Shevchenko