linux-kernel - Re: [PATCH v5 2/2] memory: pl353: Add driver for arm pl353 static memory controller

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: <20150402205852.GI32500@ld-irv-0074>
Date:	Thu, 2 Apr 2015 13:58:52 -0700
From:	Brian Norris <computersforpeace@...il.com>
To:	Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@...inx.com>
Cc:	robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, galak@...eaurora.org,
	rob@...dley.net, michal.simek@...inx.com, grant.likely@...aro.org,
	gregkh@...uxfoundation.org, jason@...edaemon.net,
	ezequiel.garcia@...e-electrons.com, arnd@...db.de,
	dwmw2@...radead.org, artem.bityutskiy@...ux.intel.com,
	pekon@...com, jussi.kivilinna@....fi, acourbot@...dia.com,
	ivan.khoronzhuk@...com, joern@...fs.org,
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
	kpc528@...il.com, kalluripunnaiahchoudary@...il.com,
	Punnaiah Choudary Kalluri <punnaia@...inx.com>
Subject: Re: [PATCH v5 2/2] memory: pl353: Add driver for arm pl353 static
 memory controller

I'm not following the review of the 'memory' portions much, but has this
gotten much review? I was looking at the MTD portions when I noticed an
obvious issue below:

On Tue, Jan 06, 2015 at 11:19:17PM +0530, Punnaiah Choudary Kalluri wrote:
> Add driver for arm pl353 static memory controller. This controller is
> used in xilinx zynq soc for interfacing the nand and nor/sram memory
> devices.
> 
> Signed-off-by: Punnaiah Choudary Kalluri <punnaia@...inx.com>
> ---
> Changes in v5:
> - Added pl353_smc_get_clkrate function, made pl353_smc_set_cycles as public
>   API
> - Removed nand timing parameter initialization and moved it to nand driver  
> Changes in v4:
> - Modified driver to support multiple instances
> - Used sleep instaed of busywait for delay
> Changes in v3:
> - None
> Changes in v2:
> - Since now the timing parameters are in nano seconds, added logic to convert
>   them to the cycles
> ---
>  drivers/memory/Kconfig           |    6 +
>  drivers/memory/Makefile          |    1 +
>  drivers/memory/pl353-smc.c       |  525 ++++++++++++++++++++++++++++++++++++++
>  include/linux/memory/pl353-smc.h |   37 +++
>  4 files changed, 569 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/memory/pl353-smc.c
>  create mode 100644 include/linux/memory/pl353-smc.h
> 
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 6d91c27..9d4880f 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -85,4 +85,10 @@ config FSL_IFC
>  	bool
>  	depends on FSL_SOC
>  
> +config PL353_SMC
> +	bool "ARM PL353 Static Memory Controller(SMC) driver"

Space after 'Controller'. i.e.:

	bool "ARM PL353 Static Memory Controller (SMC) driver"

> +	default y

We almost definitely don't want this to be 'default y', especially given
that it would then be enabled on all ARM systems. Users could add it
their configuration themselves if they want it, or you might add it to
multi_v7_defconfig.

> +	depends on ARM
> +	help
> +	  This driver is for the ARM PL353 Static Memory Controller(SMC) module.

Same spacing issue here.

>  endif

[...]

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/