Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel.

View Change

10 comments:

• File ite_ecfw.c:

  • Patch Set #14, Line 90: wait_for_ext_crystal

    the name is confusing; 1 for this bit means `use internal clock until external crystal is ready`, so correct would be `no_wait_for_ext_crystal` or `int_osc_until_crystal_ready`

  • Patch Set #14, Line 105: constant?

    yes

  • Patch Set #14, Line 112:

    enum {
    	ESPI_IF = 0xa4,
    	LPC_IF,
    };
    
    

    Actually, bit 0 controls LPC vs eSPI, so we could also have 0xa4 static and a bitfield. having the static value in the enum values isn't wrong, though.

    struct ite_string {
      ...
      uint8_t _a4_byte :7;
      uint8_t espi_lpc :1;
      ...
    }

  • Patch Set #14, Line 315: = (blocks_1_2 ? 0x94 : 0x85);

    we know the bits / have a struct. let's use it. what about sth like this?

  • Patch Set #14, Line 324: 0xaa

    this means "disable", let's give it a name or comment at least

• File ite_ecfw.c:

  • Patch Set #17, Line 142: shutdown

    isn't it `force_off` or `wdg_reset`?

  • Patch Set #17, Line 208: 0

    maybe add enum or define for the type(s)?

  • Patch Set #17, Line 782:

    	if (!ec_write_reg(0xf9, 0x20, EC_MAX_STATUS_CHECKS) ||
    	    !ec_write_reg(0xfa, 0x02, EC_MAX_STATUS_CHECKS) ||
    	    !ec_write_reg(0xfb, 0x00, EC_MAX_STATUS_CHECKS) ||
    	    !ec_write_reg(0xf8, 0xb1, EC_MAX_STATUS_CHECKS)) {
    		msg_perr("Unable to initialize controller.\n");
    		return 1;
    	}
    
    

    drop; done in line #208 already

  • Patch Set #17, Line 813: probe_ite_superio_support(ctx_data);

    don't we want to probe the sio before sending any ec commands and exit if it isn't compatible?

  • Patch Set #17, Line 835:

    
    	if (register_shutdown(ite_ecfw_shutdown, ctx_data))
    		goto ite_ecfw_init_exit;
    
    

    do we really want to reset the device, when flashrom fails to register the shutdown callback?

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