[RFC 0/2] A/B firmware update based in eMMC boot partition.

[RFC 0/2] A/B firmware update based in eMMC boot partition.

[Ying-Chun Liu]

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

Hi all,

I've implemented A/B firmware update based on eMMC hw partitions.
Normally we have 2 boot partitions on eMMC. One is active and another
is inactive. We can then flash the firmware to the inactive one and
make it active.

Also when booting we can write protect the current active one. And
left the inactive partition still writable.

So I make 2 commits that implements the following features:
1. Write protect "active" boot area when board init.
2. flash firmware to inactive boot area.
3. Change the inactive boot area after firmware update.

Thanks.

Ying-Chun Liu (PaulLiu) (2):
  drivers: mmc: write protect active boot area after mmc init.
  drivers: dfu: flash firmware to inactive boot area and active it

 drivers/dfu/dfu_mmc.c | 32 +++++++++++++++++++++---
 drivers/mmc/Kconfig   | 10 ++++++++
 drivers/mmc/mmc.c     | 58 +++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h         |  1 +
 4 files changed, 98 insertions(+), 3 deletions(-)

-- 
2.34.1

[RFC 1/2] drivers: mmc: write protect active boot area after mmc init.

[Ying-Chun Liu]

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

This commit implements write protection for active boot partition for
eMMC. The active boot partition is write protected, and it is still
able to write to the inactive boot partition.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/Kconfig | 10 ++++++++
 drivers/mmc/mmc.c   | 58 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index e0927ce1c9..c4bae743e6 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING
 	  This adds a command and an API to do hardware partitioning on eMMC
 	  devices.
 
+config MMC_WRITE_PROTECT_ACTIVE_BOOT
+	bool "Write protection for active boot partition (eMMC)"
+	depends on MMC_HW_PARTITIONING
+	default n
+	help
+	  Write protection for active boot partition when mmc initialized.
+	  This option protects the active boot partition so that it cannot
+	  be written. But it is still able to write to the inactive boot
+	  partition.
+
 config SUPPORT_EMMC_RPMB
 	bool "Support eMMC replay protected memory block (RPMB)"
 	imply CMD_MMC_RPMB
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 4d9871d69f..8620918749 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc)
 	return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1);
 }
 
+/**
+ * mmc_boot_wp_single_partition() - set write protection to a boot partition.
+ *
+ * This function sets a single boot partition to protect and leave the
+ * other partition writable.
+ *
+ * @param mmc the mmc device.
+ * @param partition 0 - first boot partition, 1 - second boot partition.
+ */
+int mmc_boot_wp_single_partition(struct mmc *mmc, int partition)
+{
+	u8 value;
+	int ret;
+
+	value = 1;
+
+	if (partition == 0) {
+		value |= 0x80;
+		ret = mmc_switch(mmc,
+				 EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_BOOT_WP,
+				 value);
+	} else if (partition == 1) {
+		value |= 0x80;
+		value |= 0x02;
+		ret = mmc_switch(mmc,
+				 EXT_CSD_CMD_SET_NORMAL,
+				 EXT_CSD_BOOT_WP,
+				 value);
+	} else {
+		ret = mmc_boot_wp(mmc);
+	}
+
+	return ret;
+}
+
+/**
+ * mmc_boot_wp_active_partition() - set write protection to active boot
+ * partition.
+ *
+ * @param mmc the mmc device.
+ */
+static int mmc_boot_wp_active_partition(struct mmc *mmc)
+{
+	u8 part;
+
+	if (mmc->part_config == MMCPART_NOAVAILABLE)
+		return -EOPNOTSUPP;
+
+	part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+
+	return mmc_boot_wp_single_partition(mmc, part - 1);
+}
+
 #if !CONFIG_IS_ENABLED(MMC_TINY)
 static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode,
 			      bool hsdowngrade)
@@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc)
 		mmc->has_init = 0;
 	else
 		mmc->has_init = 1;
+
+	if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT))
+		mmc_boot_wp_active_partition(mmc);
+
 	return err;
 }
 
