diff mbox series

[u-boot,04/14] mtd: rawnand: omap_gpmc: Optimize NAND reads

Message ID 20221011115012.6181-5-rogerq@kernel.org
State Accepted, archived
Commit cd72a950e0aaba0ea7e9cb1d03ff2f1ca0ca037a
Delegated to: Dario Binacchi
Headers show
Series rawnand: omap_gpmc: driver model support | expand

Commit Message

Roger Quadros Oct. 11, 2022, 11:50 a.m. UTC
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@kernel.org>
---
 drivers/mtd/nand/raw/omap_gpmc.c | 49 ++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 21 deletions(-)

Comments

Michael Trimarchi Oct. 15, 2022, 7:24 a.m. UTC | #1
Hi

On Tue, Oct 11, 2022 at 1:50 PM Roger Quadros <rogerq@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@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);
> +}
> +
>  #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

Otherwise

Reviewed-by: Michael Trimarchi <michael@amarulasolutions.com>
Roger Quadros Oct. 15, 2022, 1:29 p.m. UTC | #2
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@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@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);
>> +}
>> +
>>  #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@amarulasolutions.com>
> 
> 
Thanks!

--
cheers,
-roger
Michael Trimarchi Oct. 17, 2022, 6:39 a.m. UTC | #3
Hi Roger

On Sat, Oct 15, 2022 at 3:29 PM Roger Quadros <rogerq@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@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@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@amarulasolutions.com>
> >
> >
> Thanks!
>
> --
> cheers,
> -roger
Roger Quadros Oct. 17, 2022, 8:14 a.m. UTC | #4
Hi Michael,

On 17/10/2022 09:39, Michael Nazzareno Trimarchi wrote:
> Hi Roger
> 
> On Sat, Oct 15, 2022 at 3:29 PM Roger Quadros <rogerq@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@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@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

This will overrun the passed buffer and we don't want to take that risk?

> case of SPL you can use byte read and then can reduce
> the code size here

Apart from some legacy chips SPL size is not a huge problem. So
unless we are sure that we can actually run this driver in SPL
there is no point in doing any SPL optimizations now.

> 
> Michael

cheers,
-roger
diff mbox series

Patch

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);
+}
+
 #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;