[PATCH v7, 2/3] mtd: nand: jz4780: driver for NAND devices on JZ4780 SoCs
Boris Brezillon
boris.brezillon at free-electrons.com
Tue Nov 17 11:09:01 PST 2015
Hi Harvey,
On Tue, 17 Nov 2015 16:28:59 +0000
Harvey Hunt <harvey.hunt at imgtec.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
More information about the linux-mtd
mailing list