Attention is currently required from: Cliff Huang, Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph.

View Change

4 comments:

• File src/soc/intel/common/block/pcie/rtd3/rtd3.c:

  • Patch Set #2, Line 238: static bool mutex_created;

    Done

    Why we need to initialize to `false` ? Default value itself is false. Isn't it?

    #include <stdio.h>
    #include <stdbool.h>
    int main() {
       static bool a;
       bool b;
       printf("Default value of static variable : %s\n", a == true ? "true" : "false");
       printf("Default value of non-static variable : %s\n", b == true ? "true" : "false");
       return 0;
    }

    $gcc -o main *.c -lm
    $main
    Default value of static variable : false
    Default value of non-static variable : false

• File src/soc/intel/common/block/pcie/rtd3/rtd3.c:

  • Patch Set #3, Line 83: bool enabl

    Just thoughts:

    Do you really need line #90 if/else clause ?

    can't we have like this?

    enum modphy_pg_state {
      PG_DISABLE = 0,
      PG_ENABLE = 1,
    };

    static void pcie_rtd3_enable_modphy_pg(unsigned int pcie_rp, enum modphy_pg_state state)
    {
       /* Enter the critical section */
       ....
       acpigen_write_store_int_to_namestr(state, "EMPG");
       acpigen_write_delay_until_namestr_int(100, "AMPG", state);

       /* Exit the critical section */
       ....
    }

  • Patch Set #3, Line 112: pcie_rtd3_enable_modphy_pg(pcie_rp, false);

    pcie_rtd3_enable_modphy_pg(pcie_rp, PG_DISABLE);

  • Patch Set #3, Line 159: pcie_rtd3_enable_modphy_pg(pcie_rp, true);

    pcie_rtd3_enable_modphy_pg(pcie_rp, PG_ENABLE);

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