linux-kernel - Re: [PATCH v9 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200708125438.GB5221@shinobu>
Date:   Wed, 8 Jul 2020 08:54:38 -0400
From:   William Breathitt Gray <vilhelm.gray@...il.com>
To:     Syed Nayyar Waris <syednwaris@...il.com>
Cc:     linus.walleij@...aro.org, andriy.shevchenko@...ux.intel.com,
        bgolaszewski@...libre.com, michal.simek@...inx.com,
        linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 4/4] gpio: xilinx: Utilize generic bitmap_get_value
 and _set_value.

On Sat, Jun 27, 2020 at 01:44:23PM +0530, Syed Nayyar Waris wrote:
> This patch reimplements the xgpio_set_multiple function in
> drivers/gpio/gpio-xilinx.c to use the new generic functions:
> bitmap_get_value and bitmap_set_value. The code is now simpler
> to read and understand. Moreover, instead of looping for each bit
> in xgpio_set_multiple function, now we can check each channel at
> a time and save cycles.
> ---
> Changes in v9:
>  - Remove looping of 'for_each_set_clump' and instead process two
>    halves of a 64-bit bitmap separately or individually. Use normal spin_lock 
>    call for second inner lock. And take the spin_lock_init call outside the 'if'
>    condition in the 'probe' function of driver.
> 
> Changes in v8:
>  - No change.
> 
> Changes in v7:
>  - No change.
> 
> Changes in v6:
>  - No change.
> 
> Changes in v5:
>  - Minor change: Inline values '32' and '64' in code for better
>    code readability.
> 
> Changes in v4:
>  - Minor change: Inline values '32' and '64' in code for better
>    code readability.
> 
> Changes in v3:
>  - No change.
> 
> Changes in v2:
>  - No change
> 
>  drivers/gpio/gpio-xilinx.c | 66 +++++++++++++++++++-------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c
> index 67f9f82e0db0..48393d06fb55 100644
> --- a/drivers/gpio/gpio-xilinx.c
> +++ b/drivers/gpio/gpio-xilinx.c
> @@ -136,39 +136,39 @@ static void xgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>  static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
>  			       unsigned long *bits)
>  {
> -	unsigned long flags;
> +	unsigned long flag;

Hi Syed,

I have a couple minor suggestions in case you submit a version 10. Leave
this variable named "flags" because it refers to CPU flags which will
likely be more than one flag anyway.

> @@ -292,6 +292,7 @@ static int xgpio_probe(struct platform_device *pdev)
>  		chip->gpio_width[0] = 32;
>  
>  	spin_lock_init(&chip->gpio_lock[0]);
> +	spin_lock_init(&chip->gpio_lock[1]);
>  
>  	if (of_property_read_u32(np, "xlnx,is-dual", &is_dual))
>  		is_dual = 0;
> @@ -314,7 +315,6 @@ static int xgpio_probe(struct platform_device *pdev)
>  					 &chip->gpio_width[1]))
>  			chip->gpio_width[1] = 32;
>  

Remove this whitespace as well.

Thanks,

William Breathitt Gray

> -		spin_lock_init(&chip->gpio_lock[1]);
>  	}
>  
>  	chip->gc.base = -1;
> -- 
> 2.26.2
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)