mbox series

[00/18] vbe: Series part D

Message ID 20240828014538.3322013-1-sjg@chromium.org
Headers show
Series vbe: Series part D | expand

Message

Simon Glass Aug. 28, 2024, 1:45 a.m. UTC
This includes various patches towards implementing the VBE abrec
bootmeth in U-Boot.


Simon Glass (18):
  sandbox: Add missing header file
  bootstd: Add stub for bootdev_setup_for_sibling_blk()
  gzip: Correct function comment for gunzip()
  fdtdec: Support separate BSS for all XPL builds
  tiny-printf: Correct return values
  tpl: Support numbered aliases in device tree
  ram: Support driver model in TPL
  serial: Support debug UART in TPL
  armv8: Support not having separate BSS
  arm: cache: Drop a stale comment
  arm: Fix up a stale comment in sections.c
  mmc: Support driver model in TPL
  mmc: Add more debugging for SPL
  mmc: Log the error when init fails
  mmc: rockchip: Log some error returns
  mmc: rockchip: Allow clocks to be missing
  rockchip: mmc: Fix a missing colon
  rockchip: Provided SPL control over efuse presence

 arch/arm/cpu/armv8/u-boot-spl.lds   | 12 ++++++++++++
 arch/arm/lib/cache.c                |  2 --
 arch/arm/lib/sections.c             |  2 +-
 arch/sandbox/include/asm/sections.h |  1 +
 common/spl/spl_mmc.c                | 13 +++++++++++++
 drivers/core/Kconfig                |  8 ++++++++
 drivers/misc/Makefile               |  2 +-
 drivers/mmc/Kconfig                 | 12 ++++++++++++
 drivers/mmc/mmc.c                   |  2 +-
 drivers/mmc/rockchip_dw_mmc.c       | 10 ++++------
 drivers/mmc/rockchip_sdhci.c        | 11 +++++------
 drivers/ram/Kconfig                 |  9 +++++++++
 drivers/serial/Kconfig              |  7 +++++++
 include/bootdev.h                   |  8 ++++++++
 include/gzip.h                      |  6 ++++--
 lib/fdtdec.c                        |  2 +-
 lib/tiny-printf.c                   | 15 ++++++---------
 17 files changed, 93 insertions(+), 29 deletions(-)

Comments

Peter Robinson Aug. 28, 2024, 9:59 a.m. UTC | #1
Hi Simon,

> This includes various patches towards implementing the VBE abrec

What is abrec?

> bootmeth in U-Boot.
>
>
> Simon Glass (18):
>   sandbox: Add missing header file
>   bootstd: Add stub for bootdev_setup_for_sibling_blk()
>   gzip: Correct function comment for gunzip()

Is this from upstream gzip code somewher?

>   fdtdec: Support separate BSS for all XPL builds
>   tiny-printf: Correct return values
>   tpl: Support numbered aliases in device tree
>   ram: Support driver model in TPL
>   serial: Support debug UART in TPL
>   armv8: Support not having separate BSS
>   arm: cache: Drop a stale comment
>   arm: Fix up a stale comment in sections.c
>   mmc: Support driver model in TPL
>   mmc: Add more debugging for SPL
>   mmc: Log the error when init fails
>   mmc: rockchip: Log some error returns
>   mmc: rockchip: Allow clocks to be missing
>   rockchip: mmc: Fix a missing colon
>   rockchip: Provided SPL control over efuse presence

I'm not sure what most of these patches have to do with VBE?

>  arch/arm/cpu/armv8/u-boot-spl.lds   | 12 ++++++++++++
>  arch/arm/lib/cache.c                |  2 --
>  arch/arm/lib/sections.c             |  2 +-
>  arch/sandbox/include/asm/sections.h |  1 +
>  common/spl/spl_mmc.c                | 13 +++++++++++++
>  drivers/core/Kconfig                |  8 ++++++++
>  drivers/misc/Makefile               |  2 +-
>  drivers/mmc/Kconfig                 | 12 ++++++++++++
>  drivers/mmc/mmc.c                   |  2 +-
>  drivers/mmc/rockchip_dw_mmc.c       | 10 ++++------
>  drivers/mmc/rockchip_sdhci.c        | 11 +++++------
>  drivers/ram/Kconfig                 |  9 +++++++++
>  drivers/serial/Kconfig              |  7 +++++++
>  include/bootdev.h                   |  8 ++++++++
>  include/gzip.h                      |  6 ++++--
>  lib/fdtdec.c                        |  2 +-
>  lib/tiny-printf.c                   | 15 ++++++---------
>  17 files changed, 93 insertions(+), 29 deletions(-)
>
> --
> 2.34.1
>
Simon Glass Aug. 28, 2024, 10:16 p.m. UTC | #2
Hi Peter,

