[coreboot-gerrit] Change in ...coreboot[master]: [RFC] src/mainboard/cmr/cmedrobo: initial commit
build bot (Jenkins) (Code Review)
gerrit at coreboot.org
Wed Dec 19 15:52:37 CET 2018
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30312 )
Change subject: [RFC] src/mainboard/cmr/cmedrobo: initial commit
......................................................................
Patch Set 1:
(59 comments)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c
File src/mainboard/cmr/cmedrobo/irq_tables.c:
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@34
PS1, Line 34: #define PIRQ_HEADER_SIZE 32
please, no space before tabs
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@35
PS1, Line 35: #define IRQ_INFO_SIZE 16
please, no space before tabs
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@40
PS1, Line 40: PIRQ_HEADER_SIZE + (IRQ_INFO_SIZE * CONFIG_IRQ_SLOT_COUNT), /* u16 size */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@50
PS1, Line 50: /* bus, dev|fn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@51
PS1, Line 51: {0x00,(0x02 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // GFX INTA-PIRQA
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@51
PS1, Line 51: {0x00,(0x02 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // GFX INTA-PIRQA
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@51
PS1, Line 51: {0x00,(0x02 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // GFX INTA-PIRQA
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@52
PS1, Line 52: {0x00,(0x12 << 3)|0x0, {{PIRQC, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SD INTA-PIRQC
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@52
PS1, Line 52: {0x00,(0x12 << 3)|0x0, {{PIRQC, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SD INTA-PIRQC
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@52
PS1, Line 52: {0x00,(0x12 << 3)|0x0, {{PIRQC, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SD INTA-PIRQC
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@53
PS1, Line 53: {0x00,(0x13 << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SATA INTA-PIRQD
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@53
PS1, Line 53: {0x00,(0x13 << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SATA INTA-PIRQD
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@53
PS1, Line 53: {0x00,(0x13 << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // SATA INTA-PIRQD
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@54
PS1, Line 54: {0x00,(0x15 << 3)|0x0, {{PIRQF, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPE INTA-PIRQF
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@54
PS1, Line 54: {0x00,(0x15 << 3)|0x0, {{PIRQF, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPE INTA-PIRQF
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@54
PS1, Line 54: {0x00,(0x15 << 3)|0x0, {{PIRQF, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPE INTA-PIRQF
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@55
PS1, Line 55: {0x00,(0x18 << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQA, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQC, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQA, INTC-PIRQD, INTD-PIRQC
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@55
PS1, Line 55: {0x00,(0x18 << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQA, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQC, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQA, INTC-PIRQD, INTD-PIRQC
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@55
PS1, Line 55: {0x00,(0x18 << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQA, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQC, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQA, INTC-PIRQD, INTD-PIRQC
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@56
PS1, Line 56: {0x00,(0x1b << 3)|0x0, {{PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // HDA INTA-PIRQG
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@56
PS1, Line 56: {0x00,(0x1b << 3)|0x0, {{PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // HDA INTA-PIRQG
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@56
PS1, Line 56: {0x00,(0x1b << 3)|0x0, {{PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // HDA INTA-PIRQG
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@57
PS1, Line 57: {0x00,(0x1c << 3)|0x0, {{PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}, {PIRQG, PCI_IRQS}, {PIRQD, PCI_IRQS}}, 0x0, 0x0}, // PCIE INTA-PIRQE, INTB-PIRQF, INTC-PIRQG, INTD-PIRQD
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@57
PS1, Line 57: {0x00,(0x1c << 3)|0x0, {{PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}, {PIRQG, PCI_IRQS}, {PIRQD, PCI_IRQS}}, 0x0, 0x0}, // PCIE INTA-PIRQE, INTB-PIRQF, INTC-PIRQG, INTD-PIRQD
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@57
PS1, Line 57: {0x00,(0x1c << 3)|0x0, {{PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}, {PIRQG, PCI_IRQS}, {PIRQD, PCI_IRQS}}, 0x0, 0x0}, // PCIE INTA-PIRQE, INTB-PIRQF, INTC-PIRQG, INTD-PIRQD
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@58
PS1, Line 58: {0x00,(0x1d << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // EHCI INTA-PIRQD
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@58
PS1, Line 58: {0x00,(0x1d << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // EHCI INTA-PIRQD
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@58
PS1, Line 58: {0x00,(0x1d << 3)|0x0, {{PIRQD, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // EHCI INTA-PIRQD
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@59
PS1, Line 59: {0x00,(0x1e << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQD, INTC-PIRQE, INTD-PIRQF
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@59
PS1, Line 59: {0x00,(0x1e << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQD, INTC-PIRQE, INTD-PIRQF
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@59
PS1, Line 59: {0x00,(0x1e << 3)|0x0, {{PIRQB, PCI_IRQS}, {PIRQD, PCI_IRQS}, {PIRQE, PCI_IRQS}, {PIRQF, PCI_IRQS}}, 0x0, 0x0}, // SIO INTA-PIRQB, INTB-PIRQD, INTC-PIRQE, INTD-PIRQF
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@60
PS1, Line 60: {0x00,(0x1f << 3)|0x0, {{0x00, PCI_IRQS}, {PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPC INTB-PIRQG
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@60
PS1, Line 60: {0x00,(0x1f << 3)|0x0, {{0x00, PCI_IRQS}, {PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPC INTB-PIRQG
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@60
PS1, Line 60: {0x00,(0x1f << 3)|0x0, {{0x00, PCI_IRQS}, {PIRQG, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x0, 0x0}, // LPC INTB-PIRQG
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@61
PS1, Line 61: {0x04,(0x00 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x4, 0x0}, // ETH INTA-PIRQA
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@61
PS1, Line 61: {0x04,(0x00 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x4, 0x0}, // ETH INTA-PIRQA
space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@61
PS1, Line 61: {0x04,(0x00 << 3)|0x0, {{PIRQA, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}, {0x00, PCI_IRQS}}, 0x4, 0x0}, // ETH INTA-PIRQA
space required after that close brace '}'
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irq_tables.c@82
PS1, Line 82: if ((sum & 0xff) != intel_routing_table_copy.checksum) {
braces {} are not necessary for single statement blocks
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irqroute.h
File src/mainboard/cmr/cmedrobo/irqroute.h:
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irqroute.h@48
PS1, Line 48: #define PCI_DEV_PIRQ_ROUTES \
Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/irqroute.h@72
PS1, Line 72: #define PIRQ_PIC_ROUTES \
Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c
File src/mainboard/cmr/cmedrobo/mptable.c:
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@47
PS1, Line 47: #define IO_LOCAL_INT(type, intr, apicid, pin) \
macros should not use a trailing semicolon
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@53
PS1, Line 53: smp_write_pci_intsrc(mc, mp_INT, (bus), (dev), (pin), ioapic_id, (line))
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@89
PS1, Line 89:
trailing whitespace
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@89
PS1, Line 89:
please, no spaces at the start of a line
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@135
PS1, Line 135: * Bits 2-6: Originating PCI Device Number (Not its parent bridge device number)
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@138
PS1, Line 138: for(dev = all_devices; dev; dev = dev->next) {
space required before the open parenthesis '('
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@156
PS1, Line 156: if(PCI_IRQ(devn, (intpin - 1)) == existing_pci_entries[i]) {
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@156
PS1, Line 156: if(PCI_IRQ(devn, (intpin - 1)) == existing_pci_entries[i]) {
space required before the open parenthesis '('
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/mptable.c@174
PS1, Line 174: existing_pci_entries[i] = PCI_IRQ(devn, (intpin - 1));
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c
File src/mainboard/cmr/cmedrobo/romstage.c:
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@45
PS1, Line 45:
trailing whitespace
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@45
PS1, Line 45:
please, no spaces at the start of a line
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@75
PS1, Line 75: .DRAMType = 1, /* DRAM Type: 0=DDR3, 1=DDR3L, 2=DDR3U, 4=LPDDR2, 5=LPDDR3, 6=DDR4*/
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@79
PS1, Line 79: .DIMMDensity = 1, /* DRAM device data density: 0=1Gb, 1=2Gb, 2=4Gb, 3=8Gb */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@80
PS1, Line 80: .DIMMBusWidth = 3, /* DIMM Bus Width: 0=8bit, 1=16bit, 2=32bit, 3=64bit */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@83
PS1, Line 83: .DIMMtRPtRCD = 11, /* tRP and tRCD in DRAM clk - 5:12.5ns, 6:15ns, etc. */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@94
PS1, Line 94: .DRAMType = 1, /* DRAM Type: 0=DDR3, 1=DDR3L, 2=DDR3U, 4=LPDDR2, 5=LPDDR3, 6=DDR4*/
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@98
PS1, Line 98: .DIMMDensity = 1, /* DRAM device data density: 0=1Gb, 1=2Gb, 2=4Gb, 3=8Gb */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@99
PS1, Line 99: .DIMMBusWidth = 3, /* DIMM Bus Width: 0=8bit, 1=16bit, 2=32bit, 3=64bit */
line over 80 characters
https://review.coreboot.org/#/c/30312/1/src/mainboard/cmr/cmedrobo/romstage.c@102
PS1, Line 102: .DIMMtRPtRCD = 9, /* tRP and tRCD in DRAM clk - 5:12.5ns, 6:15ns, etc. */
line over 80 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/30312
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I40de818d85a15e6515fe6585f9f8c07336f17242
Gerrit-Change-Number: 30312
Gerrit-PatchSet: 1
Gerrit-Owner: Michał Żygowski <michal.zygowski at 3mdeb.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski at 3mdeb.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-CC: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Wed, 19 Dec 2018 14:52:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181219/30c02e50/attachment.html>
More information about the coreboot-gerrit
mailing list