-- 
2.34.1

[RFC 2/2] drivers: dfu: flash firmware to inactive boot area and active it

[Ying-Chun Liu]

From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

This commit implement flash to the inactive boot partition of eMMC
when dfu_alt_info set mmcpart to "inactive". After flash to the inactive
boot partition. It will switch the active partition to the one that
just flashed.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Lukasz Majewski <lukma@denx.de>
---
 drivers/dfu/dfu_mmc.c | 32 +++++++++++++++++++++++++++++---
 include/dfu.h         |  1 +
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
index 3dab5a5f63..3c1f2018d3 100644
--- a/drivers/dfu/dfu_mmc.c
+++ b/drivers/dfu/dfu_mmc.c
@@ -198,6 +198,7 @@ int dfu_write_medium_mmc(struct dfu_entity *dfu,
 		u64 offset, void *buf, long *len)
 {
 	int ret = -1;
+	struct mmc *mmc;
 
 	switch (dfu->layout) {
 	case DFU_RAW_ADDR:
@@ -218,6 +219,17 @@ int dfu_write_medium_mmc(struct dfu_entity *dfu,
 		       dfu_get_layout(dfu->layout));
 	}
 
+	if (!ret && dfu->data.mmc.hw_partition_inactive) {
+		mmc = find_mmc_device(dfu->data.mmc.dev_num);
+		if (!mmc) {
+			pr_err("Device MMC %d - not found!",
+			       dfu->data.mmc.dev_num);
+			return -ENODEV;
+		}
+
+		mmc_set_part_conf(mmc, 0, dfu->data.mmc.hw_partition, 0);
+	}
+
 	return ret;
 }
 
@@ -384,15 +396,29 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *devstr, char *s)
 		dfu->data.mmc.lba_start		= second_arg;
 		dfu->data.mmc.lba_size		= third_arg;
 		dfu->data.mmc.lba_blk_size	= mmc->read_bl_len;
+		dfu->data.mmc.hw_partition_inactive = false;
 
 		/*
 		 * Check for an extra entry at dfu_alt_info env variable
 		 * specifying the mmc HW defined partition number
 		 */
 		if (s)
