Attention is currently required from: Kapil Porwal, Nick Vaccaro, Subrata Banik, sridhar siricilla.

View Change

2 comments:

• File src/soc/intel/common/block/systemagent/systemagent.c:

  • Patch Set #6, Line 327: const struct device *dev

    SA is nothing but PCI_0_00_0 aka host bridge.
    

    \_\_pci_0_00_0' is a real reference in the static.c code. You can do pci_read_config32(\_\_pci_0_00_0, PCIEXBAR);

    are you suggesting to use rather passing the `dev` argument to the function?

    ```
    	struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT);
    ```

    You can get a reference from the devicetree struct device directly, rather than using runtime functions (see build/mainboard/../../static.c) . Using the alias is indeed a better idea than the reference via pci numbers: _dev_system_agent_ptr would be it.

    The advantage of this over the runtime, is that you get a direct reference (no searching for it in tree), and the if the device is not there (it's in chipset.cb so that won't ever happen) the linker will complain, which is a buildtime error, which are better than runtime ones.

  • Patch Set #6, Line 432: sa_get_dpr_size

    txt_get_chipset_dpr() in systemagent_early.c does the same.

    good candidate for the convergence at some point. At least it covers the raw read from the register (like what line#435 does below) and additionally, this function returning the size in MiB (which may not be needed for early code piece).

    I will pick that later for convergence. hoping u r good with that plan ?

    sounds good.

To view, visit change 80362. To unsubscribe, or for help writing mail filters, visit settings.