View Change

19 comments:

• Commit Message:

  • Patch Set #3, Line 7: sc7180: Add support for sn65dsi86 bridge.

    Please remove the dot/period at the end of the commit message.

  • Patch Set #3, Line 10:

    Please mention the datasheet name and revision.

• File src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c:

  • Patch Set #3, Line 13: #define bridge_debug(x...) do {if (0) printk(BIOS_DEBUG, x); } while (0)

    Please add a separate debug macro. Please look how other drivers do it.

  • Patch Set #3, Line 15: #define BRIDGE_GETHIGHERBYTE(x) (uint8_t)((x&0xFF00)>>8)

    Please add spaces around the operator and use lowercase hex letters.

  • Patch Set #3, Line 47: * by the bridge in Mbps unit.

    Fits on the line above?

  • Patch Set #3, Line 59: Disable

    disable

  • Patch Set #3, Line 97: int

    unsigned int

  • Patch Set #3, Line 141: int

    unsigned int

  • Patch Set #3, Line 219: int

    size_t or unsigned_int?

  • Patch Set #3, Line 224: int

    unsigned int?

  • Patch Set #3, Line 248: printk(BIOS_ERR, "ERROR: valid dp rate not found");

    Maybe:

    No valid dp rate found.

    What is the consequence for the user?

  • Patch Set #3, Line 291: printk(BIOS_ERR, "ERROR: PLL lock failure\n");

    … after 500 ms.

  • Patch Set #3, Line 310: printk(BIOS_ERR, "ERROR: Link training failed");

    … after 500 ms timeout.

    What is the consequence for the user? Display might not work?

  • Patch Set #3, Line 329: version

    versions

  • Patch Set #3, Line 332: to compatible

    to be

  • Patch Set #3, Line 327:

    /*
     * support bridge HPD function
     * some hardware version do not support bridge hdp,
     * we use 360ms to try to get the hpd single now,
     * if we can not get bridge hpd single, it will delay 360ms,
     * also meet the bridge power timing request, to compatible
     * all of the hardware version
     */
    

    Please re-flow for 96 characters.

  • Patch Set #3, Line 337: 360

    360 ms is very long for coreboot.

  • Patch Set #3, Line 385: /* Disable vstream */

    Why should it be disabled?

  • Patch Set #3, Line 393: /* color bar disabled */

    Disable color bar

    What is a color bar?

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