[PATCH 2/3] rockchip: Disable DISTRO_DEFAULTS for rockpro64

[Simon Glass]

Hi,

On Thu, 16 Mar 2023 at 12:17, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Mar 16, 2023 at 11:01:01AM +0000, Peter Robinson wrote:
> > On Wed, Feb 22, 2023 at 7:17 PM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Wed, Feb 22, 2023 at 12:03:58PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 22 Feb 2023 at 12:01, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Wed, Feb 22, 2023 at 11:58:37AM -0700, Simon Glass wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Wed, 22 Feb 2023 at 11:56, Jonas Karlman <jonas at kwiboo.se> wrote:
> > > > > > >
> > > > > > > On 2023-02-22 19:24, Tom Rini wrote:
> > > > > > > > On Wed, Feb 22, 2023 at 10:19:03AM -0800, Vagrant Cascadian wrote:
> > > > > > > >> On 2023-02-21, Vagrant Cascadian wrote:
> > > > > > > >>> On 2023-02-21, Simon Glass wrote:
> > > > > > > >>>> This board has moved to standard boot but the old 'distro_bootcmd'
> > > > > > > >>>> command is still active. Disable DISTRO_DEFAULTS to fix this.
> > > > > > > >>>
> > > > > > > >>> Works for booting rockpro64-rk3399, thanks!
> > > > > > > >>
> > > > > > > >> I can also confirm that applying a very similar patch for
> > > > > > > >> pinebook-pro-rk3399 works booting with bootstd.
> > > > > > > >>
> > > > > > > >> Seems worth adding if there is a v2 of the patch series.
> > > > > > > >>
> > > > > > > >> Alternately, rather than doing this on a board-by-board basis, is there
> > > > > > > >> some way to disable CONFIG_DISTRO_DEFAULTS when a board is using
> > > > > > > >> BOOTSTD?
> > > > > > > >
> > > > > > > > I think that's possibly a bit dangerous? DISTRO_DEFAULTS and
> > > > > > > > BOOTSTD_DEFAULTS need to be reconciled to a single new symbol, and the
> > > > > > > > DISTRO_DEFAULTS method of select not imply for most things is also I
> > > > > > > > suspect the right path.
> > > > > > > >
> > > > > > >
> > > > > > > Another option would be to set BOOTSTD_BOOTCOMMAND=y, this is the main
> > > > > > > issue with rk3399 at the moment, distro_bootcmd got removed from env,
> > > > > > > yet bootcmd is still "run distro_bootcmd" due to DISTRO_DEFAULTS=y.
> > > > > > >
> > > > > > >
> > > > > > > config BOOTCOMMAND
> > > > > > >         default "bootflow scan -lb" if BOOTSTD_BOOTCOMMAND && CMD_BOOTFLOW_FULL
> > > > > > >         default "bootflow scan" if BOOTSTD_BOOTCOMMAND && !CMD_BOOTFLOW_FULL
> > > > > > >         default "run distro_bootcmd" if !BOOTSTD_BOOTCOMMAND && DISTRO_DEFAULTS
> > > > > > >
> > > > > > > config BOOTSTD_BOOTCOMMAND
> > > > > > >         default y if !DISTRO_DEFAULTS
> > > > > >
> > > > > > That seems reasonable to me, along with a 'depends on BOOTSTD'
> > > > > >
> > > > > > At present DISTRO_DEFAULTS is both enabled in defconfig files (about
> > > > > > 350) and Kconfig (another 300). For rockchip it is the latter.
> > > > > >
> > > > > > It doesn't really make sense to use DISTRO_DEFAULTS when using
> > > > > > BOOTSTD_DEFAULTS. One is 'run distro_bootcmd' and the other is
> > > > > > 'bootflow scan', possibly with -lb flags. But we really don't want
> > > > > > people flipping back and forward.
> > > > >
> > > > > It doesn't make linguistic sense to have "distro defaults" and "bootstd
> > > > > defaults" enabled.  It makes compute sense to have these two symbols
> > > > > combined as they're nearly identical and have a similar conceptual need
> > > > > (make sure we have things like bootz/booti/ext4/etc support).
> > > >
> > > > Oh, so you are talking about DISTRO_DEFAULTS, not the actual command?
> > > > In that case I can do a patch to make them both use a common symbol to
> > > > bring in most stuff, although bootstd does not need hush and a few
> > > > other things that are needed by distro_bootcmd.
> > > >
> > > > Let me know if that makes sense.
> > >
> > > Yeah, the Kconfig symbols DISTRO_DEFAULTS and BOOTSTD_DEFAULTS need to
> > > be reconciled and combined. This is separate from setting bootcmd to
> > > "run distro_bootcmd", which should be separate, and yes, not be the
> > > chosen symbol when we have bootstd enabled.
> >
> > What came out of this decision, the first other rk3399 device I tried,
> > the rock960, is also broken booting. We really can't have devices
> > randomly broken all the time and I don't have the time or the devices
> > to go through and test everything. We should really be getting better
> > at these mass changes, it doesn't lead to a great user experience!
>
> Part of the problem is that Rockchip had a few issues at the end of the
> last cycle and then the start of this one. Another issue is that I don't
> have a Rockchip platform in my CI loop, but I'm hoping to fix that in
> the next week or so.  Is top of tree working on your platforms or no?
> They should be, at this point.

This series hopes to resolve the issues:

https://patchwork.ozlabs.org/project/uboot/list/?series=344332&state=*

Regards,
Simon