Linux Tegra architecture development
 help / color / mirror / Atom feed
* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 19:20 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Arnd Bergmann, Stephen Warren, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <CAOesGMgDwMeVmwo1WJ3tU+ZdjCSJ6F8mY42e_KwTmzAkGQsqmg@mail.gmail.com>

On Fri, Oct 14, 2011 at 09:44:34AM -0700, Olof Johansson wrote:
> That is likely to get messy.
> 
> Seems like there could be some use for a (silent) option for a
> platform to indicate that it can do XIP kernel (or zImage), and thus
> not able to use AUTO_ZRELADDR (or other options that require rewriting
> text segment of zImage or kernel).

It's not that.

The options are:

AUTO_ZRELADDR=n ZBOOT_ROM=n => fixed address for decompressed kernel
	image to be decompressed to from zreladdr make variable,
	decompressor can be loaded to any RAM address.

AUTO_ZRELADDR=y ZBOOT_ROM=n => address for decompressed kernel calculated
	from location of zImage in RAM, decompressor must be loaded to
	the correct place in RAM and must always copy itself out of
	the way prior to decompressing.

AUTO_ZRELADDR=n ZBOOT_ROM=y => fixed address for decompressed kernel
	image to be decompressed to from zreladdr make variable,
	decompressor can be loaded to any RAM address which doesn't
	overlap its BSS segment, or decompressor programmed into
	read-only memory at the address to which it was compiled.

AUTO_ZRELADDR=y ZBOOT_ROM=y => invalid (think about it - this results
	in a zImage which is built to be run from read-only memory,
	and try to write the decompressed image into that read-only
	memory.)

This has nothing to do with XIP or not - XIP is a completely separate story,
and if you're building an XIP kernel you're not using the decompressor (so
AUTO_ZRELADDR and ZBOOT_ROM don't even feature.)  Neither does the dynamic
code patching stuff like ARM_PATCH_PHYS_VIRT.

ARM_PATCH_PHYS_VIRT has no bearing on AUTO_ZRELADDR or ZBOOT_ROM; that is
a property of the decompressed kernel image itself, and while your
decompressor may restrict where it can decompress the kernel image to,
the decompressed image itself remains free of those dependencies (and
eg, can be turned into a uImage with the appropriate load address
for other platforms.)

As I've said in the past, what I'd _like_ to see is that ARM_PATCH_PHYS_VIRT
be enabled for everything irrespective of any other configuration, and
we're just left with AUTO_ZRELADDR / ZBOOT_ROM to worry about.  The
overhead from the P:V patching is soo small that it's not really worth
even having the option in the general case - the only time when P:V
patching doesn't work is with non-linear translations found on some
sparsemem platforms.

But... one thing to note is that it _is_ common to load the decompressor
at a _different_ address to that where the kernel ultimately ends up
residing to avoid the additional copy in the decompressor.  My experience
shows that this is quite common on the platforms I had supplied.  This
means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
having to have developers change their uboot setups to avoid unexpected
results.

^ permalink raw reply

* RE: Adding board support
From: Stephen Warren @ 2011-10-14 19:20 UTC (permalink / raw)
  To: Thierry Reding; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20111014185847.GA916-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>

Thierry Reding wrote at Friday, October 14, 2011 12:59 PM:
> * Stephen Warren wrote:
> > Thierry Reding wrote at Thursday, October 13, 2011 11:50 PM:
...
> > > Another question concerns testing. So far I've always booted a modified
> > > U-Boot (from Vibrante 1.1) to allow booting with an initial ramdisk (loaded
> > > from SD/MMC) as payload for fastboot/quickboot. What are other people using?
> >
> > Personally, I've recently switched to using mainline U-Boot on almost all my
> > boards (Harmony, Seaboard, Ventana). Maybe I'll add TrimSlice support there
> > too. Note that this relies on a number of patches that have been posted to
> > the U-Boot mailing list, but not yet checked into their repos. This completely
> > bypasses fastboot; Tegra's boot ROM runs U-Boot directly instead of fastboot.
> 
> I was hoping that somebody had gotten this to work. Would you mind sharing
> the script that you use? All the scripts I've seen so far create some boot
> image (using a tool named mkbootimg) that contains U-Boot.
> 
> U-Boot mainline support is another point on my TODO list. Getting the latest
> mainline code with the patches you mention (I assume you are referring to the
> patch series by Simon Glass and Tom Warren) to work would be a good step in
> that direction.

I branched from: git://git.denx.de/u-boot.git master at commit
0841ca90f22d73b0ea4642ef1ce33d879bb2f3ff.

I applied the following:

http://patchwork.ozlabs.org/patch/115862/
http://patchwork.ozlabs.org/patch/115864/
http://patchwork.ozlabs.org/patch/115865/
http://patchwork.ozlabs.org/patch/115860/
http://patchwork.ozlabs.org/patch/115863/
http://patchwork.ozlabs.org/patch/115861/

http://patchwork.ozlabs.org/patch/119321/
http://patchwork.ozlabs.org/patch/119322/
http://patchwork.ozlabs.org/patch/119323/
http://patchwork.ozlabs.org/patch/119324/
http://patchwork.ozlabs.org/patch/119325/

http://patchwork.ozlabs.org/patch/118184/

I applied the following to hack the default environment so I could boot from
SD cards layed out how mine are; you'll probably need to tweak this a bunch:

diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index 07546a4..f30f569 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -104,9 +104,19 @@
 
 /* Environment information */
 #define CONFIG_EXTRA_ENV_SETTINGS \
-       "console=ttyS0,115200n8\0" \
-       "mem=" TEGRA2_SYSMEM "\0" \
-       "smpflag=smp\0" \
+       "bootcmd=run usb0_boot ; run usb1_boot ; run mmc1_boot ; run mmc0_boot\0" \
+       "bootdelay=2\0" \
+       "loadaddr=0x00500000\0" \
+       "scriptaddr=0x408000\0" \
+       "script_img=/u-boot/boot.scr.uimg\0" \
+       "mmc0_boot=setenv devnum 0; run mmc_boot;\0" \
+       "mmc1_boot=setenv devnum 1; run mmc_boot;\0" \
+       "usb0_boot=usb start 0; run usb_boot;\0" \
+       "usb1_boot=usb start 1; run usb_boot;\0" \
+       "scr_boot=fatload ${devtype} ${devnum}:c ${scriptaddr} ${script_img};source ${scriptaddr};read ${devtype} ${devnum}:${kernelpart} ${scriptaddr} 0 10;source ${scriptaddr};\0" \
+       "mmc_boot=mmc dev ${devnum};setenv devtype mmc;setenv devname mmcblk${devnum}p;run scr_boot;\0" \
+       "usb_boot=setenv devtype usb;setenv devnum 0;setenv devname sda;run scr_boot;\0" \
+       "platform_extras=lp0_vec=0x2000@0x1C406000 kcrashmem=0x100000@0x02000000 mem=384M@0M nvmem=128M@384M mem=511M@512M\0" \
 
 #define CONFIG_LOADADDR                0x408000        /* def. location for kernel */
 #define CONFIG_BOOTDELAY       2               /* -1 to disable auto boot */

In order to actually use the resultant u-boot.bin, it's pretty simple if you
already have nvflash working with burn fastboot.

1) Edit flash.cfg (or whatever config file you pass to nvflash to define
The partitions and their content) and replace the filename entry in the
EBT partition with u-boot.bin.

