From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jorge Ramirez Ortiz Date: Fri, 26 May 2017 14:58:04 +0200 Subject: [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards In-Reply-To: <20170526124630.GM10782@bill-the-cat> References: <20170517220623.GL4631@bill-the-cat> <7fd042e6-fb1f-a4ee-0a54-c39a0e25ef40@linaro.org> <20170525203100.GE10782@bill-the-cat> <21be0cad-5ac4-63c2-851a-fd28c4dc2a35@linaro.org> <20170525211205.GF10782@bill-the-cat> <20170525220853.GG10782@bill-the-cat> <29be56cb-767b-2992-1d4a-9149ddac5092@linaro.org> <20170526124630.GM10782@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On May 26, 2017 2:46 PM, "Tom Rini" 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. I will also have to modify the kernel dts. -- Tom