On Wed, 28 Aug 2024 at 03:59, Peter Robinson <pbrobinson@gmail.com> wrote:
>
> Hi Simon,
>
> > This includes various patches towards implementing the VBE abrec
>
> What is abrec?

It's a VBE method which supports A, B and recovery images and permits
updating from SPL onwards. The idea is that you can safely update
without bricking a device.

>
> > bootmeth in U-Boot.
> >
> >
> > Simon Glass (18):
> >   sandbox: Add missing header file
> >   bootstd: Add stub for bootdev_setup_for_sibling_blk()
> >   gzip: Correct function comment for gunzip()
>
> Is this from upstream gzip code somewher?

I'm not sure, actually.

>
> >   fdtdec: Support separate BSS for all XPL builds

Needed so that VPL can use DDT

> >   tiny-printf: Correct return values

I can't remember, sorry.

> >   tpl: Support numbered aliases in device tree

The MMC needs to be accessed using its sequence number

> >   ram: Support driver model in TPL

That should say VPL...but again I'm not quite sure

> >   serial: Support debug UART in TPL

This allows debugging of the jump from TPL to VPL

> >   armv8: Support not having separate BSS

VPL doesn't want a separate BSS since SDRAM isn't available that early

> >   arm: cache: Drop a stale comment

Just something I noticed

> >   arm: Fix up a stale comment in sections.c

Another thing I noticed

> >   mmc: Support driver model in TPL

TPL needs to read VPL from MMC

> >   mmc: Add more debugging for SPL
> >   mmc: Log the error when init fails
> >   mmc: rockchip: Log some error returns
> >   mmc: rockchip: Allow clocks to be missing

These all help with debugging reading of VPL from the correct MMC device

> >   rockchip: mmc: Fix a missing colon

Just something I noticed

> >   rockchip: Provided SPL control over efuse presence

We don't want this driver in TPL or VPL

>
> I'm not sure what most of these patches have to do with VBE?

Basically there are a lot of little tweaks needed.

>
> >  arch/arm/cpu/armv8/u-boot-spl.lds   | 12 ++++++++++++
> >  arch/arm/lib/cache.c                |  2 --
> >  arch/arm/lib/sections.c             |  2 +-
> >  arch/sandbox/include/asm/sections.h |  1 +
> >  common/spl/spl_mmc.c                | 13 +++++++++++++
> >  drivers/core/Kconfig                |  8 ++++++++
> >  drivers/misc/Makefile               |  2 +-
> >  drivers/mmc/Kconfig                 | 12 ++++++++++++
> >  drivers/mmc/mmc.c                   |  2 +-
> >  drivers/mmc/rockchip_dw_mmc.c       | 10 ++++------
> >  drivers/mmc/rockchip_sdhci.c        | 11 +++++------
> >  drivers/ram/Kconfig                 |  9 +++++++++
> >  drivers/serial/Kconfig              |  7 +++++++
> >  include/bootdev.h                   |  8 ++++++++
> >  include/gzip.h                      |  6 ++++--
> >  lib/fdtdec.c                        |  2 +-
> >  lib/tiny-printf.c                   | 15 ++++++---------
> >  17 files changed, 93 insertions(+), 29 deletions(-)
> >
> > --
> > 2.34.1
> >

