linux-kernel - Re: [PATCH v3 20/36] mtd: st_spi_fsm: Provide a method to put the chip into 32bit addressing mode

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 10 Dec 2013 17:49:21 -0800
From:	Brian Norris <computersforpeace@...il.com>
To:	Lee Jones <lee.jones@...aro.org>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	dwmw2@...radead.org, angus.clark@...com, linus.walleij@...aro.org,
	linux-mtd@...ts.infradead.org
Subject: Re: [PATCH v3 20/36] mtd: st_spi_fsm: Provide a method to put the
 chip into 32bit addressing mode

On Fri, Nov 29, 2013 at 12:19:09PM +0000, Lee Jones wrote:
> --- a/drivers/mtd/devices/st_spi_fsm.c
> +++ b/drivers/mtd/devices/st_spi_fsm.c
> @@ -28,6 +28,8 @@
>  #include "st_spi_fsm.h"
>  #include "serial_flash_cmds.h"
>  
> +static struct stfsm_seq stfsm_seq_en_32bit_addr;/* Dynamically populated */

You need a space between the semicolon and the comment.

But why isn't this struct just embedded in the struct stfsm? I don't
like static instances like this, especially when they are dynamically
configured (as your comment notes). In the crazy even that we ever have
two of these FSMs on an SoC (can this happen?), you'll be introducing a
race on needlessly-shared data.

The same comment applies to your later structs stfsm_seq_read and
stfsm_seq_write.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/