[Skiboot] [PATCH v8 03/24] hdata: Fix MDST structure

[Vasant Hegde]

On 06/28/2019 06:42 AM, Nicholas Piggin wrote:
> Vasant Hegde's on June 17, 2019 3:10 am:
>> We have split the type field to accommodate below fields which are used by
> 
> In general, can you describe what each patch does in a present-tense
> imperative style?
> 
> "Split the type field..."

Sure. I will try .

> 
> And also expand on the problem it's solving in slightly more depth.
> Sometimes it's nice to describe problem first then solution but either
> way can work.

> 
>> OPAL DUMP (memory preserving IPL).
>>    - data region : dump data regions (like DUMP_REGION_* )
>>    - dump type   : Reflects MDST entry usage (used by SYSDUMP -OR- FADUMP)
> 
> ^ This is pretty difficult to understand why you are doing it.
> 
>> This patch makes structure changes and necessary code adjustment.
> 
> Then this is unnecessary.
> 
>> Note that these fields are not used by FSP to collect dump. They only care
>> about address and size from MDST structure. Hence its safe to make this change.
> 
> This is probably the most important detail about your change isn't it,
> so it should be stated a bit more prominently.
> 
>    [PATCH] hdata: Split MDST 'type' field to accommodate FADUMP
> 
>    The FADUMP facility needs to store region and type information
>    corresponding with each MDST entry because...
> 
>    The existing type field is currently not used by firmware or the FSP,
>    so it is safe to re-purpose it.

Ok.

-Vasant