Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 )

Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
......................................................................

Patch Set 10:
> > > We noticed that PCI devices were not visible on the PCI bus, since 'lpss_mode' was set to ACPI. Can not describe details of testing, since PCI/ACPI modes has been corrected in devicetree.cb.
> >
> > I'm starting to understand. I guess the coreboot code
> > simply doesn't allow anything but a 0 or 1 setting of the
> > `PcdEnable*` devicetree options and expects that only
> > `lpss_acpi_mode` is used to enable ACPI mode.
> >
> > If you have `lpss_acpi_mode` set to 0 and any `PcdEnable*`
> > to 2, I would expect that coreboot won't generate the neces-
> > sary ACPI entries.
> >
> > If you have `lpss_acpi_mode` set to 1, all LPSS devices
> > should be in ACPI mode, no matter what the `PcdEnable*`
> > options say.
> >
> > Such problems arise, when we allow direct copies of FSP
> > UPD options in the devicetree, and try to make sense of
> > them for both worlds, coreboot and FSP.
> >
> > I'm still interested in detailed test results for this
> > patch with `lpss_acpi_mode = 1`. Either it works, then your
> > patch could be merged (with a comment that the FSP documen-
> > tation is incomplete). Or it doesn't work, then we should
> > leave a comment, that the `PcdEnable*` options shouldn't
> > be set to 2 (and probably normalize values to 0/1).
>
> I tested 'lpss_acpi_mode' to 1 and set some PchEnable* to '1':
> For devices set to 1:
> PCI: Static device PCI: 00:xx.x not found, disabling it.

This is what I expected and from your reaction I guess this
probably means we are talking past each other. If I'm not
mistaken, the code in `lpss.c` won't be run in this case. But
this code is vital for proper resource allocation and repor-
ting in ACPI.

To ask more specifically: Does

TEST=Intel Cherry Hill

involve booting to an OS and testing if any of these devices
in ACPI mode are still usable?