Date:   Sat,  2 Oct 2021 20:02:12 +0530
From:   M Chetan Kumar <m.chetan.kumar@...ux.intel.com>
To:     netdev@...r.kernel.org
Cc:     kuba@...nel.org, davem@...emloft.net, johannes@...solutions.net,
        ryazanov.s.a@...il.com, loic.poulain@...aro.org,
        krishna.c.sudi@...el.com, m.chetan.kumar@...el.com,
        linuxwwan@...el.com
Subject: [PATCH net-next] net: wwan: iosm: correct devlink extra params

1. Removed driver specific extra params like download_region,
   address & region_count. The required information is passed
   as part of flash API.
2. IOSM Devlink documentation updated to reflect the same.

Signed-off-by: M Chetan Kumar <m.chetan.kumar@...ux.intel.com>
---
 Documentation/networking/devlink/iosm.rst | 32 ++-------
 drivers/net/wwan/iosm/iosm_ipc_devlink.c  | 81 +++++------------------
 drivers/net/wwan/iosm/iosm_ipc_devlink.h  | 44 ++++++++----
 drivers/net/wwan/iosm/iosm_ipc_flash.c    | 42 +++++++-----
 4 files changed, 75 insertions(+), 124 deletions(-)

diff --git a/Documentation/networking/devlink/iosm.rst b/Documentation/networking/devlink/iosm.rst
index dd460c1c15cd..6136181339aa 100644
--- a/Documentation/networking/devlink/iosm.rst
+++ b/Documentation/networking/devlink/iosm.rst
@@ -26,23 +26,6 @@ The ``iosm`` driver implements the following driver-specific parameters.
        the device during firmware flashing.
        If set, Full nand erase command will be sent to the device. By default,
        only conditional erase support is enabled.
-   * - ``download_region``
-     - u8
-     - runtime
-     - download_region parameter is used to identify if we are flashing the
-       loadmap/region file during the firmware flashing.
-   * - ``address``
-     - u32
-     - runtime
-     - address parameter is used to send the address information of the
-       loadmap/region file which is required during the firmware flashing
-       process. Each region file has be flashed to its respective flash address.
-   * - ``region_count``
-     - u8
-     - runtime
-     - region_count parameter is used to inform the driver on how many total
-       loadmap/region files are present in modem firmware image that has to be
-       flashed.
 
 
 Flash Update
@@ -87,7 +70,7 @@ Flash Commands:
 1) When modem is in Boot ROM stage, user can use below command to inject PSI RAM
 image using devlink flash command.
 
-$ devlink dev flash pci/0000:02:00.0 file <PSI_RAM_File_name> component PSI
+$ devlink dev flash pci/0000:02:00.0 file <PSI_RAM_File_name>
 
 2) If user want to do a full erase, below command need to be issued to set the
 erase full flash param (To be set only if full erase required).
@@ -95,22 +78,19 @@ erase full flash param (To be set only if full erase required).
 $ devlink dev param set pci/0000:02:00.0 name erase_full_flash value true cmode runtime
 
 3) Inject EBL after the modem is in PSI stage.
-$ devlink dev flash pci/0000:02:00.0 file <EBL_File_name> component EBL
+
+$ devlink dev flash pci/0000:02:00.0 file <EBL_File_name>
 
 4) Once EBL is injected successfully, then the actual firmware flashing takes
 place. Below is the sequence of commands used for each of the firmware images.
 
 a) Flash secure bin file.
-$ devlink dev flash pci/0000:02:00.0 file <Secure_bin_file_name> component FLS
-
-b) Flashing the Loadmap/Region file
-$ devlink dev param set pci/0000:02:00.0 name region_count value 1 cmode runtime
 
-$ devlink dev param set pci/0000:02:00.0 name download_region value true cmode runtime
+$ devlink dev flash pci/0000:02:00.0 file <Secure_bin_file_name>
 
-$ devlink dev param set pci/0000:02:00.0 name address value <Nand_address> cmode runtime
+b) Flashing the Loadmap/Region file
 
