[PATCH v2] mmc: Update environment variable with active partition

[PATCH v2] mmc: Update environment variable with active partition

[Reuben Dowle]

This patch allows uboot scripts make choices about where to boot from based
on the active mmc boot partition. This allows having two copies of kernel,
filesystems etc, and choosing which to boot from based off of the active
bootloader partition.

Signed-off-by: Reuben Dowle <reuben.dowle@4rf.com>
---
 cmd/mmc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index a10f137..afa8e7b 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -808,7 +808,7 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
 	return CMD_RET_SUCCESS;
 }
 
-static int mmc_partconf_print(struct mmc *mmc)
+static int mmc_partconf_print(struct mmc *mmc, const char *varname)
 {
 	u8 ack, access, part;
 
@@ -821,6 +821,9 @@ static int mmc_partconf_print(struct mmc *mmc)
 	ack = EXT_CSD_EXTRACT_BOOT_ACK(mmc->part_config);
 	part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
 
+	if(varname)
+		env_set_hex(varname, part);
+
 	printf("EXT_CSD[179], PARTITION_CONFIG:\n"
 		"BOOT_ACK: 0x%x\n"
 		"BOOT_PARTITION_ENABLE: 0x%x\n"
@@ -836,7 +839,7 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag,
 	struct mmc *mmc;
 	u8 ack, part_num, access;
 
-	if (argc != 2 && argc != 5)
+	if (argc != 2 && argc != 3 && argc != 5)
 		return CMD_RET_USAGE;
 
 	dev = simple_strtoul(argv[1], NULL, 10);
@@ -850,8 +853,8 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag,
 		return CMD_RET_FAILURE;
 	}
 
-	if (argc == 2)
-		return mmc_partconf_print(mmc);
+	if (argc == 2 || argc == 3)
+		return mmc_partconf_print(mmc, argc == 3 ? argv[2] : NULL);
 
 	ack = simple_strtoul(argv[2], NULL, 10);
 	part_num = simple_strtoul(argv[3], NULL, 10);
@@ -1061,8 +1064,11 @@ U_BOOT_CMD(
 	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
 	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
 	" - Change sizes of boot and RPMB partitions of specified device\n"
-	"mmc partconf <dev> [boot_ack boot_partition partition_access]\n"
-	" - Show or change the bits of the PARTITION_CONFIG field of the specified device\n"
+	"mmc partconf <dev> [varname]\n"
+	" - Show the bits of the PARTITION_CONFIG field of the specified device,\n"
+	"   optionally storing the boot_partion value into <varname>"
+	"mmc partconf <dev> <boot_ack> <boot_partition> <partition_access>\n"
+	" - Change the bits of the PARTITION_CONFIG field of the specified device\n"
 	"mmc rst-function <dev> <value>\n"
 	" - Change the RST_n_FUNCTION field of the specified device\n"
 	"   WARNING: This is a write-once field and 0 / 1 / 2 are the only valid values.\n"
-- 
2.7.4

[PATCH v2] mmc: Update environment variable with active partition

[Jaehoon Chung]

Hi Reuben,

On 5/10/21 9:14 AM, Reuben Dowle wrote:
> This patch allows uboot scripts make choices about where to boot from based
> on the active mmc boot partition. This allows having two copies of kernel,
> filesystems etc, and choosing which to boot from based off of the active
> bootloader partition.
> 
> Signed-off-by: Reuben Dowle <reuben.dowle@4rf.com>
> ---
>  cmd/mmc.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index a10f137..afa8e7b 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -808,7 +808,7 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>  
> -static int mmc_partconf_print(struct mmc *mmc)
> +static int mmc_partconf_print(struct mmc *mmc, const char *varname)
>  {
>  	u8 ack, access, part;
>  
> @@ -821,6 +821,9 @@ static int mmc_partconf_print(struct mmc *mmc)
>  	ack = EXT_CSD_EXTRACT_BOOT_ACK(mmc->part_config);
>  	part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
>  
> +	if(varname)
> +		env_set_hex(varname, part);
> +
>  	printf("EXT_CSD[179], PARTITION_CONFIG:\n"
>  		"BOOT_ACK: 0x%x\n"
>  		"BOOT_PARTITION_ENABLE: 0x%x\n"
> @@ -836,7 +839,7 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag,
>  	struct mmc *mmc;
>  	u8 ack, part_num, access;
>  
> -	if (argc != 2 && argc != 5)
> +	if (argc != 2 && argc != 3 && argc != 5)
>  		return CMD_RET_USAGE;
>  
>  	dev = simple_strtoul(argv[1], NULL, 10);
> @@ -850,8 +853,8 @@ static int do_mmc_partconf(struct cmd_tbl *cmdtp, int flag,
>  		return CMD_RET_FAILURE;
>  	}
>  
> -	if (argc == 2)
> -		return mmc_partconf_print(mmc);
> +	if (argc == 2 || argc == 3)
> +		return mmc_partconf_print(mmc, argc == 3 ? argv[2] : NULL);
>  
>  	ack = simple_strtoul(argv[2], NULL, 10);
>  	part_num = simple_strtoul(argv[3], NULL, 10);
> @@ -1061,8 +1064,11 @@ U_BOOT_CMD(
>  	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
>  	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
>  	" - Change sizes of boot and RPMB partitions of specified device\n"
> -	"mmc partconf <dev> [boot_ack boot_partition partition_access]\n"
> -	" - Show or change the bits of the PARTITION_CONFIG field of the specified device\n"
> +	"mmc partconf <dev> [varname]\n"
> +	" - Show the bits of the PARTITION_CONFIG field of the specified device,\n"
> +	"   optionally storing the boot_partion value into <varname>"
> +	"mmc partconf <dev> <boot_ack> <boot_partition> <partition_access>\n"

boot_ack / boot_partition / partition_access are optional variable, not mandatory.
And it's confused that different variable is used with same command.

I think that it's enough to use "mmc partconf <dev>". Then it's showed which partition is used.

If you have to apply this patch,

It seems that "mmc partconf <dev> [varname] [boot_ack boot_partition partition_access]" is correct way.

Best Regards,
Jaehoon Chung


> +	" - Change the bits of the PARTITION_CONFIG field of the specified device\n"
>  	"mmc rst-function <dev> <value>\n"
>  	" - Change the RST_n_FUNCTION field of the specified device\n"
>  	"   WARNING: This is a write-once field and 0 / 1 / 2 are the only valid values.\n"
> 

[PATCH v2] mmc: Update environment variable with active partition

[Reuben Dowle]

Hi Jaehoon,

> 
> boot_ack / boot_partition / partition_access are optional variable, not
> mandatory.
> And it's confused that different variable is used with same command.
> 
> I think that it's enough to use "mmc partconf <dev>". Then it's showed which
> partition is used.
> 
> If you have to apply this patch,
> 
> It seems that "mmc partconf <dev> [varname] [boot_ack boot_partition
> partition_access]" is correct way.

mmc partconf has two forms currently - one  where you set the values and one where you read the values. It would not make sense to write the environment variable when you are writing the MMC values.

When setting the values you need 4 parameters (dev + three values to set). In this case all three values are mandatory, and this is not changing.

When reading the values you currently just need 1 parameter (dev). This patch adds a second optional parameter to store the current active partition.

> 
> Best Regards,
> Jaehoon Chung
> 

[PATCH v2] mmc: Update environment variable with active partition

[Jaehoon Chung]

Hi Reuben,

On 5/10/21 1:38 PM, Reuben Dowle wrote:
> Hi Jaehoon,
> 
>>
>> boot_ack / boot_partition / partition_access are optional variable, not
>> mandatory.
>> And it's confused that different variable is used with same command.
>>
>> I think that it's enough to use "mmc partconf <dev>". Then it's showed which
>> partition is used.
>>
>> If you have to apply this patch,
>>
>> It seems that "mmc partconf <dev> [varname] [boot_ack boot_partition
>> partition_access]" is correct way.
> 
> mmc partconf has two forms currently - one  where you set the values and one where you read the values. It would not make sense to write the environment variable when you are writing the MMC values.
> 
> When setting the values you need 4 parameters (dev + three values to set). In this case all three values are mandatory, and this is not changing.

<dev> is mandatory, and [<boot_ack> <boot_partition> <partition_access>] is optional.
If optional variable will be used, those variables can be mandatory, right?

mmc partconf <dev> is displayed even without other variables.

"mmc partconf <dev> [[varname] | [<boot_ack> <boot_partition> <partition_access>]]" is correct usage for yours.

It means varname or other options variable can be chosen.

Best Regards,
Jaehoon Chung


> 
> When reading the values you currently just need 1 parameter (dev). This patch adds a second optional parameter to store the current active partition.
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
> ________________________________
> 
> [cid:4RFLogo(Custom)(2)_0f31a7de-6dd6-43cf-bc6a-a097a2b80b69.jpg]
> Reuben Dowle
> Software Architect
> 
> Phone:
> 
> Fax:
> E-Mail:
> Website:
> 
> 
> +64 4 499 6000
> 
> +64 4 473 4447
> reuben.dowle at 4rf.com<mailto:reuben.dowle@4rf.com>
> https://protect2.fireeye.com/v1/url?k=db8e7728-84154e07-db8ffc67-0cc47a31cdf8-7844d268e34be46e&q=1&e=63fae9b4-9143-4f55-bc4c-1de4d743c505&u=https%3A%2F%2Fwww.4rf.com%2F<https://protect2.fireeye.com/v1/url?k=c6895738-99126e17-c688dc77-0cc47a31cdf8-79a58163574daf40&q=1&e=63fae9b4-9143-4f55-bc4c-1de4d743c505&u=https%3A%2F%2Fwww.4rf.com%2F>
> 
> 
> ________________________________
> 
> [cid:Family_53c410b1-7227-4a5f-9acb-f09bd7617a39.png] <https://protect2.fireeye.com/v1/url?k=96150f08-c98e3627-96148447-0cc47a31cdf8-4901f25b032dcaaf&q=1&e=63fae9b4-9143-4f55-bc4c-1de4d743c505&u=http%3A%2F%2Fwww.4rf.com%2Fnews%2Fevents>
> 
> ________________________________
> The information in this email communication (inclusive of attachments) is confidential to 4RF Limited and the intended recipient(s). If you are not the intended recipient(s), please note that any use, disclosure, distribution or copying of this information or any part thereof is strictly prohibited and that the author accepts no liability for the consequences of any action taken on the basis of the information provided. If you have received this email in error, please notify the sender immediately by return email and then delete all instances of this email from your system. 4RF Limited will not accept responsibility for any consequences associated with the use of this email (including, but not limited to, damages sustained as a result of any viruses and/or any action or lack of action taken in reliance on it).
> 

[PATCH v2] mmc: Update environment variable with active partition

[Reuben Dowle]

Hi Jaehoon,

> <dev> is mandatory, and [<boot_ack> <boot_partition> <partition_access>] is optional.
> If optional variable will be used, those variables can be mandatory, right?
> 
> mmc partconf <dev> is displayed even without other variables.
> 
> "mmc partconf <dev> [[varname] | [<boot_ack> <boot_partition> <partition_access>]]" is correct usage for yours.
> 
> It means varname or other options variable can be chosen.

I agree, this seems like a better way to format the help. I have sent a new patch with it changed to your suggestion.

Regards,
Reuben