Re: [PATCH net-next] net: wwan: iosm: firmware flashing and coredump collection

On 9/13/2021 9:55 PM, Jakub Kicinski wrote:

On Mon, 13 Sep 2021 18:34:12 +0530 M Chetan Kumar wrote:

quoted

This patch brings-in support for M.2 7560 Device firmware flashing &
coredump collection using devlink.
- Driver Registers with Devlink framework.
- Register devlink params callback for configuring device params
   required in flashing or coredump flow.
- Implements devlink ops flash_update callback that programs modem
   firmware.
- Creates region & snapshot required for device coredump log collection.

On early detection of device in boot rom stage. Driver registers with
Devlink framework and establish transport channel for PSI (Primary Signed
Image) injection. Once PSI is injected to device, the device execution
stage details are read to determine whether device is in flash or
exception mode.

Is this normal operation flow for the device? After each boot device
boots from the rom image and then user has to "inject" the full FW
image with devlink?

No. It's not a normal operation flow for the device.
M.2 7560 Device has a NAND flash. Only when device needs to be reflashed 
then only fw image is injected via devlink.

In order to keep device in boot rom stage a special AT command or MBIM 
CID is issued prior to fw flash.

quoted

+int ipc_coredump_get_list(struct iosm_devlink *devlink, u16 cmd)
+{
+	u32 byte_read, num_entries, file_size;
+	struct iosm_cd_table *cd_table;
+	u8 size[MAX_SIZE_LEN], i;
+	char *filename;
+	int ret = 0;
+
+	cd_table = kzalloc(MAX_CD_LIST_SIZE, GFP_KERNEL);
+	if (!cd_table) {
+		ret = -ENOMEM;
+		goto  cd_init_fail;
+	}
+
+	ret = ipc_devlink_send_cmd(devlink, cmd, MAX_CD_LIST_SIZE);
+	if (ret) {
+		dev_err(devlink->dev, "rpsi_cmd_coredump_start failed");
+		goto cd_init_fail;
+	}
+
+	ret = ipc_imem_sys_devlink_read(devlink, (u8 *)cd_table,
+					MAX_CD_LIST_SIZE, &byte_read);
+	if (ret) {
+		dev_err(devlink->dev, "Coredump data is invalid");
+		goto cd_init_fail;
+	}
+
+	if (byte_read != MAX_CD_LIST_SIZE)
+		goto cd_init_fail;
+
+	if (cmd == rpsi_cmd_coredump_start) {
+		num_entries = le32_to_cpu(cd_table->list.num_entries);
+		if (num_entries == 0 || num_entries > IOSM_NOF_CD_REGION) {
+			ret = -EINVAL;
+			goto cd_init_fail;
+		}
+
+		for (i = 0; i < num_entries; i++) {
+			file_size = le32_to_cpu(cd_table->list.entry[i].size);
+			filename = cd_table->list.entry[i].filename;
+
+			if (file_size > devlink->cd_file_info[i].default_size) {
+				ret = -EINVAL;
+				goto cd_init_fail;
+			}
+
+			devlink->cd_file_info[i].actual_size = file_size;
+			dev_dbg(devlink->dev, "file: %s actual size %d",
+				filename, file_size);
+			devlink_flash_update_status_notify(devlink->devlink_ctx,
+							   filename,
+							   "FILENAME", 0, 0);

Why are you using flash_update_status_notify() in a snapshot collecting
function? As the name indicates that helper is for reporting flashing
progress.

Coredump collection process starts with pre-flashing procedure (primary 
fw injection), depending on current device state either in exception or 
flash stage the device reported information is notified to devlink via 
_flash_update_status_notify.

This is an early procedure for coredump collection.