Re: [PATCH 2/3] Input: rotary_encoder - fix device tree error handling

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Timo,

On Fri, Jan 15, 2016 at 10:33:49PM +0200, Timo Teräs wrote:
> of_get_gpio_flags() can fail, so pass through the error code
> properly. Most important use case is when it returns EPROBE_DEFER
> so that the probe gets called again later.
> 
> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
> ---
>  drivers/input/misc/rotary_encoder.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 0f23d32..156f40f 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -206,7 +206,7 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  	struct device_node *np = dev->of_node;
>  	struct rotary_encoder_platform_data *pdata;
>  	enum of_gpio_flags flags;
> -	int error;
> +	int ret;

I strongly prefer variables holding error codes and not being returned
to caller in success path to be called "error".

>  
>  	if (!of_id || !np)
>  		return NULL;
> @@ -219,19 +219,25 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  	of_property_read_u32(np, "rotary-encoder,steps", &pdata->steps);
>  	of_property_read_u32(np, "linux,axis", &pdata->axis);
>  
> -	pdata->gpio_a = of_get_gpio_flags(np, 0, &flags);
> +	ret = of_get_gpio_flags(np, 0, &flags);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +	pdata->gpio_a = ret;

Instead of doing this maybe we could switch to gpiod interface that
handles polarity internally and does not depend on OF? The only issue is
that we have a few board files in tree that use raw gpio numbers so
we'll need to convert them...

Thanks.

>  	pdata->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
>  
> -	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
> +	ret = of_get_gpio_flags(np, 1, &flags);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +	pdata->gpio_b = ret;
>  	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
>  
>  	pdata->relative_axis =
>  		of_property_read_bool(np, "rotary-encoder,relative-axis");
>  	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
>  
> -	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
> -				     &pdata->steps_per_period);
> -	if (error) {
> +	ret = of_property_read_u32(np, "rotary-encoder,steps-per-period",
> +				   &pdata->steps_per_period);
> +	if (ret) {
>  		/*
>  		 * The 'half-period' property has been deprecated, you must use
>  		 * 'steps-per-period' and set an appropriate value, but we still
> -- 
> 2.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html