mbox series

[RESEND,v7,00/11] rockchip: Add new rk3399 boards

Message ID 20190508054151.21762-1-jagan@amarulasolutions.com (mailing list archive)
Headers show
Series rockchip: Add new rk3399 boards | expand

Message

Jagan Teki May 8, 2019, 5:41 a.m. UTC
(Sorry for the noice, I have missed to send two patches from v7)

This is v7 resend patchset for New rk3399 boards support wrt previous
version[1]

Unfortunately initial version of creating rk3399-u-boot.dtsi and 
orangepi rk3399 changes are merged, so this is rework on top of 
u-boot-rockchip/master.

Overall this series add support below rk3399 boards
- NanoPI M4
- NanoPC T4
- NanoPI NEO4
- Orangepi RK3399
- Rock PI 4
- Rockpro64

All the respective dts(i) files are synced from Linux 5.1-rc2 and few
dts(i) from linux-next.

SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
series [3].

Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
booting via Rockchip miniloader as of now.

For booting the same with SPL NEO4 would require dynamic dram timing
detection and rest require LPDDR4 code. There is WIP[2] for these
dependencies and this would require big chunk of changes will effect
all the rk3399 boards, so I'm planning to mark it for next MW. 

Changes for v7:
- rebase on top of u-boot-rockchip/master
- add SPL_TEXT_BASE on each board defconfig
- rebase on required changes
Changes for v6:
- Include Nanopc T4 support patch
- drop rk3399-u-boot.dtsi patch since it is send separately.
Changes for v5:
- Make all changes related to move sdmmc, spi1 u-boot,dm-pre-reloc
  properties into all rk3399 dts(i) files.
Changes for v4:
- don't include existing dts(i) sdmmc, u-boot,dm-pre-reloc into
  rk3399-u-boot.dtsi
Changes for v3:
- drop NanoPC T4 for now, since board is yet to receive.
- add Rock PI-4 board.
- add separate -u-boot.dtsi file for nanopi4 sdram changes.
- collect Paul, Philipp and Kever Reviewed-by tags

Travis-CI:
https://travis-ci.org/openedev/u-boot-amarula/builds/529284236

[1] https://patchwork.ozlabs.org/cover/1096473/
[2] https://github.com/amarula/u-boot-amarula/tree/rockdev-lpddr4
[3] https://patchwork.ozlabs.org/cover/1091909/

Any inputs?
Jagan.

