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

From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
To: Jagan Teki <jagan@amarulasolutions.com>
Cc: U-Boot-Denx <u-boot@lists.denx.de>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	linux-amarula <linux-amarula@amarulasolutions.com>
Subject: Re: [RESEND PATCH v7 00/11] rockchip: Add new rk3399 boards
Date: Thu, 9 May 2019 14:40:18 +0200	[thread overview]
Message-ID: <4EB52AEE-006A-41E4-A2FC-1837DF76295B@theobroma-systems.com> (raw)
In-Reply-To: <CAMty3ZCzY+UgHvA7gabt0vAbWR_zOGv3t=gFp9bJnmTOOU_aMg@mail.gmail.com>

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 <mailto: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.

Thanks,
Philipp.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot