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

Fri May 26 16:09:37 UTC 2017

On Fri, May 26, 2017 at 02:58:04PM +0200, Jorge Ramirez Ortiz wrote:
> On May 26, 2017 2:46 PM, "Tom Rini" <trini at konsulko.com> wrote:
> 
> 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.
> 
> 
> 
> This the bit that is NOT possible. Doing that is not enough.

To be clear, are you trying this on current mainline?  Simon reminded me
that if you don't have 7452946e7f37 in your tree, the -u-boot.dtsi file
cannot disable a node.

-- 
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/da8c1b19/attachment.sig>