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

[Nicholas Piggin]

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..."

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.

Thanks,
Nick