Hi Bin, On Thu, 10 Oct 2019 at 00:23, Bin Meng <bmeng...@gmail.com> wrote: > > Hi Simon, > > On Wed, Sep 25, 2019 at 10:59 PM Simon Glass <s...@chromium.org> wrote: > > > > This should take account of the end of the new cache record since a record > > cannot extend beyond the end of the flash region. This problem was not > > seen before due to the alignment of the relatively small amount of MRC > > data. > > > > But with apollolake the MRC data is about 45KB, even if most of it is > > zeroes. > > > > Fix this bug and update the parameter name to be less confusing. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > arch/x86/lib/mrccache.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/lib/mrccache.c b/arch/x86/lib/mrccache.c > > index 33bb52039bd..e286bdf1b30 100644 > > --- a/arch/x86/lib/mrccache.c > > +++ b/arch/x86/lib/mrccache.c > > @@ -86,15 +86,16 @@ struct mrc_data_container *mrccache_find_current(struct > > mrc_region *entry) > > * @return next cache entry if found, NULL if we got to the end > > */ > > static struct mrc_data_container *find_next_mrc_cache(struct mrc_region > > *entry, > > - struct mrc_data_container *cache) > > + struct mrc_data_container *prev) > > { > > + struct mrc_data_container *cache; > > ulong base_addr, end_addr; > > > > base_addr = entry->base + entry->offset; > > end_addr = base_addr + entry->length; > > > > - cache = next_mrc_block(cache); > > - if ((ulong)cache >= end_addr) { > > + cache = next_mrc_block(prev); > > + if ((ulong)cache + mrc_block_size(prev->data_size) > end_addr) { > > This does not look good to me. Why adding the "next" cache position to > "prev" cache size? It should add the "next" cache size.
Yes, although here we assume they are the same. Should I add a comment? Alternatively I could pass in the size that the caller wants for the new item? > > I agree there is an issue in missing check of boundary, but the check > should not happen here, but mrccache_update(), before writing to SPI > flash. OK, so passing in the size would be best, I suspect. Perhaps rename the function to find_next_mrc_cache_pos()? > > > /* Crossed the boundary */ > > cache = NULL; > > debug("%s: no available entries found\n", __func__); > > -- > Regards, SImon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot