Re: [PATCH] loads: Block writes into LMB reserved areas of U-Boot

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

On Fri, Oct 15, 2021 at 04:23:27PM +0200, Marek Vasut wrote:
> On 10/14/21 5:10 PM, Simon Glass wrote:
> [...]
> > > @@ -137,6 +138,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
> > > 
> > >   static ulong load_serial(long offset)
> > >   {
> > > +       struct lmb lmb;
> > >          char    record[SREC_MAXRECLEN + 1];     /* buffer for one S-Record      */
> > >          char    binbuf[SREC_MAXBINLEN];         /* buffer for binary data       */
> > >          int     binlen;                         /* no. of data bytes in S-Rec.  */
> > > @@ -147,6 +149,9 @@ static ulong load_serial(long offset)
> > >          ulong   start_addr = ~0;
> > >          ulong   end_addr   =  0;
> > >          int     line_count =  0;
> > > +       long ret;
> > > +
> > > +       lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> > > 
> > >          while (read_record(record, SREC_MAXRECLEN + 1) >= 0) {
> > >                  type = srec_decode(record, &binlen, &addr, binbuf);
> > > @@ -172,7 +177,14 @@ static ulong load_serial(long offset)
> > >                      } else
> > >   #endif
> > >                      {
> > > +                       ret = lmb_reserve(&lmb, store_addr, binlen);
> > > +                       if (ret) {
> > > +                               printf("\nCannot overwrite reserved area (%08lx..%08lx)\n",
> > > +                                       store_addr, store_addr + binlen);
> > > +                               return ret;
> > > +                       }
> > >                          memcpy((char *)(store_addr), binbuf, binlen);
> > > +                       lmb_free(&lmb, store_addr, binlen);
> > >                      }
> > >                      if ((store_addr) < start_addr)
> > >                          start_addr = store_addr;
> > > --
> > > 2.33.0
> > > 
> > 
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > This code looks OK but I don't know what lmb_reserve() and lmb_free()
> > do. Can you add comments to the header file?
> 
> Not here, the entire LMB stuff needs (better) documentation, that's where
> (all) such clarification should go.

Is that you saying you'll do such a follow-up patch, given you've
touched this code the most of late?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]