Jagan Teki (11):
  rockchip: dts: rk3399: Sync pwm2_pin_pull_down from Linux 5.1-rc2
  Kconfig: Add default SPL_FIT_GENERATOR for rockchip
  arm: rockchip: rk3399: Move common configs in Kconfig
  rockchip: dts: rk3399: Sync rk3399-nanopi4.dtsi from Linux
  rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1
  rockchip: rk3399: Add Nanopi M4 board support
  rockchip: rk3399: Add Nanopc T4 board support
  rockchip: rk3399: Add Nanopi NEO4 board support
  rockchip: rk3399: Add Rockpro64 board support
  rockchip: rk3399: Add Rock PI 4 support
  doc: rockchip: Add global doc for rk3399 build/flash

 Kconfig                                     |   1 +
 arch/arm/dts/Makefile                       |   5 +
 arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi   |   7 +
 arch/arm/dts/rk3399-nanopc-t4.dts           |  91 +++
 arch/arm/dts/rk3399-nanopi-m4-u-boot.dtsi   |   7 +
 arch/arm/dts/rk3399-nanopi-m4.dts           |  66 ++
 arch/arm/dts/rk3399-nanopi-neo4-u-boot.dtsi |   6 +
 arch/arm/dts/rk3399-nanopi-neo4.dts         |  50 ++
 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi     |  11 +
 arch/arm/dts/rk3399-nanopi4.dtsi            | 703 +++++++++++++++++++
 arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi   |   6 +
 arch/arm/dts/rk3399-rock-pi-4.dts           | 606 +++++++++++++++++
 arch/arm/dts/rk3399-rockpro64-u-boot.dtsi   |   6 +
 arch/arm/dts/rk3399-rockpro64.dts           | 712 ++++++++++++++++++++
 arch/arm/dts/rk3399.dtsi                    |   5 +
 arch/arm/mach-rockchip/Kconfig              |  16 +
 board/rockchip/evb_rk3399/MAINTAINERS       |  32 +
 configs/chromebook_bob_defconfig            |  17 -
 configs/evb-rk3399_defconfig                |  17 -
 configs/ficus-rk3399_defconfig              |  17 -
 configs/firefly-rk3399_defconfig            |  17 -
 configs/nanopc-t4-rk3399_defconfig          |  59 ++
 configs/nanopi-m4-rk3399_defconfig          |  59 ++
 configs/nanopi-neo4-rk3399_defconfig        |  59 ++
 configs/orangepi-rk3399_defconfig           |  17 -
 configs/puma-rk3399_defconfig               |  16 -
 configs/rock-pi-4-rk3399_defconfig          |  59 ++
 configs/rock960-rk3399_defconfig            |  17 -
 configs/rockpro64-rk3399_defconfig          |  59 ++
 doc/README.rockchip                         | 233 ++++++-
 30 files changed, 2857 insertions(+), 119 deletions(-)
 create mode 100644 arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-nanopc-t4.dts
 create mode 100644 arch/arm/dts/rk3399-nanopi-m4-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-nanopi-m4.dts
 create mode 100644 arch/arm/dts/rk3399-nanopi-neo4-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-nanopi-neo4.dts
 create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-nanopi4.dtsi
 create mode 100644 arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-rock-pi-4.dts
 create mode 100644 arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
 create mode 100644 arch/arm/dts/rk3399-rockpro64.dts
 create mode 100644 configs/nanopc-t4-rk3399_defconfig
 create mode 100644 configs/nanopi-m4-rk3399_defconfig
 create mode 100644 configs/nanopi-neo4-rk3399_defconfig
 create mode 100644 configs/rock-pi-4-rk3399_defconfig
 create mode 100644 configs/rockpro64-rk3399_defconfig

Comments

Paul Kocialkowski May 9, 2019, 7:07 a.m. UTC | #1
Hi,

On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> (Sorry for the noice, I have missed to send two patches from v7)
> 
> This is v7 resend patchset for New rk3399 boards support wrt previous
> version[1]
> 
> Unfortunately initial version of creating rk3399-u-boot.dtsi and 
> orangepi rk3399 changes are merged, so this is rework on top of 
> u-boot-rockchip/master.
> 
> Overall this series add support below rk3399 boards
> - NanoPI M4
> - NanoPC T4
> - NanoPI NEO4
> - Orangepi RK3399
> - Rock PI 4
> - Rockpro64
> 
> All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> dts(i) from linux-next.
> 
> SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> series [3].
> 
> Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> booting via Rockchip miniloader as of now.

Could you send these two boards in a separate series so that we avoid
merging them for now (because SPL support is broken) and then re-
iterate the series later with the DDR bringup? Or maybe find a way to
disable SPL support, but in any case, it's not ok to merge a board with
SPL enabled and broken.

Cheers,

Paul