-$ devlink dev flash pci/0000:02:00.0 file <Load_map_file_name> component FLS
+$ devlink dev flash pci/0000:02:00.0 file <Load_map_file_name>
 
 Regions
 =======
diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.c b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
index 6fe56f73011b..17da85a8f337 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
@@ -23,31 +23,11 @@ static int ipc_devlink_get_param(struct devlink *dl, u32 id,
 				 struct devlink_param_gset_ctx *ctx)
 {
 	struct iosm_devlink *ipc_devlink = devlink_priv(dl);
-	int rc = 0;
 
-	switch (id) {
-	case IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH:
+	if (id == IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH)
 		ctx->val.vu8 = ipc_devlink->param.erase_full_flash;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_DOWNLOAD_REGION:
-		ctx->val.vu8 = ipc_devlink->param.download_region;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_ADDRESS:
-		ctx->val.vu32 = ipc_devlink->param.address;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_REGION_COUNT:
-		ctx->val.vu8 = ipc_devlink->param.region_count;
-		break;
 
-	default:
-		rc = -EOPNOTSUPP;
-		break;
-	}
-
-	return rc;
+	return 0;
 }
 
 /* Set the param values for the specific param ID's */
@@ -55,31 +35,11 @@ static int ipc_devlink_set_param(struct devlink *dl, u32 id,
 				 struct devlink_param_gset_ctx *ctx)
 {
 	struct iosm_devlink *ipc_devlink = devlink_priv(dl);
-	int rc = 0;
 
-	switch (id) {
-	case IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH:
+	if (id == IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH)
 		ipc_devlink->param.erase_full_flash = ctx->val.vu8;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_DOWNLOAD_REGION:
-		ipc_devlink->param.download_region = ctx->val.vu8;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_ADDRESS:
-		ipc_devlink->param.address = ctx->val.vu32;
-		break;
-
-	case IOSM_DEVLINK_PARAM_ID_REGION_COUNT:
-		ipc_devlink->param.region_count = ctx->val.vu8;
-		break;
-
-	default:
-		rc = -EOPNOTSUPP;
-		break;
-	}
 
-	return rc;
+	return 0;
 }
 
 /* Devlink param structure array */
@@ -89,21 +49,6 @@ static const struct devlink_param iosm_devlink_params[] = {
 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
 			     ipc_devlink_get_param, ipc_devlink_set_param,
 			     NULL),
-	DEVLINK_PARAM_DRIVER(IOSM_DEVLINK_PARAM_ID_DOWNLOAD_REGION,
-			     "download_region", DEVLINK_PARAM_TYPE_BOOL,
-			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
-			     ipc_devlink_get_param, ipc_devlink_set_param,
-			     NULL),
-	DEVLINK_PARAM_DRIVER(IOSM_DEVLINK_PARAM_ID_ADDRESS,
-			     "address", DEVLINK_PARAM_TYPE_U32,
-			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
-			     ipc_devlink_get_param, ipc_devlink_set_param,
-			     NULL),
-	DEVLINK_PARAM_DRIVER(IOSM_DEVLINK_PARAM_ID_REGION_COUNT,
-			     "region_count", DEVLINK_PARAM_TYPE_U8,
-			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
-			     ipc_devlink_get_param, ipc_devlink_set_param,
-			     NULL),
 };
 
 /* Get devlink flash component type */
