diff mbox

[U-Boot,1/2] configs: ti_armv7_keystone2: fix initrd address at args_ramfs

Message ID 1438701376-12169-2-git-send-email-vitalya@ti.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Vitaly Andrianov Aug. 4, 2015, 3:16 p.m. UTC
This patch set the correct initrd address to much rdaddr, which used at
get_fs_ramfs commad.

Fixes: 3e97f0b63c8e ("configs: ti_armv7_keystone2: switch addresses
to generic addresses")

Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
---
 include/configs/ti_armv7_keystone2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Rini Aug. 4, 2015, 3:23 p.m. UTC | #1
On Tue, Aug 04, 2015 at 11:16:15AM -0400, Vitaly Andrianov wrote:

> This patch set the correct initrd address to much rdaddr, which used at
> get_fs_ramfs commad.
> 
> Fixes: 3e97f0b63c8e ("configs: ti_armv7_keystone2: switch addresses
> to generic addresses")
> 
> Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> ---
>  include/configs/ti_armv7_keystone2.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
> index 7b67620..454f1df 100644
> --- a/include/configs/ti_armv7_keystone2.h
> +++ b/include/configs/ti_armv7_keystone2.h
> @@ -253,7 +253,7 @@
>  	"init_ramfs=run args_all args_ramfs get_fs_ramfs\0"		\
>  	"args_ramfs=setenv bootargs ${bootargs} "			\
>  		"rdinit=/sbin/init rw root=/dev/ram0 "			\
> -		"initrd=0x802000000,9M\0"				\
> +		"initrd=0x808080000,9M\0"				\

There's two related problems here.  Making initrd match ${rdaddr} would
be best done by doing "initrd=${rdaddr}" here, same as whacking in the
previous ${bootargs}.  Second
$ grep rdaddr include/configs/ti_armv7_common.h
	"rdaddr=0x88080000\0" \

So you aren't making it match here either.  Finally (and please test
this..), do you _need_ to pass in the size parameter here?  I'm assuming
that this is _not_ a uImage'd ramdisk here but instead a raw one so we
end up with the case of the kernel would know where the ramdisk starts
but would assume a max size of whatever it was compiled with and then
find an incomplete rootfs, right?  If so then yes, that's fine, we can
continue passing in (and updating in the second patch as you are) the
size parameter, I just want to be sure we aren't missing out on some
"new" smarts that exist in the kernel.  Thanks!

>  	"no_post=1\0"							\
>  	"mtdparts=mtdparts=davinci_nand.0:"				\
>  		"1024k(bootloader)ro,512k(params)ro,-(ubifs)\0"
> -- 
> 1.9.1
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Vitaly Andrianov Aug. 4, 2015, 3:45 p.m. UTC | #2
On 08/04/2015 11:23 AM, Tom Rini wrote:
> On Tue, Aug 04, 2015 at 11:16:15AM -0400, Vitaly Andrianov wrote:
>
>> This patch set the correct initrd address to much rdaddr, which used at
>> get_fs_ramfs commad.
>>
>> Fixes: 3e97f0b63c8e ("configs: ti_armv7_keystone2: switch addresses
>> to generic addresses")
>>
>> Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
>> ---
>>   include/configs/ti_armv7_keystone2.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
>> index 7b67620..454f1df 100644
>> --- a/include/configs/ti_armv7_keystone2.h
>> +++ b/include/configs/ti_armv7_keystone2.h
>> @@ -253,7 +253,7 @@
>>   	"init_ramfs=run args_all args_ramfs get_fs_ramfs\0"		\
>>   	"args_ramfs=setenv bootargs ${bootargs} "			\
>>   		"rdinit=/sbin/init rw root=/dev/ram0 "			\
>> -		"initrd=0x802000000,9M\0"				\
>> +		"initrd=0x808080000,9M\0"				\
>
> There's two related problems here.  Making initrd match ${rdaddr} would
> be best done by doing "initrd=${rdaddr}" here, same as whacking in the

For keystone devices we load filesystem at ${rdaddr}, but need to pass
to the kernel initrd=<${rdaddr} + 0x780000000)>, which is LPAE address.

How can I do that w/o using absolute address?

> previous ${bootargs}.  Second
> $ grep rdaddr include/configs/ti_armv7_common.h
> 	"rdaddr=0x88080000\0" \
>
> So you aren't making it match here either.  Finally (and please test
> this..), do you _need_ to pass in the size parameter here?  I'm assuming
> that this is _not_ a uImage'd ramdisk here but instead a raw one so we
> end up with the case of the kernel would know where the ramdisk starts
> but would assume a max size of whatever it was compiled with and then
> find an incomplete rootfs, right?  If so then yes, that's fine, we can
> continue passing in (and updating in the second patch as you are) the
> size parameter, I just want to be sure we aren't missing out on some
> "new" smarts that exist in the kernel.  Thanks!