> For booting the same with SPL NEO4 would require dynamic dram timing
> detection and rest require LPDDR4 code. There is WIP[2] for these
> dependencies and this would require big chunk of changes will effect
> all the rk3399 boards, so I'm planning to mark it for next MW. 
> 
> Changes for v7:
> - rebase on top of u-boot-rockchip/master
> - add SPL_TEXT_BASE on each board defconfig
> - rebase on required changes
> Changes for v6:
> - Include Nanopc T4 support patch
> - drop rk3399-u-boot.dtsi patch since it is send separately.
> Changes for v5:
> - Make all changes related to move sdmmc, spi1 u-boot,dm-pre-reloc
>   properties into all rk3399 dts(i) files.
> Changes for v4:
> - don't include existing dts(i) sdmmc, u-boot,dm-pre-reloc into
>   rk3399-u-boot.dtsi
> Changes for v3:
> - drop NanoPC T4 for now, since board is yet to receive.
> - add Rock PI-4 board.
> - add separate -u-boot.dtsi file for nanopi4 sdram changes.
> - collect Paul, Philipp and Kever Reviewed-by tags
> 
> Travis-CI:
> https://travis-ci.org/openedev/u-boot-amarula/builds/529284236
> 
> [1] https://patchwork.ozlabs.org/cover/1096473/
> [2] https://github.com/amarula/u-boot-amarula/tree/rockdev-lpddr4
> [3] https://patchwork.ozlabs.org/cover/1091909/
> 
> Any inputs?
> Jagan.
> 
> Jagan Teki (11):
>   rockchip: dts: rk3399: Sync pwm2_pin_pull_down from Linux 5.1-rc2
>   Kconfig: Add default SPL_FIT_GENERATOR for rockchip
>   arm: rockchip: rk3399: Move common configs in Kconfig
>   rockchip: dts: rk3399: Sync rk3399-nanopi4.dtsi from Linux
>   rockchip: dts: rk3399: nanopi4: Use CD pin as RK_FUNC_1
>   rockchip: rk3399: Add Nanopi M4 board support
>   rockchip: rk3399: Add Nanopc T4 board support
>   rockchip: rk3399: Add Nanopi NEO4 board support
>   rockchip: rk3399: Add Rockpro64 board support
>   rockchip: rk3399: Add Rock PI 4 support
>   doc: rockchip: Add global doc for rk3399 build/flash
> 
>  Kconfig                                     |   1 +
>  arch/arm/dts/Makefile                       |   5 +
>  arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi   |   7 +
>  arch/arm/dts/rk3399-nanopc-t4.dts           |  91 +++
>  arch/arm/dts/rk3399-nanopi-m4-u-boot.dtsi   |   7 +
>  arch/arm/dts/rk3399-nanopi-m4.dts           |  66 ++
>  arch/arm/dts/rk3399-nanopi-neo4-u-boot.dtsi |   6 +
>  arch/arm/dts/rk3399-nanopi-neo4.dts         |  50 ++
>  arch/arm/dts/rk3399-nanopi4-u-boot.dtsi     |  11 +
>  arch/arm/dts/rk3399-nanopi4.dtsi            | 703 +++++++++++++++++++
>  arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi   |   6 +
>  arch/arm/dts/rk3399-rock-pi-4.dts           | 606 +++++++++++++++++
>  arch/arm/dts/rk3399-rockpro64-u-boot.dtsi   |   6 +
>  arch/arm/dts/rk3399-rockpro64.dts           | 712 ++++++++++++++++++++
>  arch/arm/dts/rk3399.dtsi                    |   5 +
>  arch/arm/mach-rockchip/Kconfig              |  16 +
>  board/rockchip/evb_rk3399/MAINTAINERS       |  32 +
>  configs/chromebook_bob_defconfig            |  17 -
>  configs/evb-rk3399_defconfig                |  17 -
>  configs/ficus-rk3399_defconfig              |  17 -
>  configs/firefly-rk3399_defconfig            |  17 -
>  configs/nanopc-t4-rk3399_defconfig          |  59 ++
>  configs/nanopi-m4-rk3399_defconfig          |  59 ++
>  configs/nanopi-neo4-rk3399_defconfig        |  59 ++
>  configs/orangepi-rk3399_defconfig           |  17 -
>  configs/puma-rk3399_defconfig               |  16 -
>  configs/rock-pi-4-rk3399_defconfig          |  59 ++
>  configs/rock960-rk3399_defconfig            |  17 -
>  configs/rockpro64-rk3399_defconfig          |  59 ++
>  doc/README.rockchip                         | 233 ++++++-
>  30 files changed, 2857 insertions(+), 119 deletions(-)
>  create mode 100644 arch/arm/dts/rk3399-nanopc-t4-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-nanopc-t4.dts
>  create mode 100644 arch/arm/dts/rk3399-nanopi-m4-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-nanopi-m4.dts
>  create mode 100644 arch/arm/dts/rk3399-nanopi-neo4-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-nanopi-neo4.dts
>  create mode 100644 arch/arm/dts/rk3399-nanopi4-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-nanopi4.dtsi
>  create mode 100644 arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-rock-pi-4.dts
>  create mode 100644 arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>  create mode 100644 arch/arm/dts/rk3399-rockpro64.dts
>  create mode 100644 configs/nanopc-t4-rk3399_defconfig
>  create mode 100644 configs/nanopi-m4-rk3399_defconfig
>  create mode 100644 configs/nanopi-neo4-rk3399_defconfig
>  create mode 100644 configs/rock-pi-4-rk3399_defconfig
>  create mode 100644 configs/rockpro64-rk3399_defconfig
>
Jagan Teki May 9, 2019, 10:45 a.m. UTC | #2
Hi Paul,

