Hi Gwandal -----Original Message----- From: Gwendal Grignou [mailto:gwen...@google.com] Sent: Thursday, July 03, 2014 3:16 AM To: Gwendal Grignou Cc: Avi Shchislowski; Hsin-Hsiang Tseng; Chris Ball; Grant Grundler; c...@laptop.org; Ulf Hansson; linux-mmc@vger.kernel.org; Alex Lemberg; Olof Johansson; Puthikorn Voravootivat Subject: Re: [RFC PATCH 1/1 v7]mmc: Support-FFU-for-eMMC-v5.0
Also: - you don't need to allocate a buffer buf in mmc_ffu_download, you can directly call mmc_ffu_write with fw->data; you are memcopying the data again in mmc_ffu_area_init(). - mmc_ffu_prepare_mrq() is a copy of mmc_test_prepare_mrq(), but in our case, we don't need the write arg, it will be always 1. Is there a way to use an existing function instead of recopying what is in mmc_test.c? Gwendal. On Wed, Jul 2, 2014 at 1:35 PM, Gwendal Grignou <gwen...@chromium.org> wrote: > [resent in text mode] > Avi, > + I am testing the patch on a system where the firmware image is on > the emmc itself. It fails most of the time. The problem is > mmc_ffu_download has been call with mmc_claim_host called, preventing > mmcqd from pulling the firmware from the disk. The dead lock situation > cause the machine to panic. It the file is in the cache, the firmware > upgrade goes further. > > Looking at stacks: > > mmc: > ... > sleep_on_buffer+0xe/0x12 > ... > bh_submit_read+0x49/0x5b > ... > _request_firmware+0x3b0/0x84e > request_firmware+0x16/0x18 > mmc_ffu_download+0xc7/0x8d7 > > and mmcqd/0: > schedule+0x69/0x6b > __mmc_claim_host+0xc6/0x19a > ... > mmc_get_card+0x25/0x28 > mmc_blk_issue_rq+0x41/0x403 > > + in mmc_ffu_write(), don't you want to check that the eMMC has > received the payload by looking at > NUMBER_OF_FW_SECTORS_CORRECTLY_PROGRAMMED ext_csd register? In patch v8 we are lo0oking at it and use it to compare the payload > > + did you test with eMMC that do not support MODE_OPERATION_CODES? > In mmc_ffu_restart, you call > mmc_power_save_host/mmc_power_restore_host. Can the reset be achieve > with mmc_hw_reset() which has the advantage to be more atomic? We found out that there platforms that do not support this API > > Ulf, Chris, > + One extra advantage of using request_firmware is if we can trust > udev (or whoever is serving the firmware uevent request), we are > guaranteed that we can trust the firmware as well. If we can send the > firmware payload via ioctl, we can not be sure the caller of that > ioctl is not an attacker that wants to put a bad firmware in the eMMC. > > On Tue, Jul 1, 2014 at 4:39 AM, Avi Shchislowski > <avi.shchislow...@sandisk.com> wrote: >> Hi Chris, >> >> Re your concern that the patch was not tested – some of SanDisk’s OEM >> customers have implemented the patch, and they report that it is working for >> them. >> >> Thanks >> Avi >> >> -----Original Message----- >> From: Hsin-Hsiang Tseng [mailto:hsinhsiangts...@gmail.com] >> Sent: Tuesday, July 01, 2014 9:29 AM >> To: Chris Ball >> Cc: Grant Grundler; c...@laptop.org; Ulf Hansson; >> linux-mmc@vger.kernel.org; Alex Lemberg; Avi Shchislowski; Olof >> Johansson >> Subject: Re: [RFC PATCH 1/1 v7]mmc: Support-FFU-for-eMMC-v5.0 >> >> Hi, Chris, >> I think there are two reasons why doing FFU in kernel-space makes more sense. >> >> First, some eMMC (according to extCSD[492] FFU_FEATURES) will need to >> restart eMMC after FFU. >> >> Second, if host want to vouch "atmoic" in FFU process, we need to call >> mmc_claim_host API, and this API is the kernel-space code. I said "if" >> is because eMMC vendors may take care this. So, host may not need to worry >> about this. >> >> Thanks, >> Hsinhsiang >> >> 2014-07-01 8:34 GMT+08:00 Chris Ball <ch...@printf.net>: >>> Hi Grant, sorry about the delay. >>> >>> On Tue, Jul 01 2014, Grant Grundler wrote: >>>> ping? Another week is past. >>>> >>>> I know you guys are busy with other stuff. But please at least >>>> publish what you think the next steps for this patch are. >>>> >>>> This is a critical patch for supporting eMMC products and adding >>>> additional support for other vendors. I'd like to continue working >>>> with "upstream" and the lack of response in this case is making >>>> this unnecessarily difficult. >>> >>> It's tough to merge a significant patch that you haven't tested and >>> we haven't tested. We could do it, though -- I'd like to hear what >>> Ulf thinks. Please could you give a quick explanation of why doing >>> this in kernel-space makes more sense than using your mmc-utils version? >>> >>> Thanks, >>> >>> - Chris. >>> -- >>> Chris Ball <http://printf.net/> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" >>> in the body of a message to majord...@vger.kernel.org More majordomo >>> info at http://vger.kernel.org/majordomo-info.html
