[Skiboot] [PATCH 59/61] xive2: Add NCU_SPEC_BAR to stop engine for restore

[Pratik Sampat]

On 28/07/21 11:40 am, Cédric Le Goater wrote:
> On 7/27/21 12:50 PM, Vasant Hegde wrote:
>> On 7/23/21 8:33 PM, Pratik Sampat wrote:
>>>
>>> On 22/07/21 3:01 pm, Vasant Hegde wrote:
>>>> On 7/21/21 7:05 PM, Cédric Le Goater wrote:
>>>>> On 7/19/21 3:20 PM, Vasant Hegde wrote:
>>>>>> From: Vaidyanathan Srinivasan <svaidy at linux.ibm.com>
>>>>>>
>>>>>> P10 Stop engines have apis similar to P9 to set xscom restores
>>>>>> after wakeup from deep-sleep states.
>>>>>>
>>>>>> This xscom restore will be used to support STOP11 on P10.
>>>>>>
>>>>>> Signed-off-by: Vaidyanathan Srinivasan <svaidy at linux.ibm.com>
>>>>>> Signed-off-by: Pratik Rajesh Sampat <psampat at linux.ibm.com>
>>>>>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>>>>>> ---
>>>>>>    hw/xive2.c | 31 ++++++++++++++++++++++++++-----
>>>>>>    1 file changed, 26 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/xive2.c b/hw/xive2.c
>>>>>> index f559b0f9a..d2fc2ad69 100644
>>>>>> --- a/hw/xive2.c
>>>>>> +++ b/hw/xive2.c
>>>>>> @@ -20,8 +20,8 @@
>>>>>>    #include <bitmap.h>
>>>>>>    #include <buddy.h>
>>>>>>    #include <phys-map.h>
>>>>>> -#include <p9_stop_api.H> /* TODO (p10): need P10 stop state engine */
>>>>>> -
>>>>>> +#include <p9_stop_api.H>
>>>>>> +#include <p10_stop_api.H>
>>>>> Why do we need both ?
>>>>>
>>>> Because p10_stop_api.H uses definitions from p9_stop_api.H. May be we should move common
>>>> definitions to separate header file (say stop_api.h ) and then include that in P[9/10]_stop_api.H file?
>>>>
>>> I agree that there are a lot of common definitions around both P9 and P10 stop
>>> API header, and could definitely be cleaned up and merged. However the headers
>>> are direct mirror from hostboot and they maintain different code-bases for
>>> P9 and P10.
>>>
>>> My cause of concern stems from the fact that if we refactor the libraries here
>>> and if hostboot updates their libraries we would manually each time have
>>> to code those changes up in our refactored library too.
>> I'm referring to include/p10_stop_api.H here. AFAIC this file is already modified.
>>
>> Something like below works?
> Looks good to me.

Sure, I can call the P9 headers in P10 that way.
Thanks!

> Thanks,
>
> C.
>
>>
>> diff --git a/hw/xive2.c b/hw/xive2.c
>> index 674005895..aece99a0d 100644
>> --- a/hw/xive2.c
>> +++ b/hw/xive2.c
>> @@ -20,7 +20,6 @@
>>   #include <bitmap.h>
>>   #include <buddy.h>
>>   #include <phys-map.h>
>> -#include <p9_stop_api.H>
>>   #include <p10_stop_api.H>
>>
>>   /* Verbose debug */
>> diff --git a/include/p10_stop_api.H b/include/p10_stop_api.H
>> index f3854fefc..3a8772195 100644
>> --- a/include/p10_stop_api.H
>> +++ b/include/p10_stop_api.H
>> @@ -23,6 +23,7 @@
>>
>>   #ifdef __SKIBOOT__
>>       #include <skiboot.h>
>> +    #include <p9_stop_api.H>
>>   #endif
>>
>>
>> -Vasant