Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35293 )
Change subject: mb/lenovo: Add ThinkPad R60 iGPU support ......................................................................
Patch Set 39:
(16 comments)
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@9 PS38, Line 9: -It can be 100% open source. Please add a space after the “bullet point”.
- It can be 100 % Open Source.
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@10 PS38, Line 10: -Untested on Radeon units. Untested on boards with external Radeon graphics adapter.
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@11 PS38, Line 11: -Some problem on 1400x1050 IPS display[1] if legacy VGA text mode is used. What problem? That it is a little off to the left side?
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@15 PS38, Line 15: on in
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@16 PS38, Line 16: -Sometimes it takes 20s to boot to payload. Both SeaBIOS and GRUB?
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@19 PS38, Line 19: I want it to be in main coreboot tree for now, : and later work into making it into variant board. : I want to make it into variant board when it's merged. : Because I don't think it would be good to further convolute this patch. : I did something and I can't edit this patch. : Will make it into variant board when it's merged. What would it be a variant of? Lenovo T60?
Please use the full text width of 75 characters. Reword:
After addition to the coreboot repository, it might be converted to a variant.
https://review.coreboot.org/c/coreboot/+/35293/38//COMMIT_MSG@26 PS38, Line 26: [1] https://imgur.com/a/0wpMGsm Please add the image to the documentation.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/in... File Documentation/mainboard/index.md:
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/in... PS38, Line 72: - [R60 iGPU](lenovo/r60.md) Please sort it and put it above T4xx.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... File Documentation/mainboard/lenovo/r60.md:
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 5: magnesum magnesium
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 5: access Put on line above?
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 8: HMM Please spell it out, and link to it, if possible.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 15: flashrom(use Please add a space before the (.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 15: use flashrom-git as of 17.09.2019 What commit fixing an issue do you need?
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 20: -Several pixel black bar at the left side of screen. Doesn't appear in Linux. Add the image here.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 11: -Intel WiFi card : -Suspend and resume : -Libgfxinit : -GRUB2 payload : -Reflashing with flashrom(use flashrom-git as of 17.09.2019) : -2G+1G memory configuration working : : ## Things not working: : -2504 dock USB ports : -Several pixel black bar at the left side of screen. Doesn't appear in Linux. : -Rfkill button behaviour is inverted Please put a space after the “bullet point”.
https://review.coreboot.org/c/coreboot/+/35293/38/Documentation/mainboard/le... PS38, Line 22: Please add the known problems from the commit message here too.