2) I personally remove all the partition entries in flash.cfg except for
BCT, PT, EBT. This will avoid nvflash flashing your Android/... filesystem.
If you want your filesystem in the same flash, don't do this.

Then, run nvflash just like you would normally.

> Have you done any testing with the NVIDIA binary blobs when booting this way?
> The latest information I have from our NVIDIA contacts is that fastboot or
> quickboot are required, for some reason, to make HW accelerated video
> decoding and 3D rendering work.

It's possible the binary drivers accidentally rely on the bootloader
having configured some piece of HW.

However, in general, the bootloader product you use shouldn't affect
whether the binary drivers work.

In particular, the binary drivers certainly work after the ChromeOS
U-Boot boots a board.

Coincidentally, right before seeing your email, I just tried mainline
U-Boot on Seaboard with our Linux4Tegra alpha 1 release, and while X
started, I couldn't see anything on screen. This did previously work
with some old version of ChromeOS's U-Boot. So, there's certainly some
missing HW initialization somewhere.

BTW, you might also want to take a look at NVIDIA's web forums:

http://developer.nvidia.com/beta-forum

While I'm personally happy to answer your questions here, (I work for
a group dedicated to making general Linux support on Tegra) you may find
more people aimed at this kind of support on those forums. I'm not
active on those forums though.

I hope this all helps!

-- 
nvpublic

^ permalink raw reply related

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 19:20 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Stephen Warren, Erik Gilling, Arnd Bergmann, Peter De Schrijver,
	linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Olof Johansson,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <alpine.LFD.2.02.1110141353390.17040@xanadu.home>

On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> platform specific exclusion thinking is a step backward so I'd prefer if 
> people would refrain from going there for the moment.

Are you expecting everyone to change the way they load the zImage
overnight then?

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 20:06 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Olof Johansson, Arnd Bergmann, Stephen Warren, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014192011.GS21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> But... one thing to note is that it _is_ common to load the decompressor
> at a _different_ address to that where the kernel ultimately ends up
> residing to avoid the additional copy in the decompressor.  My experience
> shows that this is quite common on the platforms I had supplied.  This
> means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
> having to have developers change their uboot setups to avoid unexpected
> results.

Currently, U-Boot insists on having a uImage with a fixed absolute load 
address.  This is currently provided by the zreladdr value, whether or 
not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
limitation.

