Change in coreboot[master]: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default

[Benjamin Doron (Code Review)]

Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39980 )

Change subject: soc/intel/skylake: vr_config: enable PSI3 and PSI4 by default
......................................................................

Patch Set 8:
> Patch Set 8:
> > > > > > whoops, looks like I pushed an older patch when I rebased another change, I'll fix that
> > > > >
> > > > > Ah, alright. Not a problem (maybe -1 was unnecessary)
> > > >
> > > > A -1 is not a blocker. In fact, it's a good way to indicate mild dissatisfaction with a change, as it stands out in a field of green scores.
> > >
> > > but -1 is not a thing that should be used just because of a wrong comment IMHO
> >
> > There are no real guidelines, what -1 means. So do not interpret too much into it, besides another iteration is needed. -1 scores will get removed with the next iteration, so in my opinion it’s a good way to prevent accidental submission of the change-set.
>
> Agreed, and also, the -1 has a description: "I would prefer that you didn't
> submit this". Beside some very rare 1 out of some 100 changes, "this" refers
> to a patch set, not the change itself.
> > Though, as comments need to be explicitly marked as resolved nowadays, -1 are not needed for that anymore.
>
> ACR has very different mechanics, though. Anyone can mark a comment as
> resolved, even the change author. But not everyone can remove a -1 (with-
> out also losing all +2s). Many people mark comments as resolved that really
> aren't (and comment authors are not notified, unless it's a blunt Done or
> Ack). Unless we can prevent that, I think -1 is still a very valid choice.

I hadn't been entirely sure that was how ACR worked, and yes, that was my other option.

Anyway, I'm glad we addressed some of this.