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

["Kumar, M Chetan" <m.chetan.kumar@linux.intel.com>]

On 9/13/2021 9:55 PM, Jakub Kicinski wrote:
> On Mon, 13 Sep 2021 18:34:12 +0530 M Chetan Kumar wrote:
>> 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.

>> +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.