Hi Harvey, On Tue, 17 Nov 2015 16:28:59 +0000 Harvey Hunt <harvey.h...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/