[U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards

[Tom Rini]

On Fri, May 26, 2017 at 09:28:28AM +0200, Jorge Ramirez wrote:
> On 05/26/2017 12:08 AM, Tom Rini wrote:
> >On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
> >>On 05/25/2017 11:12 PM, Tom Rini wrote:
> >>>On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> >>>>On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >>>>>On 05/25/2017 10:31 PM, Tom Rini wrote:
> >>>>>>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >>>>>>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>>>>>>>>having platform data.
> >>>>>>>>>>No, I think we're going for overkill here by not doing
> >>>>>>>>>>serial_pl01x.c as
> >>>>>>>>>>platform data.  ns16550 does platform data for this already.  This
> >>>>>>>>>>sounds like the lowest overhead way to get the clock
> >>>>>>>>>>populated and not
> >>>>>>>>>>have some DT data that's not going to be accepted upstream.
> >>>>>>>>>>
> >>>>>>>>>ummmm I am a bit lost at this point, could we recap please?
> >>>>>>>>Lets update the recap:
> >>>>>>>>- Please re-submit the dts file, now with whatever form is
> >>>>>>>>in v4.12-rc1,
> >>>>>>>>   saying as such in the commit (if it's just the commit message that
> >>>>>>>>   changes, that's fine and great).
> >>>>>>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >>>>>>>
> >>>>>>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >>>>>>Well, you can either put it in the -u-boot.dtsi file for the board, and
> >>>>>>remove that later once it's upstream.
> >>>>yes I'll do that. thanks.
> >>>>
> >>>>>>>>- Please update serial_pl01x.c to be able to get the clock
> >>>>>>>>via platform
> >>>>>>>>   data, update and test your board to confirm it works.
> >>>>>>>um, It gets tricky;
> >>>>>>>I can not use platform_data since I can not use SERIAL_DM because
> >>>>>>>the device tree does have a UART node which gets picked up.
> >>>>>>How about disabling the node in -u-boot.dtsi for the board and then you
> >>>>>>can use platform data,
> >>>>I dont think that would because CONFIG_OF is enabled for USB; so the
> >>>>kernel dtsi that contains the uart node (without the clock!) will be
> >>>>picked by u-boot and the uart will not be initialized properly.
> >>>>I still think that the simplest solution is to let me merge with the
> >>>>kernel's device tree plus this u-boot.dtsi [1];
> >>>>then just get rid of the file when possible (and NEIHER the source
> >>>>code NOR the configs) would need to change
> >>>>
> >>>>[1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >>>Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> >>>use platform data, at least for now.  I do want to talk more with Rob
> >>>about the general problem this exposes.
> >>so you want me to
> >>
> >>1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >Well, a uart0 node, but no "clock" property as that just needs to go
> >away.
> 
> Sounds good to me. now see below
> 
> >>2) add status=disable
> >>3) then add the platform_data
> >>
> >>BUT for the pl011 driver to take the platform_data dont I also have
> >>to disable CONFIG_OF?
> >>
> >>but  if I disable CONFIG_OF then I lose USB_DM
> >No, the status = "disable" on uart0 should remove it from the dtb, or at
> >least we should see it and go "Oh, no, we don't have uart0 via DT".
> 
> 1) Since the UART0 is enabled in the kernel's DTS I will have to
> modify the device tree that I am importing from kernel.org to
> disable it.

Yes, the dtsi file in [1] is what modifies it.

> 2) But even doing this is not enough: I have to completely remove
> the uart0 node from the tree.

Why?  Is this in theory, or in practice?  I ask since we just had to
disable the kernel-enabled mmc3 node on am335x-evm.dts in
am335x-evm-u-boot.dtsi in order to have it boot as probing mmc3 in
U-Boot fails (we don't enable the relevant clocks).  So disabling uart0
should cause the resulting dtb file to omit entirely, or just have
&uart0 {status="disabled"} or similar and U-Boot should not do anything,
so platform data should be used.  If this is not the case, there's a bug
we need to track down.

> 
> 
> So to sum up:
> 
> In order to get the platform data for pl01x I have to either disable
> OF (so I lose the USB node as I said earlier) or *completely* remove
> the UART0 node from from the kernel dts.
> I personally would rather not modify the kernel's DTS trees that I
> am importing into uboot but I am really confused about the policy
> now.
> 
> please could you clarify?
> 
> I still think what I proposed when we started is the better way to
> go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
> two nodes that are giving trouble.

I don't understand what we're not understanding, yes, you should be
using a -u-boot.dtsi file to mark uart0 as disabled and not have to
modify the kernel dts file at all.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170526/b3b44262/attachment.sig>