linux-kernel - Re: [PATCH 1/3] mtd: spi-nand framework

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] [day] [month] [year] [list]

Message-ID: <54ADF60F.9070300@imgtec.com>
Date:	Thu, 8 Jan 2015 00:14:23 -0300
From:	Ezequiel Garcia <ezequiel.garcia@...tec.com>
To:	"Peter Pan 潘栋 (peterpandong)" 
	<peterpandong@...ron.com>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"Brian Norris" <computersforpeace@...il.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"Qi Wang 王起 (qiwang)" 
	<qiwang@...ron.com>,
	"Frank Liu 刘群 (frankliu)" 
	<frankliu@...ron.com>,
	"Melanie Zhang 张燕 (melaniezhang)" <melaniezhang@...ron.com>
Subject: Re: [PATCH 1/3] mtd: spi-nand framework



On 01/07/2015 09:48 PM, Peter Pan 潘栋 (peterpandong) wrote:
[..]
> +
> +static void spi_nand_set_defaults(struct spi_nand_chip *chip)
> +{
> +	struct spi_device *spi = chip->spi;
> +
> +	if (spi->mode & SPI_RX_QUAD)
> +		chip->read_cache = spi_nand_read_from_cache_x4;
> +	else if (spi->mode & SPI_RX_DUAL)
> +		chip->read_cache = spi_nand_read_from_cache_x2;
> +	else
> +		chip->read_cache = spi_nand_read_from_cache;
> +
> +	if (!chip->reset)
> +		chip->reset = spi_nand_reset;
> +	if (!chip->erase_block)
> +		chip->erase_block = spi_nand_erase_block;
> +	if (!chip->load_page)
> +		chip->load_page = spi_nand_read_page_to_cache;
> +	if (!chip->store_cache)
> +		chip->store_cache = spi_nand_program_data_to_cache;

IMO, this is a mistake and one of the reasons why this whole file is so big.

Do you have any reason for keeping the SPI commands (read page to cache,
program data to cache, etc.) in the spi-nand-base.c ?

This shouldn't belong to the framework, but to some spi-nand-device.c
implementing the SPI NAND commands for the devices we have seen so far.
In fact, I don't think you should inherit this "default" hook stuff from
NAND, it only makes the code more obscure.

As Brian noted, such a separation would be benefitial to support a
potential SPI-NAND-specific controller.

However, I think the stronger reason is that it results in a much
simpler and clean initial code.
-- 
Ezequiel
--
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/