-			if (!strcmp(strsep(&s, " "), "mmcpart"))
-				dfu->data.mmc.hw_partition =
-					simple_strtoul(s, NULL, 0);
+			if (!strcmp(strsep(&s, " "), "mmcpart")) {
+				if (!strcmp(s, "inactive")) {
+					u8 part_boot;
+					u8 part_target;
+
+					part_boot = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+					part_target = 3 - part_boot;
+					dfu->data.mmc.hw_partition =
+						part_target;
+					dfu->data.mmc.hw_partition_inactive =
+						true;
+				} else {
+					dfu->data.mmc.hw_partition =
+						simple_strtoul(s, NULL, 0);
+				}
+			}
 
 	} else if (!strcmp(entity_type, "part")) {
 		struct disk_partition partinfo;
diff --git a/include/dfu.h b/include/dfu.h
index f6868982df..7fbcf03063 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -53,6 +53,7 @@ struct mmc_internal_data {
 
 	/* eMMC HW partition access */
 	int hw_partition;
+	bool hw_partition_inactive;
 
 	/* FAT/EXT */
 	unsigned int dev;
-- 
2.34.1

Re: [RFC 0/2] A/B firmware update based in eMMC boot partition.

[Wolfgang Denk]

Dear Ying-Chun Liu,

In message <20220125135535.224061-1-grandpaul@gmail.com> you wrote:
>
> I've implemented A/B firmware update based on eMMC hw partitions.
> Normally we have 2 boot partitions on eMMC. One is active and another
> is inactive. We can then flash the firmware to the inactive one and
> make it active.

We have been doing software updates using U-Boot scripts etc. for
many, many years.  But during that time a large number of
restrictions and shortcomings became clear.  this is why projects
like SWUpdate [1] came into life, and U-Boot supports this only with
very basic features:

- watchdog support to reset a hanging system
- boot counter to switch to an alternate boot command in case of too
  many failed boot attempts

That's all. Not more is needed, and not more should be used in
U-Boot.

I'm not sure if you know SWUpdate - if not, you should definitely
look into it.  I strongly recommend not to try to reinvent that
wheel - poorly - in U-Boot.  We have been there before, and got rid
of this approach for a pretty long list of good reasons.


[1] https://sbabic.github.io/swupdate/


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"More software projects have gone awry for lack of calendar time than
for all other causes combined."
                         - Fred Brooks, Jr., _The Mythical Man Month_

Re: [RFC 0/2] A/B firmware update based in eMMC boot partition.

[Alexander Dahl]

Hello,

Am Tue, Jan 25, 2022 at 03:43:29PM +0100 schrieb Wolfgang Denk:
> Dear Ying-Chun Liu,
> 
> In message <20220125135535.224061-1-grandpaul@gmail.com> you wrote:
> >
> > I've implemented A/B firmware update based on eMMC hw partitions.
> > Normally we have 2 boot partitions on eMMC. One is active and another
> > is inactive. We can then flash the firmware to the inactive one and
> > make it active.
> 
> We have been doing software updates using U-Boot scripts etc. for
> many, many years.  But during that time a large number of
> restrictions and shortcomings became clear.  this is why projects
> like SWUpdate [1] came into life, and U-Boot supports this only with
> very basic features:
> 
> - watchdog support to reset a hanging system
> - boot counter to switch to an alternate boot command in case of too
>   many failed boot attempts
> 
> That's all. Not more is needed, and not more should be used in
> U-Boot.
> 
> I'm not sure if you know SWUpdate - if not, you should definitely
> look into it.  I strongly recommend not to try to reinvent that
> wheel - poorly - in U-Boot.  We have been there before, and got rid
> of this approach for a pretty long list of good reasons.
> 
> 
> [1] https://sbabic.github.io/swupdate/

FWIW, if you don't want to reinvent the wheel and evaluate
alternatives, you might also want to checkout RAUC [1].  It has
builtin support for that eMMC boot partition scenario.  The u-boot
integration is not as sophisticated as for other bootloaders, but I
got that running with u-boot last year on raw NAND flash and it's a
lot better than hacking together your own A/B update scheme scripts.

HTH & Greets
Alex

[1] https://www.rauc.io/

Re: [RFC 1/2] drivers: mmc: write protect active boot area after mmc init.

[Jaehoon Chung]

Hi,

On 1/25/22 22:55, Ying-Chun Liu wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> This commit implements write protection for active boot partition for
> eMMC. The active boot partition is write protected, and it is still
> able to write to the inactive boot partition.

It seems that you want to enable Write-protect  about boot partition without additional command, right?
After initialized eMMC, how to update image into boot partition?

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/Kconfig | 10 ++++++++
>  drivers/mmc/mmc.c   | 58 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index e0927ce1c9..c4bae743e6 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING
>  	  This adds a command and an API to do hardware partitioning on eMMC
>  	  devices.
>  
> +config MMC_WRITE_PROTECT_ACTIVE_BOOT
> +	bool "Write protection for active boot partition (eMMC)"
> +	depends on MMC_HW_PARTITIONING
> +	default n
> +	help
> +	  Write protection for active boot partition when mmc initialized.
> +	  This option protects the active boot partition so that it cannot
> +	  be written. But it is still able to write to the inactive boot
> +	  partition.
> +
>  config SUPPORT_EMMC_RPMB
>  	bool "Support eMMC replay protected memory block (RPMB)"
>  	imply CMD_MMC_RPMB
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 4d9871d69f..8620918749 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc)
>  	return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1);
>  }
>  
> +/**
> + * mmc_boot_wp_single_partition() - set write protection to a boot partition.
> + *
> + * This function sets a single boot partition to protect and leave the
> + * other partition writable.
> + *
> + * @param mmc the mmc device.
> + * @param partition 0 - first boot partition, 1 - second boot partition.
> + */
> +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition)
> +{
> +	u8 value;
> +	int ret;
> +
> +	value = 1;
> +
> +	if (partition == 0) {
> +		value |= 0x80;
> +		ret = mmc_switch(mmc,
> +				 EXT_CSD_CMD_SET_NORMAL,
> +				 EXT_CSD_BOOT_WP,
> +				 value);
> +	} else if (partition == 1) {
> +		value |= 0x80;
> +		value |= 0x02;
> +		ret = mmc_switch(mmc,
> +				 EXT_CSD_CMD_SET_NORMAL,
> +				 EXT_CSD_BOOT_WP,
> +				 value);
> +	} else {
> +		ret = mmc_boot_wp(mmc);
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * mmc_boot_wp_active_partition() - set write protection to active boot
> + * partition.
> + *
> + * @param mmc the mmc device.
> + */
> +static int mmc_boot_wp_active_partition(struct mmc *mmc)
> +{
> +	u8 part;
> +
> +	if (mmc->part_config == MMCPART_NOAVAILABLE)
> +		return -EOPNOTSUPP;
> +
> +	part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> +
> +	return mmc_boot_wp_single_partition(mmc, part - 1);
> +}
> +
>  #if !CONFIG_IS_ENABLED(MMC_TINY)
>  static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode,
>  			      bool hsdowngrade)
> @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc)
>  		mmc->has_init = 0;
>  	else
>  		mmc->has_init = 1;
> +
> +	if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT))
> +		mmc_boot_wp_active_partition(mmc);
> +
>  	return err;
>  }
>  

