build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32719 ) Change subject: drivers/apple: Add hybrid graphics driver ...................................................................... Patch Set 1: (99 comments) https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/ch... File src/drivers/apple/hybrid_graphics/chip.h: https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/ch... PS1, Line 27: unsigned int gmux_indexed; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... File src/drivers/apple/hybrid_graphics/gmux.c: https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 28: int i = 200; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 29: u8 gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 31: while (i && (gwr & 0x01)) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 32: inb(GMUX_IOSTART + GMUX_PORT_READ); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 32: inb(GMUX_IOSTART + GMUX_PORT_READ); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 33: gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 33: gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 34: udelay(100); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 34: udelay(100); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 35: i--; code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 35: i--; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 36: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 38: return !!i; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 43: int i = 200; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 44: u8 gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 46: while (i && !(gwr & 0x01)) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 47: gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 47: gwr = inb(GMUX_IOSTART + GMUX_PORT_WRITE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 48: udelay(100); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 48: udelay(100); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 49: i--; code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 49: i--; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 50: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 52: if (gwr & 0x01) please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 53: inb(GMUX_IOSTART + GMUX_PORT_READ); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 53: inb(GMUX_IOSTART + GMUX_PORT_READ); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 55: return !!i; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 60: return inb(GMUX_IOSTART + port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 65: u8 val; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 67: gmux_index_wait_ready(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 68: outb((port & 0xff), GMUX_IOSTART + GMUX_PORT_READ); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 69: gmux_index_wait_complete(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 70: val = inb(GMUX_IOSTART + GMUX_PORT_VALUE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 72: return val; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 77: outb(val, GMUX_IOSTART + port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 83: outb(val, GMUX_IOSTART + GMUX_PORT_VALUE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 84: gmux_index_wait_ready(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 85: outb(port & 0xff, GMUX_IOSTART + GMUX_PORT_WRITE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 86: gmux_index_wait_complete(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 91: return inl(GMUX_IOSTART + port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 96: u32 val; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 98: gmux_index_wait_ready(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 99: outb((port & 0xff), GMUX_IOSTART + GMUX_PORT_READ); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 100: gmux_index_wait_complete(); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 101: val = inl(GMUX_IOSTART + GMUX_PORT_VALUE); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 103: return val; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 108: const struct drivers_apple_hybrid_graphics_config *config = dev->chip_info; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 109: if (config->gmux_indexed) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 109: if (config->gmux_indexed) { braces {} are not necessary for any arm of this statement https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 110: return gmux_index_read8(port); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 110: return gmux_index_read8(port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 111: } else { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 112: return gmux_pio_read8(port); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 112: return gmux_pio_read8(port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 113: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 118: const struct drivers_apple_hybrid_graphics_config *config = dev->chip_info; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 119: if (config->gmux_indexed) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 119: if (config->gmux_indexed) { braces {} are not necessary for any arm of this statement https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 120: gmux_index_write8(port, val); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 120: gmux_index_write8(port, val); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 121: } else { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 122: gmux_pio_write8(port, val); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 122: gmux_pio_write8(port, val); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 123: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 128: const struct drivers_apple_hybrid_graphics_config *config = dev->chip_info; please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 129: if (config->gmux_indexed) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 129: if (config->gmux_indexed) { braces {} are not necessary for any arm of this statement https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 130: return gmux_index_read32(port); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 130: return gmux_index_read32(port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 131: } else { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 132: return gmux_pio_read32(port); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 132: return gmux_pio_read32(port); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 133: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 138: if (enable) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 139: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 1); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 139: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 1); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 140: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 3); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 140: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 3); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 141: } else { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 142: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 1); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 142: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 1); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 143: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 0); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 143: gmux_write8(dev, GMUX_PORT_DISCRETE_POWER, 0); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 144: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 149: if (dgpu) { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 150: gmux_write8(dev, GMUX_PORT_SWITCH_DDC, 2); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 150: gmux_write8(dev, GMUX_PORT_SWITCH_DDC, 2); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 151: gmux_write8(dev, GMUX_PORT_SWITCH_DISPLAY, 3); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 151: gmux_write8(dev, GMUX_PORT_SWITCH_DISPLAY, 3); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 152: } else { please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 153: gmux_write8(dev, GMUX_PORT_SWITCH_DDC, 1); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 153: gmux_write8(dev, GMUX_PORT_SWITCH_DDC, 1); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 154: gmux_write8(dev, GMUX_PORT_SWITCH_DISPLAY, 2); code indent should use tabs where possible https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 154: gmux_write8(dev, GMUX_PORT_SWITCH_DISPLAY, 2); please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/gm... PS1, Line 155: } please, no spaces at the start of a line https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/hy... File src/drivers/apple/hybrid_graphics/hybrid_graphics.c: https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/hy... PS1, Line 27: printk(BIOS_INFO, "Hybrid graphics enable_dev\n"); Prefer using '"%s...", __func__' to using 'enable_dev', this function's name, in a string https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/hy... PS1, Line 48: brightness = gmux_index_read32(GMUX_PORT_BRIGHTNESS) & GMUX_BRIGHTNESS_MASK; line over 80 characters https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/ro... File src/drivers/apple/hybrid_graphics/romstage.c: https://review.coreboot.org/#/c/32719/1/src/drivers/apple/hybrid_graphics/ro... PS1, Line 32: printk(BIOS_INFO, "Hybrid graphics early_hybrid_graphics\n"); Prefer using '"%s...", __func__' to using 'early_hybrid_graphics', this function's name, in a string -- To view, visit https://review.coreboot.org/c/coreboot/+/32719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22b66622cd2da0e9951ee726d650d204fbb8a5bc Gerrit-Change-Number: 32719 Gerrit-PatchSet: 1 Gerrit-Owner: Evgeny Zinoviev <me@ch1p.com> Gerrit-Reviewer: Evgeny Zinoviev <me@ch1p.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-CC: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 09 May 2019 23:06:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment