Hey Avri,
Thanks for the comments.
>dont
>^^^^
>Don't
My bad, will correct in v2
>
>
>> FFU used to divide the fw_size by native sector size.
>> If native sector size is 4K the accesses need to be aligned and a
>> multiple of 4K, other than that CMD23 SET_BLOCK_COUNT does not change.
>I am not sure what are you fixing, aside of violating a spec requirement:
>" Downloaded firmware bundle must be DATA_SECTOR_SIZE size aligned (internal padding of the bundle might be required)."
>So if the fw fluf is not sector-size aligned, you need to make it so.
So clearly the commit message does not a good job explaining either the problem or the fix, so let me try again.
First of all I only care about 4K native sector size devices here, the rest works fine, the behavior should not be changed by the patch
for 512B sector size devices. (If you think it does, please speak up!)
4k native sector size device FFU is currently broken in mmc-utils.
Why?
We get some firmware of fw_size bytes, we want to tranfer it to the device.
Let's go through the code:
First we have an "alignment" check, which ensures multiple and sets sect_size accordingly.
sect_size = (ext_csd[EXT_CSD_DATA_SECTOR_SIZE] == 0) ? 512 : 4096;
if (fw_size % sect_size) {
fprintf(stderr, "Firmware data size (%jd) is not aligned!\n", (intmax_t)fw_size);
goto out;
}
Then there is the following
/* calculate required fw blocks for CMD25 */
blocks = fw_size / sect_size;
for native 4k device this is now the mount of 4k blocks of fw.
The fw is now transferred in one CMD23/25 transfer:
/* send block count */
multi_cmd->cmds[1].opcode = MMC_SET_BLOCK_COUNT;
multi_cmd->cmds[1].arg = blocks;
multi_cmd->cmds[1].flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
Here we have the problem already, it sets blocks, the fw_size / 4k as block count, this is wrong, should also be fw_size / 512, the patch addresses this.
Now we have the transfer, with an incorrectly set CMD23 as we will see.
/* send image chunk */
multi_cmd->cmds[2].opcode = MMC_WRITE_MULTIPLE_BLOCK;
multi_cmd->cmds[2].blksz = sect_size;
multi_cmd->cmds[2].blocks = blocks;
multi_cmd->cmds[2].arg = arg;
multi_cmd->cmds[2].flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
multi_cmd->cmds[2].write_flag = 1;
mmc_ioc_cmd_set_data(multi_cmd->cmds[2], buf);
blocks*sect_size (data and fw_size size in total) is intended to be transferred, but CMD23 has just set it to fw_size/8 sectors.
Furthermore I think since we are transferring everything in one chunk, blksz and blocks should reflect this, even though the kernel does not really care in that case.
The patch addresses this (this part should not change actual 512B sector size behavior).
The error is just a misunderstanding I would say that CMD23 acts in chunks of native sector size, which is not the case, its argument just needs to be divisible by 8 in case of native 4k.
>So if the fw fluf is not sector-size aligned, you need to make it so.
For now the FFU will cancel for non-aligned fw_size, I did not change this behavior, so I'd say that is fine.
Any future comments appreciated, also I will try to make the commit message more understandable, but if you have suggestions, go ahead.
Regards,
Christian
Hyperstone GmbH | Reichenaustr. 39a | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
