Re: [PATCH v4 2/3] nvdimm acpi: introduce _FIT

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 11/03/2016 09:00 PM, Igor Mammedov wrote:




>>> just drop this and describe properly 'len' in spec section
>>> i.e. len: length of entire returned data (including the header)
>>
>> Okay, i will change the spec like this:
>>
>>     QEMU Writes Output Data (based on the offset in the page):
>>     [0x0 - 0x3]: 4 bytes, length of entire returned data (including the header)
>>
>> And drop the length field in Read_Fit return buffer, doc
>> the fit buffer like this:
>>
>>     +----------+--------+--------+-------------------------------------------+
>>     |  Field   | Length | Offset |                 Description               |
>>     +----------+--------+--------+-------------------------------------------+
> you need to add length here, otherwise this table is not correct

Ah, so i am confused.

struct NvdimmFuncReadFITOut definition is based on the layout of
Read_FI output. You suggested to drop the length filed in NvdimmFuncReadFITOut
but keep it in the layout, it is not consistent.

I missed something?

>
>
>>     |          |        |        | return status codes                       |
>>     |          |        |        | 0x100 - error caused by NFIT update while |
>>     | status   |   4    |   0    | read by _FIT wasn't completed, other      |
>>     |          |        |        | codes follow Chapter 3 in DSM Spec Rev1   |
>>     +----------+--------+--------+-------------------------------------------+
>>     | fit data | Varies |   8    | FIT data, The remaining size in the       |
>>     |          |        |        | returned buffer is used by FIT            |
>>     +----------+--------+--------+-------------------------------------------+
>>
>>
>>
>>>> +}
>>>> +
>>>> +static void nvdimm_dsm_reserved_root(AcpiNVDIMMState *state, NvdimmDsmIn *in,
>>>> +                                     hwaddr dsm_mem_addr)
>>> function name doesn't make any sense to me
>>
>> As i explained above, handle 0x10000 indicates the reserved _DSM method is
>> called on the root device...
>>
>> It makes sense now? :)
> function name should reflect what it does,
> i.e use verb and I see only nouns here.

Got it, will change it to: nvdimm_handle_reserved_root_method().