[U-Boot] [PATCH 4/4] gpio: xilinx: Simplify logic in xilinx_gpio_set_value

Stefan Herbrechtsmeier stefan at herbrechtsmeier.net
Mon Jul 30 13:31:00 UTC 2018


Hi Michal,

Am 30.07.2018 um 14:34 schrieb Michal Simek:
> There is no reason to do read/write for if/else separately.
>
> Reported-by: Stefan Herbrechtsmeier <stefan at herbrechtsmeier.net>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
>   drivers/gpio/xilinx_gpio.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/xilinx_gpio.c b/drivers/gpio/xilinx_gpio.c
> index 1e5f3da8d7e8..cccfa7561739 100644
> --- a/drivers/gpio/xilinx_gpio.c
> +++ b/drivers/gpio/xilinx_gpio.c
> @@ -61,18 +61,17 @@ static int xilinx_gpio_set_value(struct udevice *dev, unsigned offset,
>   	if (ret)
>   		return ret;
>   
> +	val = readl(&platdata->regs->gpiodata + bank * 2);
> +
>   	debug("%s: regs: %lx, value: %x, gpio: %x, bank %x, pin %x\n",
>   	      __func__, (ulong)platdata->regs, value, offset, bank, pin);
>   
> -	if (value) {
> -		val = readl(&platdata->regs->gpiodata + bank * 2);
> +	if (value)
>   		val = val | (1 << pin);
> -		writel(val, &platdata->regs->gpiodata + bank * 2);
> -	} else {
> -		val = readl(&platdata->regs->gpiodata + bank * 2);
> +	else
>   		val = val & ~(1 << pin);
> -		writel(val, &platdata->regs->gpiodata + bank * 2);
> -	}
> +
> +	writel(val, &platdata->regs->gpiodata + bank * 2);
>   
>   	return val;
>   };

This doesn't work because you read the general purpose input ports 
(register) and write to the general purpose output ports (register). 
This leads to problems if an output only pin inside an bidirectional 
port has no connection between the two registers or the input and output 
registers has different values.

The value of the output register must be saved inside the driver.

Best regards
   Stefan





More information about the U-Boot mailing list