Re: [RFC PATCH 1/1 v7]mmc: Support-FFU-for-eMMC-v5.0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <gwendal@xxxxxxxxxxxx> 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?
>
> + 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?
>
> 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.Shchislowski@xxxxxxxxxxx> 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:hsinhsiangtseng@xxxxxxxxx]
>> Sent: Tuesday, July 01, 2014 9:29 AM
>> To: Chris Ball
>> Cc: Grant Grundler; cjb@xxxxxxxxxx; Ulf Hansson; linux-mmc@xxxxxxxxxxxxxxx; 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 <chris@xxxxxxxxxx>:
>>> 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 majordomo@xxxxxxxxxxxxxxx More majordomo
>>> info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux