[u-boot][PATCH 04/14] mtd: rawnand: omap_gpmc: Optimize NAND reads
Michael Nazzareno Trimarchi
michael at amarulasolutions.com
Mon Oct 17 08:39:31 CEST 2022
Hi Roger
On Sat, Oct 15, 2022 at 3:29 PM Roger Quadros <rogerq at kernel.org> wrote:
>
> Hi Michael,
>
> On 15/10/2022 10:24, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Tue, Oct 11, 2022 at 1:50 PM Roger Quadros <rogerq at kernel.org> wrote:
> >>
> >> Rename omap_nand_read() to omap_nand_read_buf() to reflect
> >> actual behaviour.
> >>
> >> Use FIFO read address instead of raw read address for reads.
> >>
> >> The GPMC automatically converts 32-bit/16-bit reads to NAND
> >> device specific reads (8/16 bit). Use the largest possible
> >> read granularity size for more efficient reads.
> >>
> >> Signed-off-by: Roger Quadros <rogerq at kernel.org>
> >> ---
> >> drivers/mtd/nand/raw/omap_gpmc.c | 49 ++++++++++++++++++--------------
> >> 1 file changed, 28 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
> >> index d62c3e6fce..b36fe762b3 100644
> >> --- a/drivers/mtd/nand/raw/omap_gpmc.c
> >> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
> >> @@ -55,6 +55,7 @@ struct omap_nand_info {
> >> enum omap_ecc ecc_scheme;
> >> uint8_t cs;
> >> uint8_t ws; /* wait status pin (0,1) */
> >> + void __iomem *fifo;
> >> };
> >>
> >> /* We are wasting a bit of memory but al least we are safe */
> >> @@ -350,6 +351,20 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat,
> >> return 0;
> >> }
> >>
> >> +static inline void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> >> +{
> >> + struct nand_chip *chip = mtd_to_nand(mtd);
> >> + struct omap_nand_info *info = nand_get_controller_data(chip);
> >> + u32 alignment = ((uintptr_t)buf | len) & 3;
> >> +
> >> + if (alignment & 1)
> >> + readsb(info->fifo, buf, len);
> >> + else if (alignment & 3)
> >> + readsw(info->fifo, buf, len >> 1);
> >> + else
> >> + readsl(info->fifo, buf, len >> 2);
> >> +}
> >> +
Can you optimize more I think here.You can consider the disaligment
portion and then read at max len. You read more
than the actual lens but I think it should not be a big problem. In
case of SPL you can use byte read and then can reduce
the code size here
Michael
> >> #ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
> >>
> >> #define PREFETCH_CONFIG1_CS_SHIFT 24
> >> @@ -415,7 +430,7 @@ static int __read_prefetch_aligned(struct nand_chip *chip, uint32_t *buf, int le
> >> cnt = PREFETCH_STATUS_FIFO_CNT(cnt);
> >>
> >> for (i = 0; i < cnt / 4; i++) {
> >> - *buf++ = readl(CONFIG_SYS_NAND_BASE);
> >> + *buf++ = readl(info->fifo);
> >> len -= 4;
> >> }
> >> } while (len);
> >> @@ -425,16 +440,6 @@ static int __read_prefetch_aligned(struct nand_chip *chip, uint32_t *buf, int le
> >> return 0;
> >> }
> >>
> >> -static inline void omap_nand_read(struct mtd_info *mtd, uint8_t *buf, int len)
> >> -{
> >> - struct nand_chip *chip = mtd_to_nand(mtd);
> >> -
> >> - if (chip->options & NAND_BUSWIDTH_16)
> >> - nand_read_buf16(mtd, buf, len);
> >> - else
> >> - nand_read_buf(mtd, buf, len);
> >> -}
> >> -
> >> static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int len)
> >> {
> >> int ret;
> >> @@ -447,7 +452,7 @@ static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int len)
> >> */
> >> head = ((uintptr_t)buf) % 4;
> >> if (head) {
> >> - omap_nand_read(mtd, buf, head);
> >> + omap_nand_read_buf(mtd, buf, head);
> >> buf += head;
> >> len -= head;
> >> }
> >> @@ -461,10 +466,10 @@ static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int len)
> >> ret = __read_prefetch_aligned(chip, (uint32_t *)buf, len - tail);
> >> if (ret < 0) {
> >> /* fallback in case the prefetch engine is busy */
> >> - omap_nand_read(mtd, buf, len);
> >> + omap_nand_read_buf(mtd, buf, len);
> >> } else if (tail) {
> >> buf += len - tail;
> >> - omap_nand_read(mtd, buf, tail);
> >> + omap_nand_read_buf(mtd, buf, tail);
> >> }
> >> }
> >> #endif /* CONFIG_NAND_OMAP_GPMC_PREFETCH */
> >> @@ -1001,6 +1006,8 @@ int board_nand_init(struct nand_chip *nand)
> >> int32_t gpmc_config = 0;
> >> int cs = cs_next++;
> >> int err = 0;
> >> + struct omap_nand_info *info;
> >> +
> >> /*
> >> * xloader/Uboot's gpmc configuration would have configured GPMC for
> >> * nand type of memory. The following logic scans and latches on to the
> >> @@ -1029,9 +1036,12 @@ int board_nand_init(struct nand_chip *nand)
> >>
> >> nand->IO_ADDR_R = (void __iomem *)&gpmc_cfg->cs[cs].nand_dat;
> >> nand->IO_ADDR_W = (void __iomem *)&gpmc_cfg->cs[cs].nand_cmd;
> >> - omap_nand_info[cs].control = NULL;
> >> - omap_nand_info[cs].cs = cs;
> >> - omap_nand_info[cs].ws = wscfg[cs];
> >> +
> >> + info = &omap_nand_info[cs];
> >> + info->control = NULL;
> >> + info->cs = cs;
> >> + info->ws = wscfg[cs];
> >> + info->fifo = (void __iomem *)CONFIG_SYS_NAND_BASE;
> >> nand_set_controller_data(nand, &omap_nand_info[cs]);
> >> nand->cmd_ctrl = omap_nand_hwcontrol;
> >> nand->options |= NAND_NO_PADDING | NAND_CACHEPRG;
> >> @@ -1062,10 +1072,7 @@ int board_nand_init(struct nand_chip *nand)
> >> #ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
> >> nand->read_buf = omap_nand_read_prefetch;
> >> #else
> >> - if (nand->options & NAND_BUSWIDTH_16)
> >> - nand->read_buf = nand_read_buf16;
> >> - else
> >> - nand->read_buf = nand_read_buf;
> >> + nand->read_buf = omap_nand_read_buf;
> >> #endif
> >>
> >> nand->dev_ready = omap_dev_ready;
> >> --
> >> 2.17.1
> >>
> >
> > Is possible for you to split in use of fifo and optimize read/alignment
>
> Sure, I'll remember to do that when I re-spin this series.
>
> >
> > Otherwise
> >
> > Reviewed-by: Michael Trimarchi <michael at amarulasolutions.com>
> >
> >
> Thanks!
>
> --
> cheers,
> -roger
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com
More information about the U-Boot
mailing list