[coreboot-gerrit] Change in coreboot[master]: soc/intel/cannonlake: Clear the GPI IS & IE registers

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )

Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers
......................................................................

Patch Set 4:
> Patch Set 4:
> > Patch Set 4:
> > > Patch Set 4:
> > > > Patch Set 4:
> > > > > Patch Set 4:
> > > > > > Patch Set 4:
> > > > > >
> > > > > > > Patch Set 4: -Code-Review
> > > > > > > 
> > > > > > > > Patch Set 4:
> > > > > > > > 
> > > > > > > > This can go into bootblock, not ramstage.  We don't have any devices out in the field yet with locked R/O firmware.
> > > > > > > 
> > > > > > > make sense. may be finalize.c ?
> > > > > >
> > > > > > Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
> > > > >
> > > > > Something like bootblock_soc_early_init ?
> > > >
> > > > We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
> > >
> > > https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
>
> I believe it's p2sb_enable_bar(), which is called from bootblock_soc_early_init(), so  adding bootblock_mainboard_early_init() to hatch and moving this there probably makes sense

Yes that would work. But, in my opinion this change is required for all mainboards and would be good to keep in SoC code -- either in bootblock_soc_early_init() after bootblock_pch_early_init() or in bootblock_soc_init().