Message ID | 1405599840-11984-1-git-send-email-valentin.longchamp@keymile.com |
---|---|
State | Superseded |
Delegated to: | York Sun |
Headers | show |
Dear Valentin, In message <1405599840-11984-1-git-send-email-valentin.longchamp@keymile.com> you wrote: > When u-boot initializes the RAM (early in boot) it looks for the "pram" > env variable to know which is area it cannot use. > > At this early boot stage, the "pram" env variable is not avaible yet > since it gets computed in set_km_env that gets called AFTER the RAM > initialization. If the "pram" env variable is not found, the default > CONFIG_PRAM value is used. Note that I am not objecting against this patch, but I highly recommend to fix your board - RAM initialization is actually pretty late in the init sequence, and you should have a valid envionment long before. Best regards, Wolfgang Denk
Hello Wolfgang, On 07/17/2014 02:47 PM, Wolfgang Denk wrote: > Dear Valentin, > > In message <1405599840-11984-1-git-send-email-valentin.longchamp@keymile.com> you wrote: >> When u-boot initializes the RAM (early in boot) it looks for the "pram" >> env variable to know which is area it cannot use. >> >> At this early boot stage, the "pram" env variable is not avaible yet >> since it gets computed in set_km_env that gets called AFTER the RAM >> initialization. If the "pram" env variable is not found, the default >> CONFIG_PRAM value is used. > > Note that I am not objecting against this patch, but I highly > recommend to fix your board - RAM initialization is actually pretty > late in the init sequence, and you should have a valid envionment long > before. > Maybe my commit message is unclear about this. You are right, at the RAM initialization time, there is a valid environment, and that's the case on our board too. However, at the very first boot on a board, the environment is empty (or unvalid) and the default one is used where this "pram" env variable is not defined. That's why the CONFIG_PRAM is used in this case and it should be defined. This is not going to be the case at any later boot if a valid environment (with pram defined) is found and used. Best regards, Valentin
On 07/18/2014 02:10 AM, Valentin Longchamp wrote: > Hello Wolfgang, > > On 07/17/2014 02:47 PM, Wolfgang Denk wrote: >> Dear Valentin, >> >> In message <1405599840-11984-1-git-send-email-valentin.longchamp@keymile.com> you wrote: >>> When u-boot initializes the RAM (early in boot) it looks for the "pram" >>> env variable to know which is area it cannot use. >>> >>> At this early boot stage, the "pram" env variable is not avaible yet >>> since it gets computed in set_km_env that gets called AFTER the RAM >>> initialization. If the "pram" env variable is not found, the default >>> CONFIG_PRAM value is used. >> >> Note that I am not objecting against this patch, but I highly >> recommend to fix your board - RAM initialization is actually pretty >> late in the init sequence, and you should have a valid envionment long >> before. >> > > Maybe my commit message is unclear about this. You are right, at the RAM > initialization time, there is a valid environment, and that's the case on our > board too. > > However, at the very first boot on a board, the environment is empty (or > unvalid) and the default one is used where this "pram" env variable is not > defined. That's why the CONFIG_PRAM is used in this case and it should be > defined. This is not going to be the case at any later boot if a valid > environment (with pram defined) is found and used. > > Best regards, > Valentin, How about you update the commit message to explain you are defining the default CONFIG_PRAM for your board in case "pram" variable is not available? York
Hi York, On 08/12/2014 08:03 PM, York Sun wrote: > On 07/18/2014 02:10 AM, Valentin Longchamp wrote: >> Hello Wolfgang, >> >> On 07/17/2014 02:47 PM, Wolfgang Denk wrote: >>> Dear Valentin, >>> >>> In message <1405599840-11984-1-git-send-email-valentin.longchamp@keymile.com> you wrote: >>>> When u-boot initializes the RAM (early in boot) it looks for the "pram" >>>> env variable to know which is area it cannot use. >>>> >>>> At this early boot stage, the "pram" env variable is not avaible yet >>>> since it gets computed in set_km_env that gets called AFTER the RAM >>>> initialization. If the "pram" env variable is not found, the default >>>> CONFIG_PRAM value is used. >>> >>> Note that I am not objecting against this patch, but I highly >>> recommend to fix your board - RAM initialization is actually pretty >>> late in the init sequence, and you should have a valid envionment long >>> before. >>> >> >> Maybe my commit message is unclear about this. You are right, at the RAM >> initialization time, there is a valid environment, and that's the case on our >> board too. >> >> However, at the very first boot on a board, the environment is empty (or >> unvalid) and the default one is used where this "pram" env variable is not >> defined. That's why the CONFIG_PRAM is used in this case and it should be >> defined. This is not going to be the case at any later boot if a valid >> environment (with pram defined) is found and used. >> >> Best regards, >> > > > Valentin, > > How about you update the commit message to > explain you are defining the default CONFIG_PRAM for your board in case "pram" > variable is not available? > OK, I will do ... even though the commit message already does already state that, just with more details when and why (there was just a missing precision here, "when no valid environment was found") the "pram" env variable can be undefined and why it is important for us to have such a value. But maybe that's too much information ? Valentin
diff --git a/include/configs/km/km-powerpc.h b/include/configs/km/km-powerpc.h index 763c5ba..eb85a74 100644 --- a/include/configs/km/km-powerpc.h +++ b/include/configs/km/km-powerpc.h @@ -59,8 +59,9 @@ #define CONFIG_KM_PHRAM 0x100000 /* resereved pram area at the end of memroy [hex] */ #define CONFIG_KM_RESERVED_PRAM 0x0 -/* enable protected RAM */ -#define CONFIG_PRAM 0 +/* set the default PRAM value to at least PNVRAM + PHRAM when pram env variable + * is not valid yet, which is the case for when u-boot copies itself to RAM */ +#define CONFIG_PRAM ((CONFIG_KM_PNVRAM + CONFIG_KM_PHRAM)>>10) #define CONFIG_KM_CRAMFS_ADDR 0x800000 #define CONFIG_KM_KERNEL_ADDR 0x400000 /* 3968Kbytes */