Either u-Boot gets fixed so it can work with plain zImage (and this 
certainly will happen once the pressure from people wanting a single 
kernel to work on targets with different load addresses increase.  
Tegra is one such example.

Or we create a u-Boot specific Kconfig menu for uImage options that 
would be common to all architectures and kick it out from the ARM 
specific makefile.  This is not solving the u-Boot limitation though.

In either cases this is a u-Boot problem that needs fixing on the u-Boot 
side in the end.


Nicolas

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 20:12 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Olof Johansson, Arnd Bergmann, Stephen Warren, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <alpine.LFD.2.02.1110141549160.17040@xanadu.home>

On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> Currently, U-Boot insists on having a uImage with a fixed absolute load 
> address.  This is currently provided by the zreladdr value, whether or 
> not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
> limitation.
> 
> Either u-Boot gets fixed so it can work with plain zImage (and this 
> certainly will happen once the pressure from people wanting a single 
> kernel to work on targets with different load addresses increase.  
> Tegra is one such example.
> 
> Or we create a u-Boot specific Kconfig menu for uImage options that 
> would be common to all architectures and kick it out from the ARM 
> specific makefile.  This is not solving the u-Boot limitation though.
> 
> In either cases this is a u-Boot problem that needs fixing on the u-Boot 
> side in the end.

I don't think that's so with the various flavours of platform specific
uboot which float around.  For instance, on the OMAP4430 SDP, the
following commands were used as supplied to load a uImage off the SD
card into RAM at a different address to which it was built for, and
execute it at that address:

mmcinit 0
fatload mmc 0 0x80300000 uImage
bootm 80300000

Whether the 'bootm' command then copied the image and called it there,
or whether it executed it at 0x80300000 I've no idea - but why then
load the image at a different address in the first place?

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 20:14 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Stephen Warren, Olof Johansson, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014192057.GT21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > platform specific exclusion thinking is a step backward so I'd prefer if 
> > people would refrain from going there for the moment.
> 
> Are you expecting everyone to change the way they load the zImage
> overnight then?

No, of course.  But adding restrictions in the kernel build because 
u-Boot's own image format dictates such restrictions doesn't make sense.  
Those restrictions must be pushed towards the uImage encapsulation step, 
not higher the kernel config hierarchy.


Nicolas

^ permalink raw reply

* RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Stephen Warren @ 2011-10-14 20:14 UTC (permalink / raw)
  To: Nicolas Pitre, Russell King - ARM Linux
  Cc: Olof Johansson, Arnd Bergmann, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <alpine.LFD.2.02.1110141549160.17040@xanadu.home>

Nicolas Pitre wrote at Friday, October 14, 2011 2:06 PM:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > But... one thing to note is that it _is_ common to load the decompressor
> > at a _different_ address to that where the kernel ultimately ends up
> > residing to avoid the additional copy in the decompressor.  My experience
> > shows that this is quite common on the platforms I had supplied.  This
> > means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
> > having to have developers change their uboot setups to avoid unexpected
> > results.
> 
> Currently, U-Boot insists on having a uImage with a fixed absolute load
> address.  This is currently provided by the zreladdr value, whether or
> not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> limitation.

Well, that value only comes from zreladdr when running U-Boot's mkimage from
the kernel makefile rather than some other packaging script:-)

> Either u-Boot gets fixed so it can work with plain zImage (and this
> certainly will happen once the pressure from people wanting a single
> kernel to work on targets with different load addresses increase.
> Tegra is one such example.

I proposed to solve it by adding a new image format within the uImage;
"kernel-rel" rather than "kernel", where the load/entry address encoded
into the uImage is specified relative to "start of SDRAM" rather than
as an absolute address. This way, Tegra20 and Tegra30 will have the
same layout of U-Boot location, uImage load address, kernel execute
address etc. within SDRAM, but all those addresses might end up being
based at physical address 0, or physical address 2G.

Here's the patch I proposed:

http://patchwork.ozlabs.org/patch/119017/

What are your thoughts on this? I hope it will work for multi-SoC image
across vendors, although I suppose finding a common load address across
a bunch of different SoC's memory layouts might be tough?

I did originally briefly look into getting U-Boot to boot a zImage, but
that looked like a far more invasive patch. There were rumours of some
chip's custom U-Boot already having such support, but I couldn't find
it, nor any evidence of such support in mainline U-Boot.

> Or we create a u-Boot specific Kconfig menu for uImage options that
> would be common to all architectures and kick it out from the ARM
> specific makefile.  This is not solving the u-Boot limitation though.
> 
> In either cases this is a u-Boot problem that needs fixing on the u-Boot
> side in the end.

-- 
nvpublic

^ permalink raw reply

* RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Stephen Warren @ 2011-10-14 20:16 UTC (permalink / raw)
  To: Russell King - ARM Linux, Nicolas Pitre
  Cc: Olof Johansson, Arnd Bergmann, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014201223.GV21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Russell King wrote at Friday, October 14, 2011 2:12 PM:
> On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load
> > address.  This is currently provided by the zreladdr value, whether or
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> > limitation.
> >
> > Either u-Boot gets fixed so it can work with plain zImage (and this
> > certainly will happen once the pressure from people wanting a single
> > kernel to work on targets with different load addresses increase.
> > Tegra is one such example.
> >
> > Or we create a u-Boot specific Kconfig menu for uImage options that
> > would be common to all architectures and kick it out from the ARM
> > specific makefile.  This is not solving the u-Boot limitation though.
> >
> > In either cases this is a u-Boot problem that needs fixing on the u-Boot
> > side in the end.
> 
> I don't think that's so with the various flavours of platform specific
> uboot which float around.  For instance, on the OMAP4430 SDP, the
> following commands were used as supplied to load a uImage off the SD
> card into RAM at a different address to which it was built for, and
> execute it at that address:
> 
> mmcinit 0
> fatload mmc 0 0x80300000 uImage
> bootm 80300000
> 
> Whether the 'bootm' command then copied the image and called it there,
> or whether it executed it at 0x80300000 I've no idea - but why then
> load the image at a different address in the first place?

Yes, U-Boot's image handling looks at the image at 80300000, extracts the
"load address" from the image header, memcpy()'s the image to that address,
then jumps to the "entry address" in the image header.

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 20:17 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Arnd Bergmann, Stephen Warren, Olof Johansson, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <alpine.LFD.2.02.1110141609510.17040@xanadu.home>

On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > people would refrain from going there for the moment.
> > 
> > Are you expecting everyone to change the way they load the zImage
> > overnight then?
> 
> No, of course.  But adding restrictions in the kernel build because 
> u-Boot's own image format dictates such restrictions doesn't make sense.  
> Those restrictions must be pushed towards the uImage encapsulation step, 
> not higher the kernel config hierarchy.

You're not understanding again.

I'm talking about people who _explicitly_ load the zImage at a different
address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
their setup will break unless they stop that behaviour, which takes
away one of the advantages of using the zImage format.

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 20:19 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Nicolas Pitre, Olof Johansson, Arnd Bergmann, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A365@HQMAIL01.nvidia.com>

On Fri, Oct 14, 2011 at 01:16:27PM -0700, Stephen Warren wrote:
> Russell King wrote at Friday, October 14, 2011 2:12 PM:
> > I don't think that's so with the various flavours of platform specific
> > uboot which float around.  For instance, on the OMAP4430 SDP, the
> > following commands were used as supplied to load a uImage off the SD
> > card into RAM at a different address to which it was built for, and
> > execute it at that address:
> > 
> > mmcinit 0
> > fatload mmc 0 0x80300000 uImage
> > bootm 80300000
> > 
> > Whether the 'bootm' command then copied the image and called it there,
> > or whether it executed it at 0x80300000 I've no idea - but why then
> > load the image at a different address in the first place?
> 
> Yes, U-Boot's image handling looks at the image at 80300000, extracts the
> "load address" from the image header, memcpy()'s the image to that address,
> then jumps to the "entry address" in the image header.

Ah, so specifying that address is just a total waste of space then,
because you might as well specify an address which results in the
copy not being necessary.

However, I'd expect that uboot is dumb enough to still do the copy
irrespective of whether its already in the right place.

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 20:26 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Olof Johansson, Arnd Bergmann, Stephen Warren, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014201223.GV21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load 
> > address.  This is currently provided by the zreladdr value, whether or 
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
> > limitation.
> > 
> > Either u-Boot gets fixed so it can work with plain zImage (and this 
> > certainly will happen once the pressure from people wanting a single 
> > kernel to work on targets with different load addresses increase.  
> > Tegra is one such example.
> > 
> > Or we create a u-Boot specific Kconfig menu for uImage options that 
> > would be common to all architectures and kick it out from the ARM 
> > specific makefile.  This is not solving the u-Boot limitation though.
> > 
> > In either cases this is a u-Boot problem that needs fixing on the u-Boot 
> > side in the end.
> 
> I don't think that's so with the various flavours of platform specific
> uboot which float around.  For instance, on the OMAP4430 SDP, the
> following commands were used as supplied to load a uImage off the SD
> card into RAM at a different address to which it was built for, and
> execute it at that address:
> 
> mmcinit 0
> fatload mmc 0 0x80300000 uImage
> bootm 80300000
> 
> Whether the 'bootm' command then copied the image and called it there,
> or whether it executed it at 0x80300000 I've no idea - but why then
> load the image at a different address in the first place?

Maybe because the lower memory is already in use by other u-Boot related 
things, or more likely for no specific reasons at all.  But u-Boot 
_will_ relocate the image if it is not lot loaded at the address being 
recorded in its header as specified by the -a argument of mkimage.  And 
currently we use zreladdr for that when "make uImage" is used, meaning 
that by default no one is currently benefiting from the kernel 
decompressor relocation avoidance optimization on u-Boot.

Of course the u-Boot people are also claiming that u-Boot should take 
care of the kernel decompression not zImage, which I personally disagree 
with.


Nicolas

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 20:31 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Stephen Warren, Olof Johansson, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014201737.GW21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > 
> > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > people would refrain from going there for the moment.
> > > 
> > > Are you expecting everyone to change the way they load the zImage
> > > overnight then?
> > 
> > No, of course.  But adding restrictions in the kernel build because 
> > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > Those restrictions must be pushed towards the uImage encapsulation step, 
> > not higher the kernel config hierarchy.
> 
> You're not understanding again.
> 
> I'm talking about people who _explicitly_ load the zImage at a different
> address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> their setup will break unless they stop that behaviour, which takes
> away one of the advantages of using the zImage format.

Would you care to explain where you got this from?  Because I really do 
not understand what you're saying indeed.

With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
from where the decompressed kernel ends up.


Nicolas

^ permalink raw reply

* RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 20:45 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Russell King - ARM Linux, Olof Johansson, Arnd Bergmann,
	Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A362-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>

On Fri, 14 Oct 2011, Stephen Warren wrote:

> Nicolas Pitre wrote at Friday, October 14, 2011 2:06 PM:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load
> > address.  This is currently provided by the zreladdr value, whether or
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> > limitation.
> 
> Well, that value only comes from zreladdr when running U-Boot's mkimage from
> the kernel makefile rather than some other packaging script:-)

So what?  The whole idea of a kernel that can be loaded anywhere is to 
_not_ have its load address packaged into it.  The fact that you need a 
separate packaging step just for the specification of that address is 
rather silly.

> > Either u-Boot gets fixed so it can work with plain zImage (and this
> > certainly will happen once the pressure from people wanting a single
> > kernel to work on targets with different load addresses increase.
> > Tegra is one such example.
> 
> I proposed to solve it by adding a new image format within the uImage;
> "kernel-rel" rather than "kernel", where the load/entry address encoded
> into the uImage is specified relative to "start of SDRAM" rather than
> as an absolute address. This way, Tegra20 and Tegra30 will have the
> same layout of U-Boot location, uImage load address, kernel execute
> address etc. within SDRAM, but all those addresses might end up being
> based at physical address 0, or physical address 2G.
> 
> Here's the patch I proposed:
> 
> http://patchwork.ozlabs.org/patch/119017/

By all means, please push your patch upstream and free us from this 
u-Boot misery!

> What are your thoughts on this? I hope it will work for multi-SoC image
> across vendors, although I suppose finding a common load address across
> a bunch of different SoC's memory layouts might be tough?

No it is not.  We know where the decompressed kernel will end up 
relative to the start of RAM, so it is always safe to simply load the 
uImage 
there.  Better yet is to load it with an additional offset corresponding 
to the decompressed kernel size (plus some small gap) in order to allow 
the decompressor skip the relocation step.  This is all trivial to 
determine.

> I did originally briefly look into getting U-Boot to boot a zImage, but
> that looked like a far more invasive patch. There were rumours of some
> chip's custom U-Boot already having such support, but I couldn't find
> it, nor any evidence of such support in mainline U-Boot.

FRom my clone of the u-Boot repo:

$ git grep -l zImage
README
arch/arm/cpu/ixp/npe/include/IxTimeSyncAcc.h
arch/sh/lib/zimageboot.c
arch/x86/include/asm/zimage.h
arch/x86/lib/zimage.c
doc/README.mx35pdk
include/configs/apollon.h
include/configs/mpc7448hpc2.h
include/configs/mpr2.h
include/configs/ms7720se.h
include/configs/pxa255_idp.h
include/configs/trizepsiv.h

Even the name of some of those files clearly hints zImage support.

In any case, loading zImage should be even simpler than loading uImage.  
It is the same as loading uImage except that you just have to skip the 
checking and relocating steps.


Nicolas

^ permalink raw reply

* RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Stephen Warren @ 2011-10-14 21:01 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Russell King - ARM Linux, Olof Johansson, Arnd Bergmann,
	Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <alpine.LFD.2.02.1110141631530.17040-QuJgVwGFrdf/9pzu0YdTqQ@public.gmane.org>

Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
> On Fri, 14 Oct 2011, Stephen Warren wrote:
...
> > I did originally briefly look into getting U-Boot to boot a zImage, but
> > that looked like a far more invasive patch. There were rumours of some
> > chip's custom U-Boot already having such support, but I couldn't find
> > it, nor any evidence of such support in mainline U-Boot.
> 
> FRom my clone of the u-Boot repo:
> 
> $ git grep -l zImage
> README
> arch/sh/lib/zimageboot.c
> arch/x86/lib/zimage.c
> ...
> 
> Even the name of some of those files clearly hints zImage support.
> 
> In any case, loading zImage should be even simpler than loading uImage.
> It is the same as loading uImage except that you just have to skip the
> checking and relocating steps.

Just by way of background in case anyone is wondering why I wrote the
patch I did:

Those files both implement custom commands "zimageboot" and "zboot". I
was looking for integration with the existing "bootm" command.

The advantage of re-using "bootm" for this is that it already supports
all the stuff like setting up kernel command-lines, initrds, knowing how
to pass the FDT to the kernel, and whatever other OS-specific setup might
be required.

The disadvantage of adding zImage support to bootm is that I'd have to
teach a bunch of U-Boot image handling code about a new image format; it
already knows about "legacy uImage", "FIT" images, and I'd have to add a
third "zImage" format. Doing so would at least require adding a lot of
"case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
some kind of vtable for image formats to move all the image-format
knowledge into format-specific files, leaving the users of the images
with much smaller code.

I didn't feel like making such a large change. Hence, I chose to make a
small change to the existing uImage support.

Now admittedly I did say I didn't find any traces of zImage support, which
isn't what I'm saying here; I guess I forgot about the stuff I did find
soon after I chose the path of modifying the uImage formats.

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Russell King - ARM Linux @ 2011-10-14 21:13 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Arnd Bergmann, Stephen Warren, Olof Johansson, Peter De Schrijver,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org,
	Colin Cross (ccross@android.com), Erik Gilling
In-Reply-To: <alpine.LFD.2.02.1110141628230.17040@xanadu.home>

On Fri, Oct 14, 2011 at 04:31:12PM -0400, Nicolas Pitre wrote:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > > 
> > > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > > people would refrain from going there for the moment.
> > > > 
> > > > Are you expecting everyone to change the way they load the zImage
> > > > overnight then?
> > > 
> > > No, of course.  But adding restrictions in the kernel build because 
> > > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > > Those restrictions must be pushed towards the uImage encapsulation step, 
> > > not higher the kernel config hierarchy.
> > 
> > You're not understanding again.
> > 
> > I'm talking about people who _explicitly_ load the zImage at a different
> > address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> > their setup will break unless they stop that behaviour, which takes
> > away one of the advantages of using the zImage format.
> 
> Would you care to explain where you got this from?  Because I really do 
> not understand what you're saying indeed.

My I point out that it's you who decided that I was talking about u-boot
when I said no such thing in my message.  I merely pointed out about
those people who may be loading the zImage elsewhere in memory and using
that facility to cut down on the boot time.  u-boot can't load zImages
directly.

Yet you started nattering on about uboot - which we know is a pile of
crap for dealing with this stuff.

But ultimately, how people achieve the loading of the zImage is beyond
the scope of what I stated: whether that's not using u-boot but some
other boot loader, or maybe using mkimage outside of the kernel build,
or whatever.

> With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
> from where the decompressed kernel ends up.

You are correct for some values of 'different location' but not all -
and how can we know _what_ people are doing?  We don't.

#ifdef CONFIG_AUTO_ZRELADDR
                @ determine final kernel image address
                mov     r4, pc
                and     r4, r4, #0xf8000000
                add     r4, r4, #TEXT_OFFSET
#else
                ldr     r4, =zreladdr
#endif

So this means the decompressor _must_ run within the first 128MB chunk
of memory for the resulting kernel to be correctly placed at expected
place - at the beginning of system memory + TEXT_OFFSET.

Can we know that this is always the case?  I don't think so.
Can we expect there to be regressions if we force AUTO_ZRELADDR=y?  We'd
be stupid not to expect them.

^ permalink raw reply

* RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 21:28 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Russell King - ARM Linux, Erik Gilling, Arnd Bergmann,
	Peter De Schrijver, linux-kernel@vger.kernel.org,
	linux-tegra@vger.kernel.org, Colin Cross (ccross@android.com),
	Olof Johansson, linux-arm-kernel@lists.infradead.org
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A3A6@HQMAIL01.nvidia.com>

On Fri, 14 Oct 2011, Stephen Warren wrote:

> Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
> > On Fri, 14 Oct 2011, Stephen Warren wrote:
> ...
> > > I did originally briefly look into getting U-Boot to boot a zImage, but
> > > that looked like a far more invasive patch. There were rumours of some
> > > chip's custom U-Boot already having such support, but I couldn't find
> > > it, nor any evidence of such support in mainline U-Boot.
> > 
> > FRom my clone of the u-Boot repo:
> > 
> > $ git grep -l zImage
> > README
> > arch/sh/lib/zimageboot.c
> > arch/x86/lib/zimage.c
> > ...
> > 
> > Even the name of some of those files clearly hints zImage support.
> > 
> > In any case, loading zImage should be even simpler than loading uImage.
> > It is the same as loading uImage except that you just have to skip the
> > checking and relocating steps.
> 
> Just by way of background in case anyone is wondering why I wrote the
> patch I did:
> 
> Those files both implement custom commands "zimageboot" and "zboot". I
> was looking for integration with the existing "bootm" command.
> 
> The advantage of re-using "bootm" for this is that it already supports
> all the stuff like setting up kernel command-lines, initrds, knowing how
> to pass the FDT to the kernel, and whatever other OS-specific setup might
> be required.

Absolutely.  And that is a must-have.

> The disadvantage of adding zImage support to bootm is that I'd have to
> teach a bunch of U-Boot image handling code about a new image format; it
> already knows about "legacy uImage", "FIT" images, and I'd have to add a
> third "zImage" format. Doing so would at least require adding a lot of
> "case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
> some kind of vtable for image formats to move all the image-format
> knowledge into format-specific files, leaving the users of the images
> with much smaller code.
> 
> I didn't feel like making such a large change. Hence, I chose to make a
> small change to the existing uImage support.

And that change is certainly valuable.  Some people do want the extra 
feature from the uImage format.  REmoving its absolute address 
limitation is a good thing.

However, also supporting zImage directly on ARM doesn't have to be that 
hard.  Instead of adding "case IMAGE_FORMAT_ZIMAGE" everywhere, all you 
would have to do is to load zImage in memory according to the load 
address provided as an argument to the command, and then fake a uImage 
load by artificially creating the uImage header at run time with the 
data that u-Boot needs later on.


Nicolas

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Rob Herring @ 2011-10-14 22:06 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Nicolas Pitre, Russell King - ARM Linux, Erik Gilling,
	Arnd Bergmann, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Olof Johansson,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A3A6-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>

On 10/14/2011 04:01 PM, Stephen Warren wrote:
> Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
>> On Fri, 14 Oct 2011, Stephen Warren wrote:
> ...
>>> I did originally briefly look into getting U-Boot to boot a zImage, but
>>> that looked like a far more invasive patch. There were rumours of some
>>> chip's custom U-Boot already having such support, but I couldn't find
>>> it, nor any evidence of such support in mainline U-Boot.
>>
>> FRom my clone of the u-Boot repo:
>>
>> $ git grep -l zImage
>> README
>> arch/sh/lib/zimageboot.c
>> arch/x86/lib/zimage.c
>> ...
>>
>> Even the name of some of those files clearly hints zImage support.
>>
>> In any case, loading zImage should be even simpler than loading uImage.
>> It is the same as loading uImage except that you just have to skip the
>> checking and relocating steps.
> 
> Just by way of background in case anyone is wondering why I wrote the
> patch I did:
> 
> Those files both implement custom commands "zimageboot" and "zboot". I
> was looking for integration with the existing "bootm" command.
> 
> The advantage of re-using "bootm" for this is that it already supports
> all the stuff like setting up kernel command-lines, initrds, knowing how
> to pass the FDT to the kernel, and whatever other OS-specific setup might
> be required.
> 
> The disadvantage of adding zImage support to bootm is that I'd have to
> teach a bunch of U-Boot image handling code about a new image format; it
> already knows about "legacy uImage", "FIT" images, and I'd have to add a
> third "zImage" format. Doing so would at least require adding a lot of
> "case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
> some kind of vtable for image formats to move all the image-format
> knowledge into format-specific files, leaving the users of the images
> with much smaller code.
> 
> I didn't feel like making such a large change. Hence, I chose to make a
> small change to the existing uImage support.
> 
> Now admittedly I did say I didn't find any traces of zImage support, which
> isn't what I'm saying here; I guess I forgot about the stuff I did find
> soon after I chose the path of modifying the uImage formats.
> 

FYI, this exact topic has been discussed on the Linaro weekly
boot-architecture call and list. The discussion has been more general in
terms of what can be done to make installing and updating kernels
easier/work for distros.

https://wiki.linaro.org/OfficeofCTO/BootArchitecture

Rob

^ permalink raw reply

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Nicolas Pitre @ 2011-10-14 22:26 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Stephen Warren, Olof Johansson, Peter De Schrijver,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org),
	Erik Gilling
In-Reply-To: <20111014211311.GY21648-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:31:12PM -0400, Nicolas Pitre wrote:
> > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > 
> > > On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > > > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > > > 
> > > > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > > > people would refrain from going there for the moment.
> > > > > 
> > > > > Are you expecting everyone to change the way they load the zImage
> > > > > overnight then?
> > > > 
> > > > No, of course.  But adding restrictions in the kernel build because 
> > > > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > > > Those restrictions must be pushed towards the uImage encapsulation step, 
> > > > not higher the kernel config hierarchy.
> > > 
> > > You're not understanding again.
> > > 
> > > I'm talking about people who _explicitly_ load the zImage at a different
> > > address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> > > their setup will break unless they stop that behaviour, which takes
> > > away one of the advantages of using the zImage format.
> > 
> > Would you care to explain where you got this from?  Because I really do 
> > not understand what you're saying indeed.
> 
> My I point out that it's you who decided that I was talking about u-boot
> when I said no such thing in my message.  I merely pointed out about
> those people who may be loading the zImage elsewhere in memory and using
> that facility to cut down on the boot time.  u-boot can't load zImages
> directly.
> 
> Yet you started nattering on about uboot - which we know is a pile of
> crap for dealing with this stuff.

OK, sorry for confusing matters.

From your statement I inferred that making AUTO_ZRELADDR=y would be 
useless, because this is actually true with u-Boot.  Making 
AUTO_ZRELADDR the default allows for cleaning up zreladdr away, but that 
will break "make uImage".  I've yet to find the best way around that.

> > With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
> > from where the decompressed kernel ends up.
> 
> You are correct for some values of 'different location' but not all -
> and how can we know _what_ people are doing?  We don't.
> 
> #ifdef CONFIG_AUTO_ZRELADDR
>                 @ determine final kernel image address
>                 mov     r4, pc
>                 and     r4, r4, #0xf8000000
>                 add     r4, r4, #TEXT_OFFSET
> #else
>                 ldr     r4, =zreladdr
> #endif
> 
> So this means the decompressor _must_ run within the first 128MB chunk
> of memory for the resulting kernel to be correctly placed at expected
> place - at the beginning of system memory + TEXT_OFFSET.
> 
> Can we know that this is always the case?  I don't think so.

This is certainly the case for a big chunk of legacy ARM machines that 
don't have more than 128MB of RAM.  Discontigous memory banks may break 
that assumption but that's still a large limiting factor.

> Can we expect there to be regressions if we force AUTO_ZRELADDR=y?  We'd
> be stupid not to expect them.

They should be expected indeed.  However we must weight the benefits 
against the possible regressions.

For example, on X86 they decided at some point that the zImage would no 
longer include the floppy boot sector with kernel loading code.  This 
was an explicit regression because simply copying zImage to a floppy and 
sticking that floppy into a PC no longer boots the kernel.  I'm sure 
some people were affected by that, and the official word was that you 
now have to use a proper boot loader, period.

So far, I've never seen any ARM machine where the kernel is loaded more 
than 64MB away from start of RAM.  And that 64MB happened only once and 
that was from my own doing.  Most people load zImage much closer to 
start of RAM, and almost all of them simply load zImage at zreladdr.  
Or worse, they wrap it into a uImage, load it higher thinking it is 
good, and u-Boot copies it to zreladdr because that's what 'make uImage' 
uses, forcing zImage to make a second copy.

Between you and I, we've seen quite a lot of ARM setups after all those 
years. We should be able to guess the probability for causing regression 
with AUTO_ZRELADDR=y by default.  Personally I'd say that, while not 
impossible, the probability is close to zero for 99% of the cases.


Nicolas

^ permalink raw reply

* [PATCH] ASoC: Tegra: sparse cleanup
From: Olof Johansson @ 2011-10-14 22:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, swarren-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Olof Johansson

Fixes the following sparse warnings:

sound/soc/tegra/tegra_das.c:215:8: warning: Using plain integer as NULL pointer
sound/soc/tegra/tegra_das.c:237:8: warning: Using plain integer as NULL pointer
sound/soc/tegra/tegra_pcm.c:370:32: warning: symbol 'tegra_pcm_platform' was not declared. Should it be static?

Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
---
 sound/soc/tegra/tegra_das.c |    4 ++--
 sound/soc/tegra/tegra_pcm.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/tegra/tegra_das.c b/sound/soc/tegra/tegra_das.c
index 9f24ef7..3b55a44 100644
--- a/sound/soc/tegra/tegra_das.c
+++ b/sound/soc/tegra/tegra_das.c
@@ -212,7 +212,7 @@ err_release:
 	release_mem_region(res->start, resource_size(res));
 err_free:
 	kfree(das);
-	das = 0;
+	das = NULL;
 exit:
 	return ret;
 }
@@ -234,7 +234,7 @@ static int __devexit tegra_das_remove(struct platform_device *pdev)
 	release_mem_region(res->start, resource_size(res));
 
 	kfree(das);
-	das = 0;
+	das = NULL;
 
 	return 0;
 }
diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
index c7cfd96..436def1 100644
--- a/sound/soc/tegra/tegra_pcm.c
+++ b/sound/soc/tegra/tegra_pcm.c
@@ -367,7 +367,7 @@ static void tegra_pcm_free(struct snd_pcm *pcm)
 	tegra_pcm_deallocate_dma_buffer(pcm, SNDRV_PCM_STREAM_PLAYBACK);
 }
 