On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > (Sorry for the noice, I have missed to send two patches from v7)
> >
> > This is v7 resend patchset for New rk3399 boards support wrt previous
> > version[1]
> >
> > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > orangepi rk3399 changes are merged, so this is rework on top of
> > u-boot-rockchip/master.
> >
> > Overall this series add support below rk3399 boards
> > - NanoPI M4
> > - NanoPC T4
> > - NanoPI NEO4
> > - Orangepi RK3399
> > - Rock PI 4
> > - Rockpro64
> >
> > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > dts(i) from linux-next.
> >
> > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > series [3].
> >
> > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > booting via Rockchip miniloader as of now.
>
> Could you send these two boards in a separate series so that we avoid
> merging them for now (because SPL support is broken) and then re-
> iterate the series later with the DDR bringup? Or maybe find a way to
> disable SPL support, but in any case, it's not ok to merge a board with
> SPL enabled and broken.

I have explained the details about this concern on v2 [1], thought you
would comeback on the same line instead here.

Anyway, making SPL as an optional is not an idea to go with Mainline
as we make many decisions with regards to make SPL is mandatory.
Since the DDR is show stopper here (and it would really need a good
amount of time, since it effect the other boards), I can go with TPL
enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
booting stages. This way we can avoid skipping SPL usage, and many
config changes to make SPL optional.

[1] https://patchwork.ozlabs.org/cover/1086213/
Paul Kocialkowski May 9, 2019, 12:30 p.m. UTC | #3
Hi,

On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
> Hi Paul,
> 
> On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > > (Sorry for the noice, I have missed to send two patches from v7)
> > > 
> > > This is v7 resend patchset for New rk3399 boards support wrt previous
> > > version[1]
> > > 
> > > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > > orangepi rk3399 changes are merged, so this is rework on top of
> > > u-boot-rockchip/master.
> > > 
> > > Overall this series add support below rk3399 boards
> > > - NanoPI M4
> > > - NanoPC T4
> > > - NanoPI NEO4
> > > - Orangepi RK3399
> > > - Rock PI 4
> > > - Rockpro64
> > > 
> > > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > > dts(i) from linux-next.
> > > 
> > > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > > series [3].
> > > 
> > > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > > booting via Rockchip miniloader as of now.
> > 
> > Could you send these two boards in a separate series so that we avoid
> > merging them for now (because SPL support is broken) and then re-
> > iterate the series later with the DDR bringup? Or maybe find a way to
> > disable SPL support, but in any case, it's not ok to merge a board with
> > SPL enabled and broken.
> 
> I have explained the details about this concern on v2 [1], thought you
> would comeback on the same line instead here.

Yes, you have already explained the issue, but I don't think it's
enough a justification to merge broken SPL support. If it was only
partial or limited, it would be fine, but here it's just broken.

> Anyway, making SPL as an optional is not an idea to go with Mainline
> as we make many decisions with regards to make SPL is mandatory.

Yes I think making SPL mandatory is a good idea, so that's why I'm
suggesting that we don't merge the boards until they have SPL support.

> Since the DDR is show stopper here (and it would really need a good
> amount of time, since it effect the other boards), I can go with TPL
> enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> booting stages. This way we can avoid skipping SPL usage, and many
> config changes to make SPL optional.

Honestly I don't really see the point of merging these boards at all if
they don't have SPL support. People who really want to use them with
the rockchip blob can cherry-pick the patches from the list in the
meantime.

It also creates incentive for people to free the DDR init, since that
becomes a condition to have the board upstream.

What do you think?

Cheers,