@@ -134,18 +79,23 @@ static int ipc_devlink_flash_update(struct devlink *devlink,
 {
 	struct iosm_devlink *ipc_devlink = devlink_priv(devlink);
 	enum iosm_flash_comp_type fls_type;
+	struct iosm_devlink_image *header;
 	int rc = -EINVAL;
 	u8 *mdm_rsp;
 
-	if (!params->component)
+	header = (struct iosm_devlink_image *)params->fw->data;
+
+	if (!header || params->fw->size <= IOSM_DEVLINK_HDR_SIZE ||
+	    (memcmp(header->magic_header, IOSM_DEVLINK_MAGIC_HEADER,
+	     IOSM_DEVLINK_MAGIC_HEADER_LEN) != 0))
 		return -EINVAL;
 
 	mdm_rsp = kzalloc(IOSM_EBL_DW_PACK_SIZE, GFP_KERNEL);
 	if (!mdm_rsp)
 		return -ENOMEM;
 
-	fls_type = ipc_devlink_get_flash_comp_type(params->component,
-						   strlen(params->component));
+	fls_type = ipc_devlink_get_flash_comp_type(header->image_type,
+						   IOSM_DEVLINK_MAX_IMG_LEN);
 
 	switch (fls_type) {
 	case FLASH_COMP_TYPE_PSI:
@@ -165,16 +115,16 @@ static int ipc_devlink_flash_update(struct devlink *devlink,
 		break;
 	default:
 		devlink_flash_update_status_notify(devlink, "Invalid component",
-						   params->component, 0, 0);
+						   NULL, 0, 0);
 		break;
 	}
 
 	if (!rc)
 		devlink_flash_update_status_notify(devlink, "Flashing success",
-						   params->component, 0, 0);
+						   header->image_type, 0, 0);
 	else
 		devlink_flash_update_status_notify(devlink, "Flashing failed",
-						   params->component, 0, 0);
+						   header->image_type, 0, 0);
 
 	kfree(mdm_rsp);
 	return rc;