-struct snd_soc_platform_driver tegra_pcm_platform = {
+static struct snd_soc_platform_driver tegra_pcm_platform = {
 	.ops		= &tegra_pcm_ops,
 	.pcm_new	= tegra_pcm_new,
 	.pcm_free	= tegra_pcm_free,
-- 
1.7.4.1

^ permalink raw reply related

* RE: [PATCH] ASoC: Tegra: sparse cleanup
From: Stephen Warren @ 2011-10-14 22:59 UTC (permalink / raw)
  To: Olof Johansson, Mark Brown
  Cc: Liam Girdwood,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
In-Reply-To: <1318632859-14796-1-git-send-email-olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Olof Johansson wrote at Friday, October 14, 2011 4:54 PM:
> Fixes the following sparse warnings:
> 
> sound/soc/tegra/tegra_das.c:215:8: warning: Using plain integer as NULL pointer
> sound/soc/tegra/tegra_das.c:237:8: warning: Using plain integer as NULL pointer
> sound/soc/tegra/tegra_pcm.c:370:32: warning: symbol 'tegra_pcm_platform' was not declared. Should it
> be static?
> 
> Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Grumble, in C++ 0==NULL... :-)

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH] ASoC: Tegra: sparse cleanup
From: Olof Johansson @ 2011-10-14 23:01 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Mark Brown, Liam Girdwood,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A3ED-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>