Paul
Jagan Teki May 9, 2019, 12:36 p.m. UTC | #4
On Thu, May 9, 2019 at 6:01 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
> > Hi Paul,
> >
> > On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > > > (Sorry for the noice, I have missed to send two patches from v7)
> > > >
> > > > This is v7 resend patchset for New rk3399 boards support wrt previous
> > > > version[1]
> > > >
> > > > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > > > orangepi rk3399 changes are merged, so this is rework on top of
> > > > u-boot-rockchip/master.
> > > >
> > > > Overall this series add support below rk3399 boards
> > > > - NanoPI M4
> > > > - NanoPC T4
> > > > - NanoPI NEO4
> > > > - Orangepi RK3399
> > > > - Rock PI 4
> > > > - Rockpro64
> > > >
> > > > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > > > dts(i) from linux-next.
> > > >
> > > > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > > > series [3].
> > > >
> > > > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > > > booting via Rockchip miniloader as of now.
> > >
> > > Could you send these two boards in a separate series so that we avoid
> > > merging them for now (because SPL support is broken) and then re-
> > > iterate the series later with the DDR bringup? Or maybe find a way to
> > > disable SPL support, but in any case, it's not ok to merge a board with
> > > SPL enabled and broken.
> >
> > I have explained the details about this concern on v2 [1], thought you
> > would comeback on the same line instead here.
>
> Yes, you have already explained the issue, but I don't think it's
> enough a justification to merge broken SPL support. If it was only
> partial or limited, it would be fine, but here it's just broken.
>
> > Anyway, making SPL as an optional is not an idea to go with Mainline
> > as we make many decisions with regards to make SPL is mandatory.
>
> Yes I think making SPL mandatory is a good idea, so that's why I'm
> suggesting that we don't merge the boards until they have SPL support.
>
> > Since the DDR is show stopper here (and it would really need a good
> > amount of time, since it effect the other boards), I can go with TPL
> > enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> > booting stages. This way we can avoid skipping SPL usage, and many
> > config changes to make SPL optional.
>
> Honestly I don't really see the point of merging these boards at all if
> they don't have SPL support. People who really want to use them with
> the rockchip blob can cherry-pick the patches from the list in the
> meantime.
>
> It also creates incentive for people to free the DDR init, since that
> becomes a condition to have the board upstream.
>
> What do you think?

I don't know whether you get my point or not? these boards are not
merged yet. What I'm saying is we are going to support them with
TPL-enabled, that was SPL can make use of these boards which still a
valid boot-chain. moreover this way can avoid touching core files and
no specific change require while supporting ddr dtsi.

Jagan.
Paul Kocialkowski May 9, 2019, 12:39 p.m. UTC | #5
On Thu, 2019-05-09 at 18:06 +0530, Jagan Teki wrote:
> On Thu, May 9, 2019 at 6:01 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
> > > Hi Paul,
> > > 
> > > On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > > > > (Sorry for the noice, I have missed to send two patches from v7)
> > > > > 
> > > > > This is v7 resend patchset for New rk3399 boards support wrt previous
> > > > > version[1]
> > > > > 
> > > > > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > > > > orangepi rk3399 changes are merged, so this is rework on top of
> > > > > u-boot-rockchip/master.
> > > > > 
> > > > > Overall this series add support below rk3399 boards
> > > > > - NanoPI M4
> > > > > - NanoPC T4
> > > > > - NanoPI NEO4
> > > > > - Orangepi RK3399
> > > > > - Rock PI 4
> > > > > - Rockpro64
> > > > > 
> > > > > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > > > > dts(i) from linux-next.
> > > > > 
> > > > > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > > > > series [3].
> > > > > 
> > > > > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > > > > booting via Rockchip miniloader as of now.
> > > > 
> > > > Could you send these two boards in a separate series so that we avoid
> > > > merging them for now (because SPL support is broken) and then re-
> > > > iterate the series later with the DDR bringup? Or maybe find a way to
> > > > disable SPL support, but in any case, it's not ok to merge a board with
> > > > SPL enabled and broken.
> > > 
> > > I have explained the details about this concern on v2 [1], thought you
> > > would comeback on the same line instead here.
> > 
> > Yes, you have already explained the issue, but I don't think it's
> > enough a justification to merge broken SPL support. If it was only
> > partial or limited, it would be fine, but here it's just broken.
> > 
> > > Anyway, making SPL as an optional is not an idea to go with Mainline
> > > as we make many decisions with regards to make SPL is mandatory.
> > 
> > Yes I think making SPL mandatory is a good idea, so that's why I'm
> > suggesting that we don't merge the boards until they have SPL support.
> > 
> > > Since the DDR is show stopper here (and it would really need a good
> > > amount of time, since it effect the other boards), I can go with TPL
> > > enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> > > booting stages. This way we can avoid skipping SPL usage, and many
> > > config changes to make SPL optional.
> > 
> > Honestly I don't really see the point of merging these boards at all if
> > they don't have SPL support. People who really want to use them with
> > the rockchip blob can cherry-pick the patches from the list in the
> > meantime.
> > 
> > It also creates incentive for people to free the DDR init, since that
> > becomes a condition to have the board upstream.
> > 
> > What do you think?
> 
> I don't know whether you get my point or not? these boards are not
> merged yet. What I'm saying is we are going to support them with
> TPL-enabled, that was SPL can make use of these boards which still a
> valid boot-chain. moreover this way can avoid touching core files and
> no specific change require while supporting ddr dtsi.

