Re: [RFC PATCH] mmc: block: Add new ioctl to send combo commands

From: Jon Hunter
Date: Thu Sep 03 2015 - 11:08:27 EST


Hi Olof,

On 02/09/15 19:28, Olof Johansson wrote:
> Hi,
>
> On Wed, Sep 2, 2015 at 7:21 AM, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>> From: Seshagiri Holi <sholi@xxxxxxxxxx>
>>
>> Certain eMMC devices allow vendor specific device information to be read
>> via a sequence of vendor commands. These vendor commands must be issued
>> in sequence and an atomic fashion. One way to support this would be to
>> add an ioctl function for sending a sequence of commands to the device
>> atomically as proposed here. These combo commands are simple array of
>> the existing mmc_ioc_cmd structure.
>
> This seems reasonable to me, being able to do a sequence of
> back-to-back operations without system read/writes slipping through.
>
>> diff --git a/include/uapi/linux/mmc/ioctl.h b/include/uapi/linux/mmc/ioctl.h
>> index 1f5e68923929..5943e51f22b3 100644
>> --- a/include/uapi/linux/mmc/ioctl.h
>> +++ b/include/uapi/linux/mmc/ioctl.h
>> @@ -45,8 +45,23 @@ struct mmc_ioc_cmd {
>> };
>> #define mmc_ioc_cmd_set_data(ic, ptr) ic.data_ptr = (__u64)(unsigned long) ptr
>>
>> -#define MMC_IOC_CMD _IOWR(MMC_BLOCK_MAJOR, 0, struct mmc_ioc_cmd)
>> +/**
>> + * struct mmc_ioc_combo_cmd - combo command information
>> + * @num_of_cmds: number of commands to send
>> + * @mmc_ioc_cmd_list: pointer to list of commands
>> + */
>> +struct mmc_ioc_combo_cmd {
>> + uint8_t num_of_cmds;
>> + struct mmc_ioc_cmd *mmc_ioc_cmd_list;
>> +};
>
> The size of this struct will depend on the pointer size of userspace.
>
> It might be better to use a u64 for the pointer instead. Otherwise
> you'd need a compat ioctl wrapper to copy a 32-bit pointer over on a
> 64-bit kernel.

Oh, good point. I have made this change today. I have also tested
this now with a hacked version of mmc-utils to send a couple status
commands back-to-back. I have tested on -next with ARM32 and on
ARM64 with an older 3.18 kernel (due to lack of mmc support in the
mainline for my ARM64 board) and seems to be working fine. Here is
an updated version ...

Jon