On Fri, Oct 14, 2011 at 3:59 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Olof Johansson wrote at Friday, October 14, 2011 4:54 PM:
>> Fixes the following sparse warnings:
>>
>> sound/soc/tegra/tegra_das.c:215:8: warning: Using plain integer as NULL pointer
>> sound/soc/tegra/tegra_das.c:237:8: warning: Using plain integer as NULL pointer
>> sound/soc/tegra/tegra_pcm.c:370:32: warning: symbol 'tegra_pcm_platform' was not declared. Should it
>> be static?
>>
>> Signed-off-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
>
> Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> Grumble, in C++ 0==NULL... :-)

In C too, it's just sparse that is extra picky.


-Olof

^ permalink raw reply

* Re: Adding board support
From: Thierry Reding @ 2011-10-14 23:24 UTC (permalink / raw)
  To: Stephen Warren; +Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF173BE1A33E-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 5622 bytes --]

* Stephen Warren wrote:
> Thierry Reding wrote at Friday, October 14, 2011 12:59 PM:
> > * Stephen Warren wrote:
[...]
> > U-Boot mainline support is another point on my TODO list. Getting the latest
> > mainline code with the patches you mention (I assume you are referring to the
> > patch series by Simon Glass and Tom Warren) to work would be a good step in
> > that direction.
> 
> I branched from: git://git.denx.de/u-boot.git master at commit
> 0841ca90f22d73b0ea4642ef1ce33d879bb2f3ff.
> 
> I applied the following:
> 
> http://patchwork.ozlabs.org/patch/115862/
> http://patchwork.ozlabs.org/patch/115864/
> http://patchwork.ozlabs.org/patch/115865/
> http://patchwork.ozlabs.org/patch/115860/
> http://patchwork.ozlabs.org/patch/115863/
> http://patchwork.ozlabs.org/patch/115861/
> 
> http://patchwork.ozlabs.org/patch/119321/
> http://patchwork.ozlabs.org/patch/119322/
> http://patchwork.ozlabs.org/patch/119323/
> http://patchwork.ozlabs.org/patch/119324/
> http://patchwork.ozlabs.org/patch/119325/
> 
> http://patchwork.ozlabs.org/patch/118184/
> 
> I applied the following to hack the default environment so I could boot from
> SD cards layed out how mine are; you'll probably need to tweak this a bunch:
> 
> diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
> index 07546a4..f30f569 100644
> --- a/include/configs/tegra2-common.h
> +++ b/include/configs/tegra2-common.h
> @@ -104,9 +104,19 @@
>  
>  /* Environment information */
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> -       "console=ttyS0,115200n8\0" \
> -       "mem=" TEGRA2_SYSMEM "\0" \
> -       "smpflag=smp\0" \
> +       "bootcmd=run usb0_boot ; run usb1_boot ; run mmc1_boot ; run mmc0_boot\0" \
> +       "bootdelay=2\0" \
> +       "loadaddr=0x00500000\0" \
> +       "scriptaddr=0x408000\0" \
> +       "script_img=/u-boot/boot.scr.uimg\0" \
> +       "mmc0_boot=setenv devnum 0; run mmc_boot;\0" \
> +       "mmc1_boot=setenv devnum 1; run mmc_boot;\0" \
> +       "usb0_boot=usb start 0; run usb_boot;\0" \
> +       "usb1_boot=usb start 1; run usb_boot;\0" \
> +       "scr_boot=fatload ${devtype} ${devnum}:c ${scriptaddr} ${script_img};source ${scriptaddr};read ${devtype} ${devnum}:${kernelpart} ${scriptaddr} 0 10;source ${scriptaddr};\0" \
> +       "mmc_boot=mmc dev ${devnum};setenv devtype mmc;setenv devname mmcblk${devnum}p;run scr_boot;\0" \
> +       "usb_boot=setenv devtype usb;setenv devnum 0;setenv devname sda;run scr_boot;\0" \
> +       "platform_extras=lp0_vec=0x2000@0x1C406000 kcrashmem=0x100000@0x02000000 mem=384M@0M nvmem=128M@384M mem=511M@512M\0" \
>  
>  #define CONFIG_LOADADDR                0x408000        /* def. location for kernel */
>  #define CONFIG_BOOTDELAY       2               /* -1 to disable auto boot */
> 
> In order to actually use the resultant u-boot.bin, it's pretty simple if you
> already have nvflash working with burn fastboot.
> 
> 1) Edit flash.cfg (or whatever config file you pass to nvflash to define
> The partitions and their content) and replace the filename entry in the
> EBT partition with u-boot.bin.
> 
> 2) I personally remove all the partition entries in flash.cfg except for
> BCT, PT, EBT. This will avoid nvflash flashing your Android/... filesystem.
> If you want your filesystem in the same flash, don't do this.
> 
> Then, run nvflash just like you would normally.