Ah maybe I missed the point indeed.

So what you are suggesting is:
rkboot (acts as TPL) -> SPL -> U-Boot?

Then I have no problem what that.

Cheers,

Paul
Jagan Teki May 9, 2019, 12:47 p.m. UTC | #6
On Thu, May 9, 2019 at 6:09 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> On Thu, 2019-05-09 at 18:06 +0530, Jagan Teki wrote:
> > On Thu, May 9, 2019 at 6:01 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
> > > > Hi Paul,
> > > >
> > > > On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > > > > > (Sorry for the noice, I have missed to send two patches from v7)
> > > > > >
> > > > > > This is v7 resend patchset for New rk3399 boards support wrt previous
> > > > > > version[1]
> > > > > >
> > > > > > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > > > > > orangepi rk3399 changes are merged, so this is rework on top of
> > > > > > u-boot-rockchip/master.
> > > > > >
> > > > > > Overall this series add support below rk3399 boards
> > > > > > - NanoPI M4
> > > > > > - NanoPC T4
> > > > > > - NanoPI NEO4
> > > > > > - Orangepi RK3399
> > > > > > - Rock PI 4
> > > > > > - Rockpro64
> > > > > >
> > > > > > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > > > > > dts(i) from linux-next.
> > > > > >
> > > > > > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > > > > > series [3].
> > > > > >
> > > > > > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > > > > > booting via Rockchip miniloader as of now.
> > > > >
> > > > > Could you send these two boards in a separate series so that we avoid
> > > > > merging them for now (because SPL support is broken) and then re-
> > > > > iterate the series later with the DDR bringup? Or maybe find a way to
> > > > > disable SPL support, but in any case, it's not ok to merge a board with
> > > > > SPL enabled and broken.
> > > >
> > > > I have explained the details about this concern on v2 [1], thought you
> > > > would comeback on the same line instead here.
> > >
> > > Yes, you have already explained the issue, but I don't think it's
> > > enough a justification to merge broken SPL support. If it was only
> > > partial or limited, it would be fine, but here it's just broken.
> > >
> > > > Anyway, making SPL as an optional is not an idea to go with Mainline
> > > > as we make many decisions with regards to make SPL is mandatory.
> > >
> > > Yes I think making SPL mandatory is a good idea, so that's why I'm
> > > suggesting that we don't merge the boards until they have SPL support.
> > >
> > > > Since the DDR is show stopper here (and it would really need a good
> > > > amount of time, since it effect the other boards), I can go with TPL
> > > > enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> > > > booting stages. This way we can avoid skipping SPL usage, and many
> > > > config changes to make SPL optional.
> > >
> > > Honestly I don't really see the point of merging these boards at all if
> > > they don't have SPL support. People who really want to use them with
> > > the rockchip blob can cherry-pick the patches from the list in the
> > > meantime.
> > >
> > > It also creates incentive for people to free the DDR init, since that
> > > becomes a condition to have the board upstream.
> > >
> > > What do you think?
> >
> > I don't know whether you get my point or not? these boards are not
> > merged yet. What I'm saying is we are going to support them with
> > TPL-enabled, that was SPL can make use of these boards which still a
> > valid boot-chain. moreover this way can avoid touching core files and
> > no specific change require while supporting ddr dtsi.
>
> Ah maybe I missed the point indeed.
>
> So what you are suggesting is:
> rkboot (acts as TPL) -> SPL -> U-Boot?

