[PATCH v1 0/5] PolarFire SoC Auto Update design info support

[PATCH v1 0/5] PolarFire SoC Auto Update design info support

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

There's a document that is internally available that the author of the
design info/overlay format stuff wrote about how it is composed and I
need to go read it and make a version of it publicly available before
this can be merged.

While implementing the design info support, I found a few opportunities
for cleaning up the code and fixed a bug that had been mentioned
internally about failure cases printing success. The scope based cleanup
stuff in particular is rather helpful for the drivers using the system
services mailbox, so I'll roll that out to the other users soonTM.

CC: Conor Dooley <conor.dooley@microchip.com>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Cyril Jean <cyril.jean@microchip.com>
CC: linux-riscv@lists.infradead.org
CC: linux-kernel@vger.kernel.org

Conor Dooley (5):
  firmware: microchip: support writing bitstream info to flash
  firmware: microchip: don't unconditionally print validation success
  firmware: microchip: clarify that sizes and addresses are in hex
  firmware: microchip: move buffer allocation into
    mpfs_auto_update_set_image_address()
  firmware: microchip: use scope-based cleanup where possible

 drivers/firmware/microchip/mpfs-auto-update.c | 140 +++++++++---------
 1 file changed, 70 insertions(+), 70 deletions(-)

-- 
2.43.0

[PATCH v1 1/5] firmware: microchip: support writing bitstream info to flash

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Updating the FPGA image might bring with it changes visible to Linux,
so it is helpful to also co-locate dt-overlays that describe the new
image contents. If these are packaged in a specific format [1] (detected
by first 4 bytes being MCHP, since FPGA images have no magic), load
the file to the reserved 1 MiB region immediately after the directory in
flash. The Beagle-V Fire's "gateware" already creates these files and
puts them in flash [2].

Link: exists on confluence, needs to be made public
Link: https://openbeagle.org/beaglev-fire/gateware/-/blob/main/gateware_scripts/generate_gateware_overlays.py?ref_type=heads [2]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/firmware/microchip/mpfs-auto-update.c | 47 +++++++++++++++----
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index 32394c24b37d..33343e83373c 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -71,8 +71,9 @@
 #define AUTO_UPDATE_UPGRADE_DIRECTORY	(AUTO_UPDATE_DIRECTORY_WIDTH * AUTO_UPDATE_UPGRADE_INDEX)
 #define AUTO_UPDATE_BLANK_DIRECTORY	(AUTO_UPDATE_DIRECTORY_WIDTH * AUTO_UPDATE_BLANK_INDEX)
 #define AUTO_UPDATE_DIRECTORY_SIZE	SZ_1K
-#define AUTO_UPDATE_RESERVED_SIZE	SZ_1M
-#define AUTO_UPDATE_BITSTREAM_BASE	(AUTO_UPDATE_DIRECTORY_SIZE + AUTO_UPDATE_RESERVED_SIZE)
+#define AUTO_UPDATE_INFO_BASE		AUTO_UPDATE_DIRECTORY_SIZE
+#define AUTO_UPDATE_INFO_SIZE		SZ_1M
+#define AUTO_UPDATE_BITSTREAM_BASE	(AUTO_UPDATE_DIRECTORY_SIZE + AUTO_UPDATE_INFO_SIZE)
 
 #define AUTO_UPDATE_TIMEOUT_MS		60000
 
@@ -86,6 +87,17 @@ struct mpfs_auto_update_priv {
 	bool cancel_request;
 };
 
