Messages in this thread | | | Date | Tue, 17 Nov 2015 20:09:01 +0100 | From | Boris Brezillon <> | 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@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
| |