Regards,
SImon
Tom Rini Aug. 30, 2024, 12:49 a.m. UTC | #3
On Wed, Aug 28, 2024 at 04:16:55PM -0600, Simon Glass wrote:
> Hi Peter,
> 
> On Wed, 28 Aug 2024 at 03:59, Peter Robinson <pbrobinson@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > > This includes various patches towards implementing the VBE abrec
> >
> > What is abrec?
> 
> It's a VBE method which supports A, B and recovery images and permits
> updating from SPL onwards. The idea is that you can safely update
> without bricking a device.
> 
> >
> > > bootmeth in U-Boot.
> > >
> > >
> > > Simon Glass (18):
> > >   sandbox: Add missing header file
> > >   bootstd: Add stub for bootdev_setup_for_sibling_blk()
> > >   gzip: Correct function comment for gunzip()
> >
> > Is this from upstream gzip code somewher?
> 
> I'm not sure, actually.
> 
> >
> > >   fdtdec: Support separate BSS for all XPL builds
> 
> Needed so that VPL can use DDT
> 
> > >   tiny-printf: Correct return values
> 
> I can't remember, sorry.
> 
> > >   tpl: Support numbered aliases in device tree
> 
> The MMC needs to be accessed using its sequence number
> 
> > >   ram: Support driver model in TPL
> 
> That should say VPL...but again I'm not quite sure
> 
> > >   serial: Support debug UART in TPL
> 
> This allows debugging of the jump from TPL to VPL
> 
> > >   armv8: Support not having separate BSS
> 
> VPL doesn't want a separate BSS since SDRAM isn't available that early
> 
> > >   arm: cache: Drop a stale comment
> 
> Just something I noticed
> 
> > >   arm: Fix up a stale comment in sections.c
> 
> Another thing I noticed
> 
> > >   mmc: Support driver model in TPL
> 
> TPL needs to read VPL from MMC
> 
> > >   mmc: Add more debugging for SPL
> > >   mmc: Log the error when init fails
> > >   mmc: rockchip: Log some error returns
> > >   mmc: rockchip: Allow clocks to be missing
> 
> These all help with debugging reading of VPL from the correct MMC device
> 
> > >   rockchip: mmc: Fix a missing colon
> 
> Just something I noticed
> 
> > >   rockchip: Provided SPL control over efuse presence
> 
> We don't want this driver in TPL or VPL
> 
> >
> > I'm not sure what most of these patches have to do with VBE?
> 
> Basically there are a lot of little tweaks needed.

Well, I think this gets back to some common feedback. You have a lot of
little bugfixes, which is good, but they get sprinkled in to the next
big series you post, which is bad and hard to review. And that leads to
fixes not getting merged / reviewed timely because nominally simple fix
A is in part 3 of a series to introduce something larger and to which
there is feedback to work through.
Simon Glass Aug. 30, 2024, 1:06 a.m. UTC | #4
Hi Tom,

On Thu, 29 Aug 2024 at 18:49, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Aug 28, 2024 at 04:16:55PM -0600, Simon Glass wrote:
> > Hi Peter,
> >
> > On Wed, 28 Aug 2024 at 03:59, Peter Robinson <pbrobinson@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > > This includes various patches towards implementing the VBE abrec
> > >
> > > What is abrec?
> >
> > It's a VBE method which supports A, B and recovery images and permits
> > updating from SPL onwards. The idea is that you can safely update
> > without bricking a device.
> >
> > >
> > > > bootmeth in U-Boot.
> > > >
> > > >
> > > > Simon Glass (18):
> > > >   sandbox: Add missing header file
> > > >   bootstd: Add stub for bootdev_setup_for_sibling_blk()
> > > >   gzip: Correct function comment for gunzip()
> > >
> > > Is this from upstream gzip code somewher?
> >
> > I'm not sure, actually.
> >
> > >
> > > >   fdtdec: Support separate BSS for all XPL builds
> >
> > Needed so that VPL can use DDT
> >
> > > >   tiny-printf: Correct return values
> >
> > I can't remember, sorry.
> >
> > > >   tpl: Support numbered aliases in device tree
> >
> > The MMC needs to be accessed using its sequence number
> >
> > > >   ram: Support driver model in TPL
> >
> > That should say VPL...but again I'm not quite sure
> >
> > > >   serial: Support debug UART in TPL
> >
> > This allows debugging of the jump from TPL to VPL
> >
> > > >   armv8: Support not having separate BSS
> >
> > VPL doesn't want a separate BSS since SDRAM isn't available that early
> >
> > > >   arm: cache: Drop a stale comment
> >
> > Just something I noticed
> >
> > > >   arm: Fix up a stale comment in sections.c
> >
> > Another thing I noticed
> >
> > > >   mmc: Support driver model in TPL
> >
> > TPL needs to read VPL from MMC
> >
> > > >   mmc: Add more debugging for SPL
> > > >   mmc: Log the error when init fails
> > > >   mmc: rockchip: Log some error returns
> > > >   mmc: rockchip: Allow clocks to be missing
> >
> > These all help with debugging reading of VPL from the correct MMC device
> >
> > > >   rockchip: mmc: Fix a missing colon
> >
> > Just something I noticed
> >
> > > >   rockchip: Provided SPL control over efuse presence
> >
> > We don't want this driver in TPL or VPL
> >
> > >
> > > I'm not sure what most of these patches have to do with VBE?
> >
> > Basically there are a lot of little tweaks needed.
>
> Well, I think this gets back to some common feedback. You have a lot of
> little bugfixes, which is good, but they get sprinkled in to the next
> big series you post, which is bad and hard to review. And that leads to
> fixes not getting merged / reviewed timely because nominally simple fix
> A is in part 3 of a series to introduce something larger and to which
> there is feedback to work through.

