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

[Stewart Smith]

On Sun, Nov 10, 2019, at 2:46 AM, Nicholas Piggin wrote:
> 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.

Yeah, you have to end up being studious and getting every code path. I was more thinking of being kinder to those who don't do that.