Re: [RFC 1/2] drivers: mmc: write protect active boot area after mmc init.

[Paul Liu]

Hi Jaehoon,

There are 2 boot partitions on eMMC. So the active one is write-protected.
Users can write the new firmware to another boot partition (inactive) which
is not write-protected.
And then switch it on.

In U-boot, execute "mmc wp" write-protect all of the boot partitions.
Maybe we can add an additional command that just write-protect only one
boot partition.

Yours,
Paul


On Thu, 27 Jan 2022 at 07:24, Jaehoon Chung <jh80.chung@samsung.com> wrote:

> Hi,
>
> On 1/25/22 22:55, Ying-Chun Liu wrote:
> > From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> >
> > This commit implements write protection for active boot partition for
> > eMMC. The active boot partition is write protected, and it is still
> > able to write to the inactive boot partition.
>
> It seems that you want to enable Write-protect  about boot partition
> without additional command, right?
> After initialized eMMC, how to update image into boot partition?
>
> Best Regards,
> Jaehoon Chung
>
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Cc: Jaehoon Chung <jh80.chung@samsung.com>
> > ---
> >  drivers/mmc/Kconfig | 10 ++++++++
> >  drivers/mmc/mmc.c   | 58 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 68 insertions(+)
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > index e0927ce1c9..c4bae743e6 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING
> >         This adds a command and an API to do hardware partitioning on
> eMMC
> >         devices.
> >
> > +config MMC_WRITE_PROTECT_ACTIVE_BOOT
> > +     bool "Write protection for active boot partition (eMMC)"
> > +     depends on MMC_HW_PARTITIONING
> > +     default n
> > +     help
> > +       Write protection for active boot partition when mmc initialized.
> > +       This option protects the active boot partition so that it cannot
> > +       be written. But it is still able to write to the inactive boot
> > +       partition.
> > +
> >  config SUPPORT_EMMC_RPMB
> >       bool "Support eMMC replay protected memory block (RPMB)"
> >       imply CMD_MMC_RPMB
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index 4d9871d69f..8620918749 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc)
> >       return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1);
> >  }
> >
> > +/**
> > + * mmc_boot_wp_single_partition() - set write protection to a boot
> partition.
> > + *
> > + * This function sets a single boot partition to protect and leave the
> > + * other partition writable.
> > + *
> > + * @param mmc the mmc device.
> > + * @param partition 0 - first boot partition, 1 - second boot partition.
> > + */
> > +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition)
> > +{
> > +     u8 value;
> > +     int ret;
> > +
> > +     value = 1;
> > +
> > +     if (partition == 0) {
> > +             value |= 0x80;
> > +             ret = mmc_switch(mmc,
> > +                              EXT_CSD_CMD_SET_NORMAL,
> > +                              EXT_CSD_BOOT_WP,
> > +                              value);
> > +     } else if (partition == 1) {
> > +             value |= 0x80;
> > +             value |= 0x02;
> > +             ret = mmc_switch(mmc,
> > +                              EXT_CSD_CMD_SET_NORMAL,
> > +                              EXT_CSD_BOOT_WP,
> > +                              value);
> > +     } else {
> > +             ret = mmc_boot_wp(mmc);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +/**
> > + * mmc_boot_wp_active_partition() - set write protection to active boot
> > + * partition.
> > + *
> > + * @param mmc the mmc device.
> > + */
> > +static int mmc_boot_wp_active_partition(struct mmc *mmc)
> > +{
> > +     u8 part;
> > +
> > +     if (mmc->part_config == MMCPART_NOAVAILABLE)
> > +             return -EOPNOTSUPP;
> > +
> > +     part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> > +
> > +     return mmc_boot_wp_single_partition(mmc, part - 1);
> > +}
> > +
> >  #if !CONFIG_IS_ENABLED(MMC_TINY)
> >  static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode,
> >                             bool hsdowngrade)
> > @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc)
> >               mmc->has_init = 0;
> >       else
> >               mmc->has_init = 1;
> > +
> > +     if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT))
> > +             mmc_boot_wp_active_partition(mmc);
> > +
> >       return err;
> >  }
> >
>
>

