Ivan Vatlin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38143 )
Change subject: mb/asus/p5qc: Add ASUS P5Q as a variant ......................................................................
Patch Set 6:
(16 comments)
I added back devicetree.cb for P5Q
https://review.coreboot.org/c/coreboot/+/38143/1//COMMIT_MSG Commit Message:
PS1:
Congrats on your first working coreboot port :) […]
Okay, I'll try write something useful in doc :)
- My RS232-USB cable and gender changer for it are still delivering to me (I hate international delivery), so ASAP I will try&test it - PCI card? I don't have one but I guess I can buy it to test slots - Okay, I'll test it - As soon as I find my old router, I could test this too - Hey, there is no reason to drop floppy support (I'll try to buy it too) - I can't test TPM since it's optional plugable module - Test FireWire: check
https://review.coreboot.org/c/coreboot/+/38143/1//COMMIT_MSG@10 PS1, Line 10:
Everybody forgets to sign-off from time to time :P […]
How could I add automatically Signed-off-by? Sorry, some sort of git noob
https://review.coreboot.org/c/coreboot/+/38143/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38143/4//COMMIT_MSG@10 PS4, Line 10: Signed-off by
Missing a `-`: […]
Yikes!
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/Kco... File src/mainboard/asus/p5qc/Kconfig:
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/Kco... PS1, Line 55: config GPIO_C
I've checked and it matches the current gpio.c file. […]
I did it on stock BIOS as you wrote I am the kind of person who reads several times to be sure (even contracts)
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/Kco... PS1, Line 64: # The MARVELL IDE controller delays SeaBIOS a lot and results in an unbootable : # bogus disk. Compiling SeaBIOS without ATA support is a workaround. : : # The Asus P5QL PRO's Marvell controller (88SE6102-NNC2) does not need this, apparently.
Does the Asus P5Q have a Marvell 88SE6102? lspci seems to differ, but the best way to know is to loo […]
My lspci output is: [jenrus@bbox inteltool]$ lspci | grep Marvell 03:00.0 IDE interface: Marvell Technology Group Ltd. 88SE6111/6121 SATA II / PATA Controller (rev b2)
https://review.coreboot.org/c/coreboot/+/38143/4/src/mainboard/asus/p5qc/Kco... File src/mainboard/asus/p5qc/Kconfig:
https://review.coreboot.org/c/coreboot/+/38143/4/src/mainboard/asus/p5qc/Kco... PS4, Line 39: default "p5qc" if BOARD_ASUS_P5QC || BOARD_ASUS_P5Q
I've added some suggestions to the devicetree on Patchset 1, so you might have to undo this. […]
I'll think about it, sounds like lotta work
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... File src/mainboard/asus/p5qc/variants/p5q/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 29: end
Personally, I like to align these `end` keywords. […]
Aligned
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 31: device pci 2.0 off end # Integrated graphics controller : device pci 2.1 off end # Integrated graphics controller 2
These devices don't exist on the P45 northbridge, so you can remove these lines
Deleted
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 33: device pci 3.0 off end # ME : device pci 3.1 off end # ME : device pci 3.2 off end # ME : device pci 3.3 off end # ME
Since these Asus boards don't have any ME firmware, the ME devices should be disabled by default, so […]
Deleted
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 41: # Set AHCI mode.
This comment is outdated (AHCI mode was made the default option, because it's better). […]
Changed
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 43: register "sata_clock_request" = "0" : register "sata_traffic_monitor" = "0"
You can remove these entries. The default value is zero already.
Deleted
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 58: device pci 1c.0 on end # PCIe 1 slot 1
The used PCIe ports may be different on the P5Q. You can check `lspci` while running the stock BIOS. […]
Okay, I'll try to enable Silicon Image controller
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 70: chip superio/winbond/w83667hg-a
You can use `util/superiotool` while running the stock BIOS to double-check these registers. […]
Okay, I'll check it
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 106: irq 0xe4 = 0x10 # 3VSBSW# enable
Even if superiotool does not set bit 4 of this register, it should be enabled. […]
Should I change somrthing here?
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 120: device pci 1f.1 off end # PATA/IDE
This device does not exist on ICH10, so you can remove this line
Deleted
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... PS1, Line 124: IDE
Strictly speaking, this is still SATA, but when it's configured in Legacy mode.
So what should I do with it?