[Skiboot] [PATCH 8/8] Remove dead POWER7 code

Nicholas Piggin npiggin at gmail.com
Sun Nov 10 21:46:30 AEDT 2019


Stewart Smith's on November 10, 2019 3:48 am:
> On Thu, Nov 7, 2019, at 5:52 AM, Nicholas Piggin wrote:
>> diff --git a/core/direct-controls.c b/core/direct-controls.c
>> index 507a16f50..424b3d94c 100644
>> --- a/core/direct-controls.c
>> +++ b/core/direct-controls.c
>> @@ -522,9 +522,6 @@ int dctl_set_special_wakeup(struct cpu_thread *t)
>>  	struct cpu_thread *c = t->primary;
>>  	int rc = OPAL_SUCCESS;
>>  
>> -	if (proc_gen != proc_gen_p9 && proc_gen != proc_gen_p8)
>> -		return OPAL_UNSUPPORTED;
>> -
> 
> It looks like this changes the behaviour when we don't have code to set special wakeup. 
> 
> we'd actually fall through to attempting the p8 special wakeup code, which is probably going to awkwardly fail on non-p8. I don't *think* there's a check higher up the call stack that'd catch it

Right, this is Vasant's point as well...

But we don't support anything except P8 and P9, so I don't really see
the importance of this. Yes, if you could drop this firmware onto some
arbitrary CPU and expect things to mostly work, then it might make sense
to be worried about unknown CPUs. But that's not really the case. We
have to go through every single instance of 'proc_gen' -- which I did
for P10 (not implementing everything but at least adding placeholders).

So I don't see much point. If we wanted to have a policy like adding

  else if (proc_gen == proc_gen_unknown)
    assert(0);

in cases that we need to fill in to support a new CPU, then fine, but
I don't see much benefit because searching for proc_gen is good enough.

Thanks,
Nick



More information about the Skiboot mailing list