Re: [RFC 1/2] drivers: mmc: write protect active boot area after mmc init.

[Jaehoon Chung]

Hi Paul

On 1/27/22 10:23, Paul Liu wrote:
> Hi Jaehoon,
> 
> There are 2 boot partitions on eMMC. So the active one is write-protected.
> Users can write the new firmware to another boot partition (inactive) which
> is not write-protected.
> And then switch it on.

I know there are two boot partitions. Well, it seems that is for A/B switching scheme, right?


> 
> In U-boot, execute "mmc wp" write-protect all of the boot partitions.
> Maybe we can add an additional command that just write-protect only one
> boot partition.
> 
> Yours,
> Paul
> 
> 
> On Thu, 27 Jan 2022 at 07:24, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> 
>> Hi,
>>
>> On 1/25/22 22:55, Ying-Chun Liu wrote:
>>> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
>>>
>>> This commit implements write protection for active boot partition for
>>> eMMC. The active boot partition is write protected, and it is still
>>> able to write to the inactive boot partition.
>>
>> It seems that you want to enable Write-protect  about boot partition
>> without additional command, right?
>> After initialized eMMC, how to update image into boot partition?
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
>>> Cc: Peng Fan <peng.fan@nxp.com>
>>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/mmc/Kconfig | 10 ++++++++
>>>  drivers/mmc/mmc.c   | 58 +++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 68 insertions(+)
>>>
>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>> index e0927ce1c9..c4bae743e6 100644
>>> --- a/drivers/mmc/Kconfig
>>> +++ b/drivers/mmc/Kconfig
>>> @@ -100,6 +100,16 @@ config MMC_HW_PARTITIONING
>>>         This adds a command and an API to do hardware partitioning on
>> eMMC
>>>         devices.
>>>
>>> +config MMC_WRITE_PROTECT_ACTIVE_BOOT
>>> +     bool "Write protection for active boot partition (eMMC)"
>>> +     depends on MMC_HW_PARTITIONING
>>> +     default n
>>> +     help
>>> +       Write protection for active boot partition when mmc initialized.
>>> +       This option protects the active boot partition so that it cannot
>>> +       be written. But it is still able to write to the inactive boot
>>> +       partition.
>>> +
>>>  config SUPPORT_EMMC_RPMB
>>>       bool "Support eMMC replay protected memory block (RPMB)"
>>>       imply CMD_MMC_RPMB
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index 4d9871d69f..8620918749 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -860,6 +860,60 @@ int mmc_boot_wp(struct mmc *mmc)
>>>       return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP, 1);
>>>  }
>>>
>>> +/**
>>> + * mmc_boot_wp_single_partition() - set write protection to a boot
>> partition.
>>> + *
>>> + * This function sets a single boot partition to protect and leave the
>>> + * other partition writable.
>>> + *
>>> + * @param mmc the mmc device.
>>> + * @param partition 0 - first boot partition, 1 - second boot partition.
>>> + */
>>> +int mmc_boot_wp_single_partition(struct mmc *mmc, int partition)
>>> +{
>>> +     u8 value;
>>> +     int ret;
>>> +
>>> +     value = 1;
>>> +
>>> +     if (partition == 0) {
>>> +             value |= 0x80;

Use macro instead of 0x80.

>>> +             ret = mmc_switch(mmc,
>>> +                              EXT_CSD_CMD_SET_NORMAL,
>>> +                              EXT_CSD_BOOT_WP,
>>> +                              value);
>>> +     } else if (partition == 1) {
>>> +             value |= 0x80;
>>> +             value |= 0x02;

Ditto.

>>> +             ret = mmc_switch(mmc,
>>> +                              EXT_CSD_CMD_SET_NORMAL,
>>> +                              EXT_CSD_BOOT_WP,
>>> +                              value);
>>> +     } else {
>>> +             ret = mmc_boot_wp(mmc);
>>> +     }

how aboutusing switch statements?

switch (partition) {
case 0:
	value |= 0x80;
case 1:
	value |= 0x02;
	mmc_switch();
	break;

default:
	mmc_boot_wp();

}




>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +/**
>>> + * mmc_boot_wp_active_partition() - set write protection to active boot
>>> + * partition.
>>> + *
>>> + * @param mmc the mmc device.
>>> + */
>>> +static int mmc_boot_wp_active_partition(struct mmc *mmc)
>>> +{
>>> +     u8 part;
>>> +
>>> +     if (mmc->part_config == MMCPART_NOAVAILABLE)
>>> +             return -EOPNOTSUPP;
>>> +
>>> +     part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
>>> +
>>> +     return mmc_boot_wp_single_partition(mmc, part - 1);
>>> +}
>>> +
>>>  #if !CONFIG_IS_ENABLED(MMC_TINY)
>>>  static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode,
>>>                             bool hsdowngrade)
>>> @@ -2925,6 +2979,10 @@ static int mmc_complete_init(struct mmc *mmc)
>>>               mmc->has_init = 0;
>>>       else
>>>               mmc->has_init = 1;
>>> +
>>> +     if (!err && CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT))

