From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752878AbbIPQBr (ORCPT ); Wed, 16 Sep 2015 12:01:47 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:12749 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752335AbbIPQBp (ORCPT ); Wed, 16 Sep 2015 12:01:45 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Wed, 16 Sep 2015 09:01:29 -0700 Subject: Re: [PATCH V3] mmc: block: Add new ioctl to send multi commands To: Ulf Hansson References: <1442242844-6859-1-git-send-email-jonathanh@nvidia.com> CC: linux-mmc , "linux-kernel@vger.kernel.org" , Seshagiri Holi , "Arnd Bergmann" , Grant Grundler , "Olof Johansson" From: Jon Hunter Message-ID: <55F99261.60003@nvidia.com> Date: Wed, 16 Sep 2015 17:01:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.21.132.159] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16/09/15 12:08, Ulf Hansson wrote: > On 14 September 2015 at 17:00, Jon Hunter wrote: >> From: Seshagiri Holi >> >> 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 multi commands are simple array of >> the existing mmc_ioc_cmd structure. >> >> The structure passed via the ioctl uses a __u64 type to specify the number >> of commands (so that the structure is aligned on a 64-bit boundary) and a >> zero length array as a header for list of commands to be issued. The >> maximum number of commands that can be sent is determined by >> MMC_IOC_MAX_CMDS (which defaults to 255 and should be more than >> sufficient). >> >> Signed-off-by: Seshagiri Holi >> Cc: Arnd Bergmann >> Cc: Grant Grundler >> Cc: Olof Johansson >> [ jonathanh@nvidia.com: Rebased on linux-next from v3.18. Changed >> userspace pointer type for multi command to be a u64. Renamed >> from combo commands to multi commands. Updated patch based upon >> feedback review comments received. Updated commit message ] >> Signed-off-by: Jon Hunter > > Overall this looks good to me, only some minor nits. > > Also, it does seem like you have invested quite some work here. > Perhaps you should claim the authorship and instead give Seshagiri > some cred in the commit message + his signed-off by? Yes that's fine with me, plus everyone will know who to blame ;-) > Anyway, I am fine with whatever! > >> --- >> V3 changes: >> - Updated mmc_ioc_multi_cmd structure per Grant's feedback >> V2 changes: >> - Updated changelog per Arnd's feedback >> - Moved mmc_put/get_card() outside of __mmc_blk_ioctl_cmd() >> >> drivers/mmc/card/block.c | 214 ++++++++++++++++++++++++++++++----------- >> include/uapi/linux/mmc/ioctl.h | 19 +++- >> 2 files changed, 177 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >> index c742cfd7674e..2007023815cb 100644 >> --- a/drivers/mmc/card/block.c >> +++ b/drivers/mmc/card/block.c >> @@ -387,6 +387,24 @@ out: >> return ERR_PTR(err); >> } [snip] >> +static int mmc_blk_ioctl_cmd(struct block_device *bdev, >> + struct mmc_ioc_cmd __user *ic_ptr) >> +{ >> + struct mmc_blk_ioc_data *idata; >> + struct mmc_blk_data *md; >> + struct mmc_card *card; >> + int err; >> + >> + /* >> + * The caller must have CAP_SYS_RAWIO, and must be calling this on the >> + * whole block device, not on a partition. This prevents overspray >> + * between sibling partitions. >> + */ >> + if ((!capable(CAP_SYS_RAWIO)) || (bdev != bdev->bd_contains)) >> + return -EPERM; > > This check is common for multi and non-multi. Please move it to the > mmc_blk_ioctl() to avoid some code duplication. Yes that's true. I can move but it means also passing bdev to __mmc_blk_ioctl_cmd() as another argument. It is not a big deal, but it was more convenient to test here. If your preference is to consolidate the tests to one place then I will move this test. Cheers Jon