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]

 



Hi Gwandal

-----Original Message-----
From: Gwendal Grignou [mailto:gwendal@xxxxxxxxxx] 
Sent: Thursday, July 03, 2014 3:16 AM
To: Gwendal Grignou
Cc: Avi Shchislowski; Hsin-Hsiang Tseng; Chris Ball; Grant Grundler; cjb@xxxxxxxxxx; Ulf Hansson; linux-mmc@xxxxxxxxxxxxxxx; 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 <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?
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.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
��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥





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

  Powered by Linux