@@ -182,7 +132,6 @@ static int ipc_devlink_flash_update(struct devlink *devlink,
 
 /* Call back function for devlink ops */
 static const struct devlink_ops devlink_flash_ops = {
-	.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT,
 	.flash_update = ipc_devlink_flash_update,
 };
 
diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.h b/drivers/net/wwan/iosm/iosm_ipc_devlink.h
index fa2b388a2f8a..35c2d013b9cc 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.h
@@ -12,6 +12,18 @@
 #include "iosm_ipc_imem_ops.h"
 #include "iosm_ipc_pcie.h"
 
+/* Image ext max len */
+#define IOSM_DEVLINK_MAX_IMG_LEN 3
+/* Magic Header */
+#define IOSM_DEVLINK_MAGIC_HEADER "IOSM_DEVLINK_HEADER"
+/* Magic Header len */
+#define IOSM_DEVLINK_MAGIC_HEADER_LEN 20
+/* Devlink image type */
+#define IOSM_DEVLINK_IMG_TYPE 4
+/* Reserve header size */
+#define IOSM_DEVLINK_RESERVED 34
+/* Devlink Image Header size */
+#define IOSM_DEVLINK_HDR_SIZE sizeof(struct iosm_devlink_image)
 /* MAX file name length */
 #define IOSM_MAX_FILENAME_LEN 32
 /* EBL response size */
@@ -32,19 +44,11 @@
  * enum iosm_devlink_param_id - Enum type to different devlink params
  * @IOSM_DEVLINK_PARAM_ID_BASE:			Devlink param base ID
  * @IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH:     Set if full erase required
- * @IOSM_DEVLINK_PARAM_ID_DOWNLOAD_REGION:	Set if fls file to be
- *						flashed is Loadmap/region file
- * @IOSM_DEVLINK_PARAM_ID_ADDRESS:		Address of the region to be
- *						flashed
- * @IOSM_DEVLINK_PARAM_ID_REGION_COUNT:		Max region count
  */
 
 enum iosm_devlink_param_id {
 	IOSM_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
 	IOSM_DEVLINK_PARAM_ID_ERASE_FULL_FLASH,
-	IOSM_DEVLINK_PARAM_ID_DOWNLOAD_REGION,
-	IOSM_DEVLINK_PARAM_ID_ADDRESS,
-	IOSM_DEVLINK_PARAM_ID_REGION_COUNT,
 };
 
 /**
@@ -94,22 +98,34 @@ struct iosm_devlink_sio {
 
 /**
  * struct iosm_flash_params - List of flash params required for flashing
- * @address:		Address of the region file to be flashed
- * @region_count:	Maximum no of regions for each fls file
- * @download_region:	To be set if region is being flashed
  * @erase_full_flash:   To set the flashing mode
  *                      erase_full_flash = 1; full erase
  *                      erase_full_flash = 0; no erase
  * @erase_full_flash_done: Flag to check if it is a full erase
  */
 struct iosm_flash_params {
-	u32 address;
-	u8 region_count;
-	u8 download_region;
 	u8 erase_full_flash;
 	u8 erase_full_flash_done;
 };
 
+/**
+ * struct iosm_devlink_image - Structure with Fls file header info
+ * @magic_header:	Header of the firmware image
+ * @image_type:		Firmware image type
+ * @region_address:	Address of the region to be flashed
+ * @download_region:	Field to identify if it is a region
+ * @last_region:	Field to identify if it is last region
+ * @reserved:		Reserved field
+ */
+struct iosm_devlink_image {
+	char magic_header[IOSM_DEVLINK_MAGIC_HEADER_LEN];
+	char image_type[IOSM_DEVLINK_IMG_TYPE];
+	__le32 region_address;
+	u8 download_region;
+	u8 last_region;
+	u8 reserved[IOSM_DEVLINK_RESERVED];
+} __packed;
+
 /**
  * struct iosm_ebl_ctx_data -  EBL ctx data used during flashing
  * @ebl_sw_info_version: SWID version info obtained from EBL
diff --git a/drivers/net/wwan/iosm/iosm_ipc_flash.c b/drivers/net/wwan/iosm/iosm_ipc_flash.c
index ebceedf7c9f5..d890914aa349 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_flash.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_flash.c
@@ -330,18 +330,20 @@ static int ipc_flash_full_erase(struct iosm_devlink *ipc_devlink, u8 *mdm_rsp)
 static int ipc_flash_download_region(struct iosm_devlink *ipc_devlink,
 				     const struct firmware *fw, u8 *mdm_rsp)
 {
+	u32 raw_len, rest_len = fw->size - IOSM_DEVLINK_HDR_SIZE;
+	struct iosm_devlink_image *fls_data;
 	__le32 reg_info[2]; /* 0th position region address, 1st position size */
+	u32 nand_address;
 	char *file_ptr;
-	u32 rest_len;
-	u32 raw_len;
 	int ret;
 
-	file_ptr = (char *)fw->data;
-	reg_info[0] = cpu_to_le32(ipc_devlink->param.address);
+	fls_data = (struct iosm_devlink_image *)fw->data;
+	file_ptr = (void *)(fls_data + 1);
+	nand_address = le32_to_cpu(fls_data->region_address);
+	reg_info[0] = cpu_to_le32(nand_address);
 
 	if (!ipc_devlink->param.erase_full_flash_done) {
-		reg_info[1] = cpu_to_le32(ipc_devlink->param.address +
-					  fw->size - 2);
+		reg_info[1] = cpu_to_le32(nand_address + rest_len - 2);
 		ret = ipc_flash_send_receive(ipc_devlink, FLASH_ERASE_START,
 					     (u8 *)reg_info, IOSM_MDM_SEND_8,
 					     mdm_rsp);
@@ -359,8 +361,6 @@ static int ipc_flash_download_region(struct iosm_devlink *ipc_devlink,
 	if (ret)
 		goto dl_region_fail;
 
-	rest_len = fw->size;
-
 	/* Request Flash Write Raw Image */
 	ret = ipc_flash_send_data(ipc_devlink, IOSM_EBL_DW_PACK_SIZE,
 				  FLASH_WRITE_IMAGE_RAW, (u8 *)&rest_len,
@@ -399,9 +399,12 @@ static int ipc_flash_download_region(struct iosm_devlink *ipc_devlink,
 int ipc_flash_send_fls(struct iosm_devlink *ipc_devlink,
 		       const struct firmware *fw, u8 *mdm_rsp)
 {
+	u32 fw_size = fw->size - IOSM_DEVLINK_HDR_SIZE;
+	struct iosm_devlink_image *fls_data;
 	u16 flash_cmd;
 	int ret;
 
+	fls_data = (struct iosm_devlink_image *)fw->data;
 	if (ipc_devlink->param.erase_full_flash) {
 		ipc_devlink->param.erase_full_flash = false;
 		ret = ipc_flash_full_erase(ipc_devlink, mdm_rsp);
@@ -410,19 +413,20 @@ int ipc_flash_send_fls(struct iosm_devlink *ipc_devlink,
 	}
 
 	/* Request Sec Start */
-	if (!ipc_devlink->param.download_region) {
+	if (!fls_data->download_region) {
 		ret = ipc_flash_send_receive(ipc_devlink, FLASH_SEC_START,
-					     (u8 *)fw->data, fw->size, mdm_rsp);
+					     (u8 *)fw->data +
+					     IOSM_DEVLINK_HDR_SIZE, fw_size,
+					     mdm_rsp);
 		if (ret)
 			goto ipc_flash_err;
 	} else {
 		/* Download regions */
-		ipc_devlink->param.region_count -= IOSM_SET_FLAG;
 		ret = ipc_flash_download_region(ipc_devlink, fw, mdm_rsp);
 		if (ret)
 			goto ipc_flash_err;
 
-		if (!ipc_devlink->param.region_count) {
+		if (fls_data->last_region) {
 			/* Request Sec End */
 			flash_cmd = IOSM_MDM_SEND_DATA;
 			ret = ipc_flash_send_receive(ipc_devlink, FLASH_SEC_END,
@@ -445,17 +449,18 @@ int ipc_flash_send_fls(struct iosm_devlink *ipc_devlink,
 int ipc_flash_boot_psi(struct iosm_devlink *ipc_devlink,
 		       const struct firmware *fw)
 {
+	u32 bytes_read, psi_size = fw->size - IOSM_DEVLINK_HDR_SIZE;
 	u8 psi_ack_byte[IOSM_PSI_ACK], read_data[2];
-	u32 bytes_read;
 	u8 *psi_code;
 	int ret;
 
 	dev_dbg(ipc_devlink->dev, "Boot transfer PSI");
-	psi_code = kmemdup(fw->data, fw->size, GFP_KERNEL);
+	psi_code = kmemdup(fw->data + IOSM_DEVLINK_HDR_SIZE, psi_size,
+			   GFP_KERNEL);
 	if (!psi_code)
 		return -ENOMEM;
 
-	ret = ipc_imem_sys_devlink_write(ipc_devlink, psi_code, fw->size);
+	ret = ipc_imem_sys_devlink_write(ipc_devlink, psi_code, psi_size);
 	if (ret) {
 		dev_err(ipc_devlink->dev, "RPSI Image write failed");
 		goto ipc_flash_psi_free;
@@ -501,7 +506,7 @@ int ipc_flash_boot_psi(struct iosm_devlink *ipc_devlink,
 int ipc_flash_boot_ebl(struct iosm_devlink *ipc_devlink,
 		       const struct firmware *fw)
 {
-	u32 ebl_size = fw->size;
+	u32 ebl_size = fw->size - IOSM_DEVLINK_HDR_SIZE;
 	u8 read_data[2];
 	u32 bytes_read;
 	int ret;
@@ -553,8 +558,9 @@ int ipc_flash_boot_ebl(struct iosm_devlink *ipc_devlink,
 		goto ipc_flash_ebl_err;
 	}
 
-	ret = ipc_imem_sys_devlink_write(ipc_devlink, (unsigned char *)fw->data,
-					 fw->size);
+	ret = ipc_imem_sys_devlink_write(ipc_devlink,
+					 (u8 *)fw->data + IOSM_DEVLINK_HDR_SIZE,
+					 ebl_size);
 	if (ret) {
 		dev_err(ipc_devlink->dev, "EBL data transfer failed");
 		goto ipc_flash_ebl_err;
-- 
2.25.1

Powered by blists - more mailing lists