Exactly, to make it confirm.

Here is boot-chain on NEO4:
------------------------------------

DDR Version 1.20 20190314
In
Channel 0: DDR3, 800MHz
Bus Width=32 Col=10 Bank=8 Row=15 CS=1 Die Bus-Width=16 Size=1024MB
no stride
ch 0 ddrconfig = 0x101, ddrsize = 0x20
pmugrf_os_reg[2] = 0x10006281, stride = 0x17
OUT

U-Boot SPL 2019.07-rc1-00243-gbd57cc7444 (May 09 2019 - 18:10:25 +0530)
Trying to boot from MMC1


U-Boot 2019.07-rc1-00243-gbd57cc7444 (May 09 2019 - 18:12:59 +0530)

Model: FriendlyARM NanoPi NEO4
Paul Kocialkowski May 9, 2019, 12:51 p.m. UTC | #7
Hi,

On Thu, 2019-05-09 at 14:40 +0200, Philipp Tomsich wrote:
> Jagan,
> 
> > On 09.05.2019, at 14:36, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > 
> > On Thu, May 9, 2019 at 6:01 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > > 
> > > On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
> > > > Hi Paul,
> > > > 
> > > > On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
> > > > > > (Sorry for the noice, I have missed to send two patches from v7)
> > > > > > 
> > > > > > This is v7 resend patchset for New rk3399 boards support wrt previous
> > > > > > version[1]
> > > > > > 
> > > > > > Unfortunately initial version of creating rk3399-u-boot.dtsi and
> > > > > > orangepi rk3399 changes are merged, so this is rework on top of
> > > > > > u-boot-rockchip/master.
> > > > > > 
> > > > > > Overall this series add support below rk3399 boards
> > > > > > - NanoPI M4
> > > > > > - NanoPC T4
> > > > > > - NanoPI NEO4
> > > > > > - Orangepi RK3399
> > > > > > - Rock PI 4
> > > > > > - Rockpro64
> > > > > > 
> > > > > > All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> > > > > > dts(i) from linux-next.
> > > > > > 
> > > > > > SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> > > > > > series [3].
> > > > > > 
> > > > > > Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> > > > > > booting via Rockchip miniloader as of now.
> > > > > 
> > > > > Could you send these two boards in a separate series so that we avoid
> > > > > merging them for now (because SPL support is broken) and then re-
> > > > > iterate the series later with the DDR bringup? Or maybe find a way to
> > > > > disable SPL support, but in any case, it's not ok to merge a board with
> > > > > SPL enabled and broken.
> > > > 
> > > > I have explained the details about this concern on v2 [1], thought you
> > > > would comeback on the same line instead here.
> > > 
> > > Yes, you have already explained the issue, but I don't think it's
> > > enough a justification to merge broken SPL support. If it was only
> > > partial or limited, it would be fine, but here it's just broken.
> > > 
> > > > Anyway, making SPL as an optional is not an idea to go with Mainline
> > > > as we make many decisions with regards to make SPL is mandatory.
> > > 
> > > Yes I think making SPL mandatory is a good idea, so that's why I'm
> > > suggesting that we don't merge the boards until they have SPL support.
> > > 
> > > > Since the DDR is show stopper here (and it would really need a good
> > > > amount of time, since it effect the other boards), I can go with TPL
> > > > enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> > > > booting stages. This way we can avoid skipping SPL usage, and many
> > > > config changes to make SPL optional.
> > > 
> > > Honestly I don't really see the point of merging these boards at all if
> > > they don't have SPL support. People who really want to use them with
> > > the rockchip blob can cherry-pick the patches from the list in the
> > > meantime.
> > > 
> > > It also creates incentive for people to free the DDR init, since that
> > > becomes a condition to have the board upstream.
> > > 
> > > What do you think?
> > 
> > I don't know whether you get my point or not? these boards are not
> > merged yet. What I'm saying is we are going to support them with
> > TPL-enabled, that was SPL can make use of these boards which still a
> > valid boot-chain. moreover this way can avoid touching core files and
> > no specific change require while supporting ddr dtsi.
> 
> On some boards, there will be no TPL and only a SPL stage that will
> initialise DRAM (as the move to having TPL on the RK3399 is optional).
> 
> I agree with Paul that the DRAM init should be part of U-Boot whenever
> we add new boards and make an open DRAM init a prerequisite.

