From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760907AbcLPMcs (ORCPT ); Fri, 16 Dec 2016 07:32:48 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36803 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759496AbcLPMcj (ORCPT ); Fri, 16 Dec 2016 07:32:39 -0500 From: Pali =?utf-8?q?Roh=C3=A1r?= To: Javier Martinez Canillas Subject: Re: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs Date: Fri, 16 Dec 2016 13:32:36 +0100 User-Agent: KMail/1.13.7 (Linux/3.13.0-105-generic; KDE/4.14.2; x86_64; ; ) Cc: Tony Lindgren , "Russell King - ARM Linux" , Arnd Bergmann , Robin Murphy , Linus Walleij , Ben Dooks , Ivaylo Dimitrov , Sebastian Reichel , Aaro Koskinen , Pavel Machek , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1481749963-8664-1-git-send-email-pali.rohar@gmail.com> <201612161246.12155@pali> <029ec718-89c0-b08e-de7e-279946c1a9bd@osg.samsung.com> In-Reply-To: <029ec718-89c0-b08e-de7e-279946c1a9bd@osg.samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart10591345.bRCaLpB8pg"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <201612161332.36406@pali> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart10591345.bRCaLpB8pg Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote: > Hello Pali, >=20 > On 12/16/2016 08:46 AM, Pali Roh=C3=A1r wrote: > > On Thursday 15 December 2016 01:09:20 Pali Roh=C3=A1r wrote: > >> On Thursday 15 December 2016 00:52:24 Russell King - ARM Linux > >> wrote: > >>> On Wed, Dec 14, 2016 at 10:12:43PM +0100, Pali Roh=C3=A1r wrote: > >>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi > >>>> usage") broke support for setting cmdline on Nokia N900 via > >>>> CONFIG_CMDLINE. > >>>>=20 > >>>> It is because arm code booted in DT mode parse cmdline only via > >>>> function early_init_dt_scan_chosen() and that function does not > >>>> fill variable boot_command_line when DTB does not contain > >>>> /chosen entry. It is called from function > >>>> early_init_dt_scan_nodes() in setup_machine_fdt(). > >>>>=20 > >>>> This patch fixes it by explicitly filling boot_command_line in > >>>> function setup_machine_fdt() after calling > >>>> early_init_dt_scan_nodes() in case boot_command_line still > >>>> remains empty. > >>>=20 > >>> This looks like a hack. > >>>=20 > >>> First, the matter of the ATAGs compatibility. The decompressor > >>> relies on there being a pre-existing /chosen node to insert the > >>> command line and other parameters into. If we've dropped it (by > >>> dropping skeleton.dtsi) then we've just regressed more than just > >>> N900 - the decompressor won't be able to merge the ATAGs into the > >>> concatenated FDT. > >>=20 > >> Hm... I did not think about it. But right this can be broken > >> too... > >=20 > > Tony, Javier: are you aware of above =E2=86=91=E2=86=91=E2=86=91 proble= m? > >=20 > > It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove > > skeleton.dtsi usage") should be really reverted. >=20 > I don't think reverting the mentioned commit is the correct fix for > your problem. We are trying to get rid of skeleton.dtsi for many > reasons, see commit commit ("3ebee5a2e141 arm64: dts: kill > skeleton.dtsi"). $ git show 3ebee5a2e141 * The default empty /chosen and /aliases are somewhat useless... That is not truth, they are not useless as Russell King wrote --=20 removing them break ATAG support. (But that commit is for arm64 which probably is not using ATAGs... But I=20 do not know. At least it is not truth for 32bit arm.) > Also, the chosen node is mentioned to be optional in the ePAPR > document and u-boot creates a chosen node if isn't found [0] so this > issue is only present in boards that don't use u-boot like the > N900/N950/N9 phones. Linux arm decompressor does not propagate ATAGs when /chosen is missing.=20 Sorry, but if for Linux /chosen is required (and without it is broken!)=20 then some ePARP document does not apply there. Either Linux code needs=20 to be fixed (so /chosen will be really only optional) or /chosen stay in=20 Linux required. There is no other option. And I hope that U-boot is not the only one bootloader which Linux kernel=20 supports. I thought that I can use *any* bootloader to boot Linux kernel=20 not just U-Boot which is doing some magic... With this step you are basically going to break booting Linux kernel=20 with all others bootloaders... And personally I really dislike this=20 idea. > So if NOLO doesn't do the same than u-boot and the kernel expects a > chosen node, I suggest to add an empty chosen node in the > omap3-n900.dts and omap3-n950-n9.dtsi device tree source files. That would fix a problem for N900, N950 and N9. But not for all other=20 ARM devices which bootloader pass some ATAGs. IIRC rule of kernel is not to break compatibility and that commit=20 008a2ebcd677 really did it. Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just saying=20 that it cause problems which need to be properly fixed. And if fixing=20 them is harder and will take more time, then correct option is to revert=20 008a2ebcd677 due to breaking support for more devices. > [0]: > http://git.denx.de/?p=3Du-boot.git;a=3Dblob;f=3Dcommon/fdt_support.c;h=3D= c9f > 7019e38e8de1469f506cdd57353fd27d8e134;hb=3DHEAD#l226 >=20 > Best regards, =2D-=20 Pali Roh=C3=A1r pali.rohar@gmail.com --nextPart10591345.bRCaLpB8pg Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEABECAAYFAlhT3uQACgkQi/DJPQPkQ1JCrgCeJhPkoz/GxP4/kGF4KE6WUt1L 5M4An3VFiFjOkm98wyzCy5bXKlyJdG/1 =T30k -----END PGP SIGNATURE----- --nextPart10591345.bRCaLpB8pg--