Okay, I've been able to build U-Boot and setup some scripts that should
automate the nvflash procedure according to your instructions. I'll have to
wait until I get back to work on Monday to see whether it actually works,
though.

> > Have you done any testing with the NVIDIA binary blobs when booting this way?
> > The latest information I have from our NVIDIA contacts is that fastboot or
> > quickboot are required, for some reason, to make HW accelerated video
> > decoding and 3D rendering work.
> 
> It's possible the binary drivers accidentally rely on the bootloader
> having configured some piece of HW.
> 
> However, in general, the bootloader product you use shouldn't affect
> whether the binary drivers work.
> 
> In particular, the binary drivers certainly work after the ChromeOS
> U-Boot boots a board.
> 
> Coincidentally, right before seeing your email, I just tried mainline
> U-Boot on Seaboard with our Linux4Tegra alpha 1 release, and while X
> started, I couldn't see anything on screen. This did previously work
> with some old version of ChromeOS's U-Boot. So, there's certainly some
> missing HW initialization somewhere.

Right, I'll probably run into the same problems then. But if it can actually
make the system boot, it's enough to prepare some patches on top for mainline
support.

I've just caught up with most of my email backlog and I'm looking forward to
testing the device-tree support in U-Boot that's recently been posted.

> BTW, you might also want to take a look at NVIDIA's web forums:
> 
> http://developer.nvidia.com/beta-forum
> 
> While I'm personally happy to answer your questions here, (I work for
> a group dedicated to making general Linux support on Tegra) you may find
> more people aimed at this kind of support on those forums. I'm not
> active on those forums though.