You are right. That is to load a standalone file system to the ram and
let kernel to know the start address.

Thanks

>
>>   	"no_post=1\0"							\
>>   	"mtdparts=mtdparts=davinci_nand.0:"				\
>>   		"1024k(bootloader)ro,512k(params)ro,-(ubifs)\0"
>> --
>> 1.9.1
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
Tom Rini Aug. 4, 2015, 4:16 p.m. UTC | #3
On Tue, Aug 04, 2015 at 11:45:17AM -0400, Vitaly Andrianov wrote:
> 
> 
> On 08/04/2015 11:23 AM, Tom Rini wrote:
> >On Tue, Aug 04, 2015 at 11:16:15AM -0400, Vitaly Andrianov wrote:
> >
> >>This patch set the correct initrd address to much rdaddr, which used at
> >>get_fs_ramfs commad.
> >>
> >>Fixes: 3e97f0b63c8e ("configs: ti_armv7_keystone2: switch addresses
> >>to generic addresses")
> >>
> >>Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> >>---
> >>  include/configs/ti_armv7_keystone2.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
> >>index 7b67620..454f1df 100644
> >>--- a/include/configs/ti_armv7_keystone2.h
> >>+++ b/include/configs/ti_armv7_keystone2.h
> >>@@ -253,7 +253,7 @@
> >>  	"init_ramfs=run args_all args_ramfs get_fs_ramfs\0"		\
> >>  	"args_ramfs=setenv bootargs ${bootargs} "			\
> >>  		"rdinit=/sbin/init rw root=/dev/ram0 "			\
> >>-		"initrd=0x802000000,9M\0"				\
> >>+		"initrd=0x808080000,9M\0"				\
> >
> >There's two related problems here.  Making initrd match ${rdaddr} would
> >be best done by doing "initrd=${rdaddr}" here, same as whacking in the
> 
> For keystone devices we load filesystem at ${rdaddr}, but need to pass
> to the kernel initrd=<${rdaddr} + 0x780000000)>, which is LPAE address.

Why do you need to do that?  Shouldn't the kernel be able to move things
around as needed?  But if we must then please do some expr to do that
addition so that if people change rdaddr they don't get unexpected
breakage.
Tom Rini Aug. 13, 2015, 1:23 p.m. UTC | #4
On Tue, Aug 04, 2015 at 11:16:15AM -0400, Vitaly Andrianov wrote:

> This patch set the correct initrd address to much rdaddr, which used at
> get_fs_ramfs commad.
> 
> Fixes: 3e97f0b63c8e ("configs: ti_armv7_keystone2: switch addresses
> to generic addresses")
> 
> Signed-off-by: Vitaly Andrianov <vitalya@ti.com>

So after thinking about this more, I bet that no, on a 32bit platform
setexpr probably does not do the right thing with 64bit math so rather
than doing a lot of hoop jumping for a simple thing, lets just fix this
the simple way.

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
index 7b67620..454f1df 100644
--- a/include/configs/ti_armv7_keystone2.h
+++ b/include/configs/ti_armv7_keystone2.h
@@ -253,7 +253,7 @@ 
 	"init_ramfs=run args_all args_ramfs get_fs_ramfs\0"		\
 	"args_ramfs=setenv bootargs ${bootargs} "			\
 		"rdinit=/sbin/init rw root=/dev/ram0 "			\
-		"initrd=0x802000000,9M\0"				\
+		"initrd=0x808080000,9M\0"				\
 	"no_post=1\0"							\
 	"mtdparts=mtdparts=davinci_nand.0:"				\
 		"1024k(bootloader)ro,512k(params)ro,-(ubifs)\0"