[PATCH] added check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART definition

[PATCH] added check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART definition

[Manuel Reis]

Check whether user has explicitly defined device and partition where
environment file will be located. Voids the need to set such partition
as bootable to work with the 'dev:auto'

Signed-off-by: Manuel Reis <mluis.reis@gmail.com>
Cc: Patrick Delaunay <patrick.delaunay@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
---

 board/st/stm32mp1/stm32mp1.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 4553329b25..4dda70740d 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -868,6 +868,11 @@ const char *env_ext4_get_intf(void)
 
 const char *env_ext4_get_dev_part(void)
 {
+	static char *const env_dev_part = CONFIG_ENV_EXT4_DEVICE_AND_PART;
+
+	if (strlen(env_dev_part) > 0)
+		return env_dev_part;
+
 	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
 	u32 bootmode = get_bootmode();
 
-- 
2.27.0

[PATCH] added check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART definition

[Michael Opdenacker]

Hi Manuel,

Thanks for this fix. Indeed, without it, I don't manage to get U-Boot to
use the EXT4 partition for the environment on stm32mp1 with the below
configuration:

CONFIG_ENV_IS_IN_EXT4=y
CONFIG_ENV_EXT4_INTERFACE="mmc"
CONFIG_ENV_EXT4_DEVICE_AND_PART="0:4"

On 11/22/20 4:19 PM, Manuel Reis wrote:
> Check whether user has explicitly defined device and partition where
> environment file will be located. Voids the need to set such partition
> as bootable to work with the 'dev:auto'
>
> Signed-off-by: Manuel Reis <mluis.reis@gmail.com>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 4553329b25..4dda70740d 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -868,6 +868,11 @@ const char *env_ext4_get_intf(void)
>  
>  const char *env_ext4_get_dev_part(void)
>  {
> +	static char *const env_dev_part = CONFIG_ENV_EXT4_DEVICE_AND_PART;
> +
> +	if (strlen(env_dev_part) > 0)
> +		return env_dev_part;
> +
>  	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
>  	u32 bootmode = get_bootmode();
>  


Tested-by: Michael Opdenacker <michael.opdenacker@bootlin.com>

-- 
Michael Opdenacker, CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH] added check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART definition

[Patrick DELAUNAY]

Hi,

> From: Manuel Reis <mluis.reis@gmail.com>
> Sent: dimanche 22 novembre 2020 16:20
> 
> Check whether user has explicitly defined device and partition where environment
> file will be located. Voids the need to set such partition as bootable to work with
> the 'dev:auto'
> 
> Signed-off-by: Manuel Reis <mluis.reis@gmail.com>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> ---
> 
>  board/st/stm32mp1/stm32mp1.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 4553329b25..4dda70740d 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -868,6 +868,11 @@ const char *env_ext4_get_intf(void)
> 
>  const char *env_ext4_get_dev_part(void)  {
> +	static char *const env_dev_part =
> CONFIG_ENV_EXT4_DEVICE_AND_PART;
> +
> +	if (strlen(env_dev_part) > 0)
> +		return env_dev_part;
> +
>  	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
>  	u32 bootmode = get_bootmode();
> 
> --
> 2.27.0

Behavior seems ok, but patch mixes code and variables.
can you move the code part after the variables definitions ?

  const char *env_ext4_get_dev_part(void)  {
  	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
  	u32 bootmode = get_bootmode();
 +	static char *const env_dev_part =
 CONFIG_ENV_EXT4_DEVICE_AND_PART;
 +
 +	if (strlen(env_dev_part) > 0)
 +		return env_dev_part;
 +

Regards

PAtrick

[PATCH] added check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART definition

[Manuel Reis]

Hi,

thanks for the feedback.

Just sent a new corrected patch with fixed commit message title:
[PATCH] add check for ignored CONFIG_ENV_EXT4_DEVICE_AND_PART 
definition.

regards,
Manuel


On qua, 25 nov, 2020 at 08:45, Patrick DELAUNAY 
<patrick.delaunay@st.com> wrote:
> Hi,
> 
>>  From: Manuel Reis <mluis.reis@gmail.com>
>>  Sent: dimanche 22 novembre 2020 16:20
>> 
>>  Check whether user has explicitly defined device and partition 
>> where environment
>>  file will be located. Voids the need to set such partition as 
>> bootable to work with
>>  the 'dev:auto'
>> 
>>  Signed-off-by: Manuel Reis <mluis.reis@gmail.com>
>>  Cc: Patrick Delaunay <patrick.delaunay@st.com>
>>  Cc: Patrice Chotard <patrice.chotard@st.com>
>>  ---
>> 
>>   board/st/stm32mp1/stm32mp1.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>> 
>>  diff --git a/board/st/stm32mp1/stm32mp1.c 
>> b/board/st/stm32mp1/stm32mp1.c
>>  index 4553329b25..4dda70740d 100644
>>  --- a/board/st/stm32mp1/stm32mp1.c
>>  +++ b/board/st/stm32mp1/stm32mp1.c
>>  @@ -868,6 +868,11 @@ const char *env_ext4_get_intf(void)
>> 
>>   const char *env_ext4_get_dev_part(void)  {
>>  +	static char *const env_dev_part =
>>  CONFIG_ENV_EXT4_DEVICE_AND_PART;
>>  +
>>  +	if (strlen(env_dev_part) > 0)
>>  +		return env_dev_part;
>>  +
>>   	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
>>   	u32 bootmode = get_bootmode();
>> 
>>  --
>>  2.27.0
> 
> Behavior seems ok, but patch mixes code and variables.
> can you move the code part after the variables definitions ?
> 
>   const char *env_ext4_get_dev_part(void)  {
>   	static char *const dev_part[] = {"0:auto", "1:auto", "2:auto"};
>   	u32 bootmode = get_bootmode();
>  +	static char *const env_dev_part =
>  CONFIG_ENV_EXT4_DEVICE_AND_PART;
>  +
>  +	if (strlen(env_dev_part) > 0)
>  +		return env_dev_part;
>  +
> 
> Regards
> 
> PAtrick