I'm not a big fan of forums, but I guess I should check it out. A co-worker
has actually done more work with the binary drivers and I don't know how much
I will be involved there either, but I will make sure to pass the information
on.

> I hope this all helps!

Yes, very helpful indeed! Thanks again.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* [PATCH 1/1] ARM: tegra: paz00: Fix board pinmux table.
From: Leon Romanovsky @ 2011-10-15 15:18 UTC (permalink / raw)
  To: olof-nZhT3qVonbNeoWH0uzbU5w, ccross-z5hGa2qSFaRBDgjK7y7TUQ
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

This fix updates the CDEV1 pinmux for the paz00 board to be as in the
Harmony board. Paz00 board is originally based on Harmony design.

Signed-off-by: Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>
---
 arch/arm/mach-tegra/board-paz00-pinmux.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
index bdd2627..7de8182 100644
--- a/arch/arm/mach-tegra/board-paz00-pinmux.c
+++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
@@ -27,7 +27,7 @@ static struct tegra_pingroup_config paz00_pinmux[] = {
 	{TEGRA_PINGROUP_ATC,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
 	{TEGRA_PINGROUP_ATD,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
 	{TEGRA_PINGROUP_ATE,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
-	{TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
+	{TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
 	{TEGRA_PINGROUP_CDEV2, TEGRA_MUX_PLLP_OUT4,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_NORMAL},
 	{TEGRA_PINGROUP_CRTP,  TEGRA_MUX_CRT,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},
 	{TEGRA_PINGROUP_CSUS,  TEGRA_MUX_PLLC_OUT1,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default
From: Tixy @ 2011-10-15 15:29 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Stephen Warren, Erik Gilling, Arnd Bergmann, Nicolas Pitre,
	Peter De Schrijver, linux-kernel@vger.kernel.org, Olof Johansson,
	Colin Cross (ccross@android.com), linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20111014201953.GX21648@n2100.arm.linux.org.uk>

On Fri, 2011-10-14 at 21:19 +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 14, 2011 at 01:16:27PM -0700, Stephen Warren wrote:
> > Russell King wrote at Friday, October 14, 2011 2:12 PM:
> > > I don't think that's so with the various flavours of platform specific
> > > uboot which float around.  For instance, on the OMAP4430 SDP, the
> > > following commands were used as supplied to load a uImage off the SD
> > > card into RAM at a different address to which it was built for, and
> > > execute it at that address:
> > > 
> > > mmcinit 0
> > > fatload mmc 0 0x80300000 uImage
> > > bootm 80300000
> > > 
> > > Whether the 'bootm' command then copied the image and called it there,
> > > or whether it executed it at 0x80300000 I've no idea - but why then
> > > load the image at a different address in the first place?
> > 
> > Yes, U-Boot's image handling looks at the image at 80300000, extracts the
> > "load address" from the image header, memcpy()'s the image to that address,
> > then jumps to the "entry address" in the image header.
> 
> Ah, so specifying that address is just a total waste of space then,
> because you might as well specify an address which results in the
> copy not being necessary.
> 
> However, I'd expect that uboot is dumb enough to still do the copy
> irrespective of whether its already in the right place.

U-Boot doesn't do an unnecessary copy if the body of the image is
already at the load address specified in the image header.

Unfortunately, it also doesn't do a copy if the header of the image is
at the load address, (I guess to support images constructed with an
embedded header). This means that

  fatload mmc 0 0x80300000 uImage
  bootm 0x80300000

will crash as it will attempt to boot Linux by executing the image
header. To successfully boot without requiring a relocation we would
need:

  fatload mmc 0 0x80300000-sizeof(header) uImage
  bootm 0x80300000

-- 
Tixy

^ permalink raw reply

* Re: [PATCH 1/1] ARM: tegra: paz00: Fix board pinmux table.
From: Marc Dietrich @ 2011-10-15 16:34 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: olof-nZhT3qVonbNeoWH0uzbU5w, ccross-z5hGa2qSFaRBDgjK7y7TUQ,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1318691913-9597-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>

Hi Leon,

Am Samstag 15 Oktober 2011, 17:18:33 schrieb Leon Romanovsky:
> This fix updates the CDEV1 pinmux for the paz00 board to be as in the
> Harmony board. Paz00 board is originally based on Harmony design.

the fact that this patch makes sound work on paz00 does not necessary mean
that this is the "right thing" to do. First, the initial state should be
TRISTATE, because sound layer should switch between NORMAL/TRISTATE 
automaticly to save some power. Second, Android has TEGRA_PUPD_PULL_DOWN 
with sound working and I didn't found out yet why this also works.

So there is something strange going on and we should first find out what
it is. Furthermore, there is no codec (ALC5632) and no glue for paz00 yet 
in the mainline kernel (and of course no board support for it) so this can 
still wait IMHO.

I will include your patch in the series which will add sound support once
the fog has lifted a bit.

Thanks

Marc

> Signed-off-by: Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>
> ---
>  arch/arm/mach-tegra/board-paz00-pinmux.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
> index bdd2627..7de8182 100644
> --- a/arch/arm/mach-tegra/board-paz00-pinmux.c
> +++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
> @@ -27,7 +27,7 @@ static struct tegra_pingroup_config paz00_pinmux[] = {
>  	{TEGRA_PINGROUP_ATC,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
>  	{TEGRA_PINGROUP_ATD,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
>  	{TEGRA_PINGROUP_ATE,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
> -	{TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
> +	{TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
>  	{TEGRA_PINGROUP_CDEV2, TEGRA_MUX_PLLP_OUT4,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_NORMAL},
>  	{TEGRA_PINGROUP_CRTP,  TEGRA_MUX_CRT,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},
>  	{TEGRA_PINGROUP_CSUS,  TEGRA_MUX_PLLC_OUT1,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox