linux-kernel - Re: [PATCH v7, 2/3] mtd: nand: jz4780: driver for NAND devices on JZ4780 SoCs

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: <20151117200901.675b9c58@bbrezillon>
Date:	Tue, 17 Nov 2015 20:09:01 +0100
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Harvey Hunt <harvey.hunt@...tec.com>
Cc:	<linux-mtd@...ts.infradead.org>,
	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>,
	<linux-kernel@...r.kernel.org>, Alex Smith <alex.smith@...tec.com>,
	Alex Smith <alex@...x-smith.me.uk>,
	Brian Norris <computersforpeace@...il.com>,
	David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH v7, 2/3] mtd: nand: jz4780: driver for NAND devices on
 JZ4780 SoCs

Hi Harvey,

On Tue, 17 Nov 2015 16:28:59 +0000
Harvey Hunt <harvey.hunt@...tec.com> wrote:


> >> +/* Timeout for BCH calculation/correction in microseconds. */
> >> +#define BCH_TIMEOUT			100000
> >
> > Suffixing the macro name with _MS would make it clearer.
> 
> Do you mean _US?

Yes.



> >> +static void jz4780_nand_select_chip(struct mtd_info *mtd, int chipnr)
> >> +{
> >> +	struct jz4780_nand *nand = to_jz4780_nand(mtd);
> >> +	struct jz4780_nand_chip *chip;
> >> +
> >> +	if (chipnr == -1) {
> >> +		/* Ensure the currently selected chip is deasserted. */
> >> +		if (nand->selected >= 0) {
> >> +			chip = &nand->chips[nand->selected];
> >> +			jz4780_nemc_assert(nand->dev, chip->bank, false);
> >> +		}
> >> +	} else {
> >> +		chip = &nand->chips[chipnr];
> >> +		nand->chip.IO_ADDR_R = chip->base + OFFSET_DATA;
> >> +		nand->chip.IO_ADDR_W = chip->base + OFFSET_DATA;
> >
> > How about providing helper functions that would use the nand->selected
> > + chips information to access the correct set of registers instead of
> > adapting IO_ADDR_R/IO_ADDR_W values?
> 
> I'm not sure what you mean - are you suggesting something such as:
> 
> u8 *jz4780_nand_read_io_line(struct jz4780_nand *nand, unsigned int off)
> {
> 	return readb(&nand->chips[nand->selected]->base + off);
> }
> 
> Does the NAND core code not make use of IO_ADDR_{W,R}?

Right, I missed that. It's used to implement the default
nand_read/write_buf/byte/word() functions, but I still think it would be
preferable to implement your own read/write_xxx() functions than using
those fields, but that's up to you.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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/