I actually thought I was splitting them up quite nicely. What do you suggest?

Regards,
Simon
Tom Rini Aug. 30, 2024, 1:46 a.m. UTC | #5
On Thu, Aug 29, 2024 at 07:06:32PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 29 Aug 2024 at 18:49, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Aug 28, 2024 at 04:16:55PM -0600, Simon Glass wrote:
> > > Hi Peter,
> > >
> > > On Wed, 28 Aug 2024 at 03:59, Peter Robinson <pbrobinson@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > > This includes various patches towards implementing the VBE abrec
> > > >
> > > > What is abrec?
> > >
> > > It's a VBE method which supports A, B and recovery images and permits
> > > updating from SPL onwards. The idea is that you can safely update
> > > without bricking a device.
> > >
> > > >
> > > > > bootmeth in U-Boot.
> > > > >
> > > > >
> > > > > Simon Glass (18):
> > > > >   sandbox: Add missing header file
> > > > >   bootstd: Add stub for bootdev_setup_for_sibling_blk()
> > > > >   gzip: Correct function comment for gunzip()
> > > >
> > > > Is this from upstream gzip code somewher?
> > >
> > > I'm not sure, actually.
> > >
> > > >
> > > > >   fdtdec: Support separate BSS for all XPL builds
> > >
> > > Needed so that VPL can use DDT
> > >
> > > > >   tiny-printf: Correct return values
> > >
> > > I can't remember, sorry.
> > >
> > > > >   tpl: Support numbered aliases in device tree
> > >
> > > The MMC needs to be accessed using its sequence number
> > >
> > > > >   ram: Support driver model in TPL
> > >
> > > That should say VPL...but again I'm not quite sure
> > >
> > > > >   serial: Support debug UART in TPL
> > >
> > > This allows debugging of the jump from TPL to VPL
> > >
> > > > >   armv8: Support not having separate BSS
> > >
> > > VPL doesn't want a separate BSS since SDRAM isn't available that early
> > >
> > > > >   arm: cache: Drop a stale comment
> > >
> > > Just something I noticed
> > >
> > > > >   arm: Fix up a stale comment in sections.c
> > >
> > > Another thing I noticed
> > >
> > > > >   mmc: Support driver model in TPL
> > >
> > > TPL needs to read VPL from MMC
> > >
> > > > >   mmc: Add more debugging for SPL
> > > > >   mmc: Log the error when init fails
> > > > >   mmc: rockchip: Log some error returns
> > > > >   mmc: rockchip: Allow clocks to be missing
> > >
> > > These all help with debugging reading of VPL from the correct MMC device
> > >
> > > > >   rockchip: mmc: Fix a missing colon
> > >
> > > Just something I noticed
> > >
> > > > >   rockchip: Provided SPL control over efuse presence
> > >
> > > We don't want this driver in TPL or VPL
> > >
> > > >
> > > > I'm not sure what most of these patches have to do with VBE?
> > >
> > > Basically there are a lot of little tweaks needed.
> >
> > Well, I think this gets back to some common feedback. You have a lot of
> > little bugfixes, which is good, but they get sprinkled in to the next
> > big series you post, which is bad and hard to review. And that leads to
> > fixes not getting merged / reviewed timely because nominally simple fix
> > A is in part 3 of a series to introduce something larger and to which
> > there is feedback to work through.
> 
> I actually thought I was splitting them up quite nicely. What do you suggest?

I would refer back to your own comments about what some of the patches
do and say that "Just something I noticed" should be a one off, and
little fixes for this-and-that should be a 1-2 part series and so on.