Attention is currently required from: Cliff Huang, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jingyuan Liang, Kapil Porwal, Kyoung Il Kim, Paul Menzel, Subrata Banik, Tarun.

View Change

3 comments:

• File src/mainboard/google/rex/variants/rex0/fw_config.c:

  • Patch Set #4, Line 10: #if CONFIG(SOC_INTEL_TOUCH)

    Why is the preprocessor magic needed?

    I'd say it would make much more sense to put the "touch" stuff in some `touch.c` file, which would be conditionally built.

• File src/mainboard/google/rex/variants/rex0/overridetree.cb:

  • Patch Set #2, Line 33: THC NOTE for P1 and P2:reworked

    Please add a space after the colon.

    Done

  • Patch Set #2, Line 84: register "thc_configuration" = "THC_CONFIG_DOUBLE_CONTROLLER"

    First, the name 'thc_configuration' is chosen instead of thc_config since 'config' has been used for […]

    Looks like the requirement for THC 0 to always be enabled is because of PCI: function 0 of a multi-function device always needs to exist. https://github.com/coreboot/coreboot/blob/fce08d78837431a3309d2b8705555fd81bd09c29/src/soc/intel/meteorlake/include/soc/pci_devs.h#L87 shows that THC 0 is function 0, whereas THC 1 is function 1; as I expected.

    Given that the THCs (Touch Host Controllers?) have corresponding PCI devices, one could use the PCI devices themselves to know if a given controller is enabled. This would only require mapping ports to controllers (one port maps to only one controller). Something like this should work:

    ```
    // In chip.h
    enum touch_port_to_ctrlr {
    	TO_THC_DISABLED = 0,
    	TO_THC_0,
    	TO_THC_1,
    };

    // In chip config
    enum touch_port_to_ctrlr port_to_thc_map[2];
    ```

    This would allow mapping any port to any THC, and should directly map to FSP UPDs as on TGL (UPD names may differ): https://github.com/coreboot/coreboot/blob/fce08d78837431a3309d2b8705555fd81bd09c29/src/soc/intel/tigerlake/fsp_params.c#L507

    Now I wonder, would this work for Rex use-case, or do any enabled THCs require having at least one port assigned?

    ```
    device ref thc_0 on
    	register "port_to_thc_map[0]" = "TO_THC_DISABLED"
    end
    device ref thc_1 on
    	register "port_to_thc_map[1]" = "TO_THC_1"
    end
    ```

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