I think that "if (CONFIG_IS_ENABLED(MMC_WRITE_PROTECT_ACTIVE_BOOT) && !err)" is more better.


>>> +             mmc_boot_wp_active_partition(mmc);

Need to handle error.

Best Regards,
Jaehoon Chung


>>> +
>>>       return err;
>>>  }
>>>
>>
>>
> 

Re: [RFC 0/2] A/B firmware update based in eMMC boot partition.

[Tom Rini]

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

On Tue, Jan 25, 2022 at 03:43:29PM +0100, Wolfgang Denk wrote:
> Dear Ying-Chun Liu,
> 
> In message <20220125135535.224061-1-grandpaul@gmail.com> you wrote:
> >
> > I've implemented A/B firmware update based on eMMC hw partitions.
> > Normally we have 2 boot partitions on eMMC. One is active and another
> > is inactive. We can then flash the firmware to the inactive one and
> > make it active.
> 
> We have been doing software updates using U-Boot scripts etc. for
> many, many years.  But during that time a large number of
> restrictions and shortcomings became clear.
[snip]

I just want to chime in here and agree with the overall point that
Wolfgang is making.  There are a number of update schemes that handle
firmware (to whatever level can be safely done) and I see U-Boot's role
here as to support these mechanisms as easily and cleanly as possible
(so, watchdog, bootcount, etc, etc) but to not implement a specific
scheme directly.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]