Well, my initial point was to forbid merging broken SPL support, but I
am totally in favor of requiring free DRAM init for merging new boards.

Sadly it's hard to enforce this as a general rule in U-Boot since some
platforms are plagued by non-free first-stage bootloaders because of
signature checks, and that's where DRAM init happens.

But for RK3399, we can totally have that rule, which directly creates
incentive to free the blobs.

Cheers,

Paul
Jagan Teki May 9, 2019, 12:57 p.m. UTC | #8
Hi Philipp,

On Thu, May 9, 2019 at 6:10 PM Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
> Jagan,
>
> On 09.05.2019, at 14:36, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Thu, May 9, 2019 at 6:01 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
>
>
> Hi,
>
> On Thu, 2019-05-09 at 16:15 +0530, Jagan Teki wrote:
>
> Hi Paul,
>
> On Thu, May 9, 2019 at 12:38 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Wed, 2019-05-08 at 11:11 +0530, Jagan Teki wrote:
>
> (Sorry for the noice, I have missed to send two patches from v7)
>
> This is v7 resend patchset for New rk3399 boards support wrt previous
> version[1]
>
> Unfortunately initial version of creating rk3399-u-boot.dtsi and
> orangepi rk3399 changes are merged, so this is rework on top of
> u-boot-rockchip/master.
>
> Overall this series add support below rk3399 boards
> - NanoPI M4
> - NanoPC T4
> - NanoPI NEO4
> - Orangepi RK3399
> - Rock PI 4
> - Rockpro64
>
> All the respective dts(i) files are synced from Linux 5.1-rc2 and few
> dts(i) from linux-next.
>
> SoC u-boot specific dtsi rk3399-u-boot.dtsi changes are part of another
> series [3].
>
> Out of all above boards Rockpor64, Rock-PI and Nanopi NEO4 would support
> booting via Rockchip miniloader as of now.
>
>
> Could you send these two boards in a separate series so that we avoid
> merging them for now (because SPL support is broken) and then re-
> iterate the series later with the DDR bringup? Or maybe find a way to
> disable SPL support, but in any case, it's not ok to merge a board with
> SPL enabled and broken.
>
>
> I have explained the details about this concern on v2 [1], thought you
> would comeback on the same line instead here.
>
>
> Yes, you have already explained the issue, but I don't think it's
> enough a justification to merge broken SPL support. If it was only
> partial or limited, it would be fine, but here it's just broken.
>
> Anyway, making SPL as an optional is not an idea to go with Mainline
> as we make many decisions with regards to make SPL is mandatory.
>
>
> Yes I think making SPL mandatory is a good idea, so that's why I'm
> suggesting that we don't merge the boards until they have SPL support.
>
> Since the DDR is show stopper here (and it would really need a good
> amount of time, since it effect the other boards), I can go with TPL
> enabled boot-chain where ddr bin, SPL and U-Boot proper can be part of
> booting stages. This way we can avoid skipping SPL usage, and many
> config changes to make SPL optional.
>
>
> Honestly I don't really see the point of merging these boards at all if
> they don't have SPL support. People who really want to use them with
> the rockchip blob can cherry-pick the patches from the list in the
> meantime.
>
> It also creates incentive for people to free the DDR init, since that
> becomes a condition to have the board upstream.
>
> What do you think?
>
>
> I don't know whether you get my point or not? these boards are not
> merged yet. What I'm saying is we are going to support them with
> TPL-enabled, that was SPL can make use of these boards which still a
> valid boot-chain. moreover this way can avoid touching core files and
> no specific change require while supporting ddr dtsi.
>
>
> On some boards, there will be no TPL and only a SPL stage that will
> initialise DRAM (as the move to having TPL on the RK3399 is optional).

True, my suggestion here the same. SPL is mandatory.

>
> I agree with Paul that the DRAM init should be part of U-Boot whenever
> we add new boards and make an open DRAM init a prerequisite.

True, I agree this point. Since we have an option of having DRAM init
at TPL I'm proposing this boot-chain (along with commitment on dram
work).