+static bool mpfs_auto_update_is_bitstream_info(const u8 *data, u32 size)
+{
+	if (size < 4)
+		return false;
+
+	if (data[0] == 0x4d && data[1] == 0x43 && data[2] == 0x48 && data[3] == 0x50)
+		return true;
+
+	return false;
+}
+
 static enum fw_upload_err mpfs_auto_update_prepare(struct fw_upload *fw_uploader, const u8 *data,
 						   u32 size)
 {
@@ -287,22 +299,37 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	loff_t directory_address = AUTO_UPDATE_UPGRADE_DIRECTORY;
 	size_t erase_size = AUTO_UPDATE_DIRECTORY_SIZE;
 	size_t bytes_written = 0;
+	bool is_info = mpfs_auto_update_is_bitstream_info(data, size);
 	u32 image_address;
 	int ret;
 
 	erase_size = round_up(erase_size, (u64)priv->flash->erasesize);
 
-	image_address = AUTO_UPDATE_BITSTREAM_BASE +
-		AUTO_UPDATE_UPGRADE_INDEX * priv->size_per_bitstream;
+	if (is_info)
+		image_address = AUTO_UPDATE_INFO_BASE;
+	else
+		image_address = AUTO_UPDATE_BITSTREAM_BASE +
+				AUTO_UPDATE_UPGRADE_INDEX * priv->size_per_bitstream;
 
 	buffer = devm_kzalloc(priv->dev, erase_size, GFP_KERNEL);
 	if (!buffer)
 		return -ENOMEM;
 
-	ret = mpfs_auto_update_set_image_address(priv, buffer, image_address, directory_address);
-	if (ret) {
-		dev_err(priv->dev, "failed to set image address in the SPI directory: %d\n", ret);
-		goto out;
+	/*
+	 * For bitstream info, the descriptor is written to a fixed offset,
+	 * so there is no need to set the image address.
+	 */
+	if (!is_info) {
+		ret = mpfs_auto_update_set_image_address(priv, buffer, image_address, directory_address);
+		if (ret) {
+			dev_err(priv->dev, "failed to set image address in the SPI directory: %d\n", ret);
+			return ret;
+		}
+	} else {
+		if (size > AUTO_UPDATE_INFO_SIZE) {
+			dev_err(priv->dev, "bitstream info exceeds permitted size\n");
+			return -ENOSPC;
+		}
 	}
 
 	/*
@@ -334,6 +361,7 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	}
 
 	*written = bytes_written;
+	dev_info(priv->dev, "Wrote 0x%zx bytes to the flash\n", bytes_written);
 
 out:
 	devm_kfree(priv->dev, buffer);
@@ -360,6 +388,9 @@ static enum fw_upload_err mpfs_auto_update_write(struct fw_upload *fw_uploader,
 		goto out;
 	}
 
+	if (mpfs_auto_update_is_bitstream_info(data, size))
+		goto out;
+
 	ret = mpfs_auto_update_verify_image(fw_uploader);
 	if (ret)
 		err = FW_UPLOAD_ERR_FW_INVALID;
-- 
2.43.0

[PATCH v1 2/5] firmware: microchip: don't unconditionally print validation success

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

If validation fails, both prints are made. Skip the success one in the
failure case.

Fixes: ec5b0f1193ad ("firmware: microchip: add PolarFire SoC Auto Update support")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/firmware/microchip/mpfs-auto-update.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index 33343e83373c..298ad21e139b 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -218,10 +218,12 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
 	if (ret | response->resp_status) {
 		dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
 		ret = ret ? ret : -EBADMSG;
+		goto free_message;
 	}
 
 	dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
 
+free_message:
 	devm_kfree(priv->dev, message);
 free_response:
 	devm_kfree(priv->dev, response);
-- 
2.43.0

Re: [PATCH v1 2/5] firmware: microchip: don't unconditionally print validation success

[Alexandre Ghiti]

Hi Conor,

On 10/04/2024 13:58, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> If validation fails, both prints are made. Skip the success one in the
> failure case.
>
> Fixes: ec5b0f1193ad ("firmware: microchip: add PolarFire SoC Auto Update support")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>   drivers/firmware/microchip/mpfs-auto-update.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
> index 33343e83373c..298ad21e139b 100644
> --- a/drivers/firmware/microchip/mpfs-auto-update.c
> +++ b/drivers/firmware/microchip/mpfs-auto-update.c
> @@ -218,10 +218,12 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
>   	if (ret | response->resp_status) {
>   		dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
>   		ret = ret ? ret : -EBADMSG;
> +		goto free_message;
>   	}
>   
>   	dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
>   
> +free_message:
>   	devm_kfree(priv->dev, message);
>   free_response:
>   	devm_kfree(priv->dev, response);


This should go into -fixes, but not sure if you take care of this or if 
Palmer should, please let me know so that I can remove this from my list :)

Thanks,

Alex

Re: [PATCH v1 2/5] firmware: microchip: don't unconditionally print validation success

[Conor Dooley]

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

On Wed, Apr 24, 2024 at 10:26:35PM +0200, Alexandre Ghiti wrote:
> Hi Conor,
> 
> On 10/04/2024 13:58, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > If validation fails, both prints are made. Skip the success one in the
> > failure case.
> > 
> > Fixes: ec5b0f1193ad ("firmware: microchip: add PolarFire SoC Auto Update support")
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >   drivers/firmware/microchip/mpfs-auto-update.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
> > index 33343e83373c..298ad21e139b 100644
> > --- a/drivers/firmware/microchip/mpfs-auto-update.c
> > +++ b/drivers/firmware/microchip/mpfs-auto-update.c
> > @@ -218,10 +218,12 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
> >   	if (ret | response->resp_status) {
> >   		dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
> >   		ret = ret ? ret : -EBADMSG;
> > +		goto free_message;
> >   	}
> >   	dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
> > +free_message:
> >   	devm_kfree(priv->dev, message);
> >   free_response:
> >   	devm_kfree(priv->dev, response);
> 
> 
> This should go into -fixes, but not sure if you take care of this or if
> Palmer should, please let me know so that I can remove this from my list :)


Yea, probably this and "firmware: microchip: clarify that sizes and
addresses are in hex" should go on fixes. FYI, I usually set the
delegate on patchwork for things that I take to me, so generally you
should be able to tell from that.

Cheers,
Conor.


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

Re: [PATCH v1 2/5] firmware: microchip: don't unconditionally print validation success

[Conor Dooley]

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

On Wed, Apr 24, 2024 at 09:59:36PM +0100, Conor Dooley wrote:
> On Wed, Apr 24, 2024 at 10:26:35PM +0200, Alexandre Ghiti wrote:
> > Hi Conor,
> > 
> > On 10/04/2024 13:58, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > If validation fails, both prints are made. Skip the success one in the
> > > failure case.
> > > 
> > > Fixes: ec5b0f1193ad ("firmware: microchip: add PolarFire SoC Auto Update support")
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > >   drivers/firmware/microchip/mpfs-auto-update.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
> > > index 33343e83373c..298ad21e139b 100644
> > > --- a/drivers/firmware/microchip/mpfs-auto-update.c
> > > +++ b/drivers/firmware/microchip/mpfs-auto-update.c
> > > @@ -218,10 +218,12 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
> > >   	if (ret | response->resp_status) {
> > >   		dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
> > >   		ret = ret ? ret : -EBADMSG;
> > > +		goto free_message;
> > >   	}
> > >   	dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
> > > +free_message:
> > >   	devm_kfree(priv->dev, message);
> > >   free_response:
> > >   	devm_kfree(priv->dev, response);
> > 
> > 
> > This should go into -fixes, but not sure if you take care of this or if
> > Palmer should, please let me know so that I can remove this from my list :)
> 
> 
> Yea, probably this and "firmware: microchip: clarify that sizes and
> addresses are in hex" should go on fixes. FYI, I usually set the
> delegate on patchwork for things that I take to me, so generally you
> should be able to tell from that.

I picked up 2 and 3. I'll send a PR with them later in the week, thanks
for the reminder. Patches like these I kinda dislike applying without a
review, but don't really get reviewed unless I harass someone at work to
do so, which I did not do here.

Cheers,
Conor.

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

[PATCH v1 3/5] firmware: microchip: clarify that sizes and addresses are in hex

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

As it says on the tin. It can be kinda confusing when "22830" is in hex,
so prefix the hex numbers with a "0x".

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/firmware/microchip/mpfs-auto-update.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index 298ad21e139b..078ff328f261 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -279,7 +279,7 @@ static int mpfs_auto_update_set_image_address(struct mpfs_auto_update_priv *priv
 	       AUTO_UPDATE_DIRECTORY_WIDTH);
 	memset(buffer + AUTO_UPDATE_BLANK_DIRECTORY, 0x0, AUTO_UPDATE_DIRECTORY_WIDTH);
 
-	dev_info(priv->dev, "Writing the image address (%x) to the flash directory (%llx)\n",
+	dev_info(priv->dev, "Writing the image address (0x%x) to the flash directory (0x%llx)\n",
 		 image_address, directory_address);
 
 	ret = mtd_write(priv->flash, 0x0, erase_size, &bytes_written, (u_char *)buffer);
@@ -342,7 +342,7 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	erase.len = round_up(size, (size_t)priv->flash->erasesize);
 	erase.addr = image_address;
 
-	dev_info(priv->dev, "Erasing the flash at address (%x)\n", image_address);
+	dev_info(priv->dev, "Erasing the flash at address (0x%x)\n", image_address);
 	ret = mtd_erase(priv->flash, &erase);
 	if (ret)
 		goto out;
@@ -352,7 +352,7 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	 * will do all of that itself - including verifying that the bitstream
 	 * is valid.
 	 */
-	dev_info(priv->dev, "Writing the image to the flash at address (%x)\n", image_address);
+	dev_info(priv->dev, "Writing the image to the flash at address (0x%x)\n", image_address);
 	ret = mtd_write(priv->flash, (loff_t)image_address, size, &bytes_written, data);
 	if (ret)
 		goto out;
-- 
2.43.0

[PATCH v1 4/5] firmware: microchip: move buffer allocation into mpfs_auto_update_set_image_address()

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

This buffer is used exclusively by mpfs_auto_update_set_image_address(),
so move the management of it there, employing the recently added cleanup
infrastructure to avoid littering the function with gotos.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/firmware/microchip/mpfs-auto-update.c | 32 ++++++++-----------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index 078ff328f261..d7ce27f4ba1b 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -9,6 +9,7 @@
  *
  * Author: Conor Dooley <conor.dooley@microchip.com>
  */
+#include <linux/cleanup.h>
 #include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <linux/math.h>
@@ -233,15 +234,17 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
 	return ret;
 }
 
-static int mpfs_auto_update_set_image_address(struct mpfs_auto_update_priv *priv, char *buffer,
+static int mpfs_auto_update_set_image_address(struct mpfs_auto_update_priv *priv,
 					      u32 image_address, loff_t directory_address)
 {
 	struct erase_info erase;
-	size_t erase_size = AUTO_UPDATE_DIRECTORY_SIZE;
+	size_t erase_size = round_up(AUTO_UPDATE_DIRECTORY_SIZE, (u64)priv->flash->erasesize);
 	size_t bytes_written = 0, bytes_read = 0;
+	char *buffer __free(kfree) = kzalloc(erase_size, GFP_KERNEL);
 	int ret;
 
-	erase_size = round_up(erase_size, (u64)priv->flash->erasesize);
+	if (!buffer)
+		return -ENOMEM;
 
 	erase.addr = AUTO_UPDATE_DIRECTORY_BASE;
 	erase.len = erase_size;
@@ -287,7 +290,7 @@ static int mpfs_auto_update_set_image_address(struct mpfs_auto_update_priv *priv
 		return ret;
 
 	if (bytes_written != erase_size)
-		return ret;
+		return -EIO;
 
 	return 0;
 }
@@ -297,7 +300,6 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 {
 	struct mpfs_auto_update_priv *priv = fw_uploader->dd_handle;
 	struct erase_info erase;
-	char *buffer;
 	loff_t directory_address = AUTO_UPDATE_UPGRADE_DIRECTORY;
 	size_t erase_size = AUTO_UPDATE_DIRECTORY_SIZE;
 	size_t bytes_written = 0;
@@ -313,16 +315,12 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 		image_address = AUTO_UPDATE_BITSTREAM_BASE +
 				AUTO_UPDATE_UPGRADE_INDEX * priv->size_per_bitstream;
 
-	buffer = devm_kzalloc(priv->dev, erase_size, GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
 	/*
 	 * For bitstream info, the descriptor is written to a fixed offset,
 	 * so there is no need to set the image address.
 	 */
 	if (!is_info) {
-		ret = mpfs_auto_update_set_image_address(priv, buffer, image_address, directory_address);
+		ret = mpfs_auto_update_set_image_address(priv, image_address, directory_address);
 		if (ret) {
 			dev_err(priv->dev, "failed to set image address in the SPI directory: %d\n", ret);
 			return ret;
@@ -345,7 +343,7 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	dev_info(priv->dev, "Erasing the flash at address (0x%x)\n", image_address);
 	ret = mtd_erase(priv->flash, &erase);
 	if (ret)
-		goto out;
+		return ret;
 
 	/*
 	 * No parsing etc of the bitstream is required. The system controller
@@ -355,19 +353,15 @@ static int mpfs_auto_update_write_bitstream(struct fw_upload *fw_uploader, const
 	dev_info(priv->dev, "Writing the image to the flash at address (0x%x)\n", image_address);
 	ret = mtd_write(priv->flash, (loff_t)image_address, size, &bytes_written, data);
 	if (ret)
-		goto out;
+		return ret;
 
-	if (bytes_written != size) {
-		ret = -EIO;
-		goto out;
-	}
+	if (bytes_written != size)
+		return -EIO;
 
 	*written = bytes_written;
 	dev_info(priv->dev, "Wrote 0x%zx bytes to the flash\n", bytes_written);
 
-out:
-	devm_kfree(priv->dev, buffer);
-	return ret;
+	return 0;
 }
 
 static enum fw_upload_err mpfs_auto_update_write(struct fw_upload *fw_uploader, const u8 *data,
-- 
2.43.0

[PATCH v1 5/5] firmware: microchip: use scope-based cleanup where possible

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

There's a bunch of structs created and freed every time the mailbox is
used. Move them to use the scope-based cleanup infrastructure to avoid
manually tearing them down. mpfs_auto_update_available() didn't free the
memory that it used (albeit it allocated exactly once during probe) so
that gets moved over too.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/firmware/microchip/mpfs-auto-update.c | 59 +++++--------------
 1 file changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c
index d7ce27f4ba1b..30de47895b1c 100644
--- a/drivers/firmware/microchip/mpfs-auto-update.c
+++ b/drivers/firmware/microchip/mpfs-auto-update.c
@@ -175,28 +175,17 @@ static enum fw_upload_err mpfs_auto_update_poll_complete(struct fw_upload *fw_up
 static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
 {
 	struct mpfs_auto_update_priv *priv = fw_uploader->dd_handle;
-	struct mpfs_mss_response *response;
-	struct mpfs_mss_msg *message;
-	u32 *response_msg;
+	u32 *response_msg __free(kfree) =
+		kzalloc(AUTO_UPDATE_FEATURE_RESP_SIZE * sizeof(*response_msg), GFP_KERNEL);
+	struct mpfs_mss_response *response __free(kfree) =
+		kzalloc(sizeof(struct mpfs_mss_response), GFP_KERNEL);
+	struct mpfs_mss_msg *message __free(kfree) =
+		kzalloc(sizeof(struct mpfs_mss_msg), GFP_KERNEL);
 	int ret;
 
-	response_msg = devm_kzalloc(priv->dev, AUTO_UPDATE_FEATURE_RESP_SIZE * sizeof(response_msg),
-				    GFP_KERNEL);
-	if (!response_msg)
+	if (!response_msg || !response || !message)
 		return -ENOMEM;
 
-	response = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_response), GFP_KERNEL);
-	if (!response) {
-		ret = -ENOMEM;
-		goto free_response_msg;
-	}
-
-	message = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_msg), GFP_KERNEL);
-	if (!message) {
-		ret = -ENOMEM;
-		goto free_response;
-	}
-
 	/*
 	 * The system controller can verify that an image in the flash is valid.
 	 * Rather than duplicate the check in this driver, call the relevant
@@ -218,20 +207,12 @@ static int mpfs_auto_update_verify_image(struct fw_upload *fw_uploader)
 	ret = mpfs_blocking_transaction(priv->sys_controller, message);
 	if (ret | response->resp_status) {
 		dev_warn(priv->dev, "Verification of Upgrade Image failed!\n");
-		ret = ret ? ret : -EBADMSG;
-		goto free_message;
+		return ret ? ret : -EBADMSG;
 	}
 
 	dev_info(priv->dev, "Verification of Upgrade Image passed!\n");
 
-free_message:
-	devm_kfree(priv->dev, message);
-free_response:
-	devm_kfree(priv->dev, response);
-free_response_msg:
-	devm_kfree(priv->dev, response_msg);
-
-	return ret;
+	return 0;
 }
 
 static int mpfs_auto_update_set_image_address(struct mpfs_auto_update_priv *priv,
@@ -406,23 +387,15 @@ static const struct fw_upload_ops mpfs_auto_update_ops = {
 
 static int mpfs_auto_update_available(struct mpfs_auto_update_priv *priv)
 {
-	struct mpfs_mss_response *response;
-	struct mpfs_mss_msg *message;
-	u32 *response_msg;
+	u32 *response_msg __free(kfree) =
+		kzalloc(AUTO_UPDATE_FEATURE_RESP_SIZE * sizeof(*response_msg), GFP_KERNEL);
+	struct mpfs_mss_response *response __free(kfree) =
+		kzalloc(sizeof(struct mpfs_mss_response), GFP_KERNEL);
+	struct mpfs_mss_msg *message __free(kfree) =
+		kzalloc(sizeof(struct mpfs_mss_msg), GFP_KERNEL);
 	int ret;
 
-	response_msg = devm_kzalloc(priv->dev,
-				    AUTO_UPDATE_FEATURE_RESP_SIZE * sizeof(*response_msg),
-				    GFP_KERNEL);
-	if (!response_msg)
-		return -ENOMEM;
-
-	response = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_response), GFP_KERNEL);
-	if (!response)
-		return -ENOMEM;
-
-	message = devm_kzalloc(priv->dev, sizeof(struct mpfs_mss_msg), GFP_KERNEL);
-	if (!message)
+	if (!response_msg || !response || !message)
 		return -ENOMEM;
 
 	/*
-- 
2.43.0

Re: (subset) [PATCH v1 0/5] PolarFire SoC Auto Update design info support

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

On Wed, 10 Apr 2024 12:58:03 +0100, Conor Dooley wrote:
> There's a document that is internally available that the author of the
> design info/overlay format stuff wrote about how it is composed and I
> need to go read it and make a version of it publicly available before
> this can be merged.
> 
> While implementing the design info support, I found a few opportunities
> for cleaning up the code and fixed a bug that had been mentioned
> internally about failure cases printing success. The scope based cleanup
> stuff in particular is rather helpful for the drivers using the system
> services mailbox, so I'll roll that out to the other users soonTM.
> 
> [...]

The document that 1/5 relied on has been published, so I've applied
these to riscv-firmware-for-next.

[1/5] firmware: microchip: support writing bitstream info to flash
      https://git.kernel.org/conor/c/a2bf9dfe0090
[4/5] firmware: microchip: move buffer allocation into mpfs_auto_update_set_image_address()
      https://git.kernel.org/conor/c/e277026b5e2d

And 5/5 too, a conflict resolution seems to have upset b4.

Thanks,
Conor.