From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760699AbcLPMtc (ORCPT ); Fri, 16 Dec 2016 07:49:32 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:36615 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759552AbcLPMsi (ORCPT ); Fri, 16 Dec 2016 07:48:38 -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:48:35 +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> <201612161332.36406@pali> <5c56e769-76be-295e-b655-8431dde35370@osg.samsung.com> In-Reply-To: <5c56e769-76be-295e-b655-8431dde35370@osg.samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart1658460.0rCHPbzsbK"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <201612161348.35917@pali> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart1658460.0rCHPbzsbK Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote: > Hello Pali, >=20 > On 12/16/2016 09:32 AM, Pali Roh=C3=A1r wrote: > > Hi! > >=20 > > 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 > >>>>=20 > >>>> 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 prob= lem? > >>>=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"). > >=20 > > $ git show 3ebee5a2e141 > >=20 > > * The default empty /chosen and /aliases are somewhat useless... > >=20 > > That is not truth, they are not useless as Russell King wrote -- > > removing them break ATAG support. > >=20 > > (But that commit is for arm64 which probably is not using ATAGs... > > But I do not know. At least it is not truth for 32bit arm.) > >=20 > >> 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. > >=20 > > Linux arm decompressor does not propagate ATAGs when /chosen is > > missing. Sorry, but if for Linux /chosen is required (and without > > it is broken!) then some ePARP document does not apply there. > > Either Linux code needs to be fixed (so /chosen will be really > > only optional) or /chosen stay in Linux required. There is no > > other option. > >=20 > > And I hope that U-boot is not the only one bootloader which Linux > > kernel supports. I thought that I can use *any* bootloader to boot > > Linux kernel not just U-Boot which is doing some magic... > >=20 > > With this step you are basically going to break booting Linux > > kernel with all others bootloaders... And personally I really > > dislike this idea. > >=20 > >> 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. > >=20 > > That would fix a problem for N900, N950 and N9. But not for all > > other ARM devices which bootloader pass some ATAGs. > >=20 > > IIRC rule of kernel is not to break compatibility and that commit > > 008a2ebcd677 really did it. > >=20 > > Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just > > saying that it cause problems which need to be properly fixed. And > > if fixing them is harder and will take more time, then correct > > option is to revert 008a2ebcd677 due to breaking support for more > > devices. >=20 > If you think that others boards may have the same issue, then you > could add an empty chosen node to omap3.dtsi. As I said I think that > in practice this will only be needed for the machines using NOLO but > you are right that in theory you could boot them using other > bootloaders and having an empty node doesn't cause any harm anyway. Should not be it part of any arm board? IIRC ATAG support is (or was)=20 not omap3 specified. > >> [0]: > >> http://git.denx.de/?p=3Du-boot.git;a=3Dblob;f=3Dcommon/fdt_support.c;h= =3Dc > >> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=3DHEAD#l226 > >>=20 > >> Best regards, >=20 > Best regards, =2D-=20 Pali Roh=C3=A1r pali.rohar@gmail.com --nextPart1658460.0rCHPbzsbK 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) iEYEABECAAYFAlhT4qMACgkQi/DJPQPkQ1I5yACeKcO9CgMYiRGFyctsScTmb3ty 9aAAnj2LJRcJtmJO55Oaybjqv9/wj9Kx =h6dL -----END PGP SIGNATURE----- --nextPart1658460.0rCHPbzsbK-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: pali.rohar@gmail.com (Pali =?utf-8?q?Roh=C3=A1r?=) Date: Fri, 16 Dec 2016 13:48:35 +0100 Subject: [PATCH] arm: dt: Initialize boot_command_line from CONFIG_CMDLINE in case DT does not provide /chosen/bootargs In-Reply-To: <5c56e769-76be-295e-b655-8431dde35370@osg.samsung.com> References: <1481749963-8664-1-git-send-email-pali.rohar@gmail.com> <201612161332.36406@pali> <5c56e769-76be-295e-b655-8431dde35370@osg.samsung.com> Message-ID: <201612161348.35917@pali> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Friday 16 December 2016 13:38:34 Javier Martinez Canillas wrote: > Hello Pali, > > On 12/16/2016 09:32 AM, Pali Roh?r wrote: > > Hi! > > > > On Friday 16 December 2016 13:13:34 Javier Martinez Canillas wrote: > >> Hello Pali, > >> > >> On 12/16/2016 08:46 AM, Pali Roh?r wrote: > >>> On Thursday 15 December 2016 01:09:20 Pali Roh?r 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?r wrote: > >>>>>> Commit 008a2ebcd677 ("ARM: dts: omap3: Remove skeleton.dtsi > >>>>>> usage") broke support for setting cmdline on Nokia N900 via > >>>>>> CONFIG_CMDLINE. > >>>>>> > >>>>>> 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(). > >>>>>> > >>>>>> 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. > >>>>> > >>>>> This looks like a hack. > >>>>> > >>>>> 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. > >>>> > >>>> Hm... I did not think about it. But right this can be broken > >>>> too... > >>> > >>> Tony, Javier: are you aware of above ??? problem? > >>> > >>> It looks like commit 008a2ebcd677 ("ARM: dts: omap3: Remove > >>> skeleton.dtsi usage") should be really reverted. > >> > >> 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 -- > > removing them break ATAG support. > > > > (But that commit is for arm64 which probably is not using ATAGs... > > But I 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. Sorry, but if for Linux /chosen is required (and without > > it is broken!) then some ePARP document does not apply there. > > Either Linux code needs to be fixed (so /chosen will be really > > only optional) or /chosen stay in Linux required. There is no > > other option. > > > > And I hope that U-boot is not the only one bootloader which Linux > > kernel supports. I thought that I can use *any* bootloader to boot > > Linux kernel not just U-Boot which is doing some magic... > > > > With this step you are basically going to break booting Linux > > kernel with all others bootloaders... And personally I really > > dislike this 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 ARM devices which bootloader pass some ATAGs. > > > > IIRC rule of kernel is not to break compatibility and that commit > > 008a2ebcd677 really did it. > > > > Note: I'm not saying if 008a2ebcd677 is good or bad. I'm just > > saying that it cause problems which need to be properly fixed. And > > if fixing them is harder and will take more time, then correct > > option is to revert 008a2ebcd677 due to breaking support for more > > devices. > > If you think that others boards may have the same issue, then you > could add an empty chosen node to omap3.dtsi. As I said I think that > in practice this will only be needed for the machines using NOLO but > you are right that in theory you could boot them using other > bootloaders and having an empty node doesn't cause any harm anyway. Should not be it part of any arm board? IIRC ATAG support is (or was) not omap3 specified. > >> [0]: > >> http://git.denx.de/?p=u-boot.git;a=blob;f=common/fdt_support.c;h=c > >> 9f 7019e38e8de1469f506cdd57353fd27d8e134;hb=HEAD#l226 > >> > >> Best regards, > > Best regards, -- Pali Roh?r pali.rohar at gmail.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: This is a digitally signed message part. URL: