Dear Kerry,
Am Mittwoch, den 14.09.2011, 10:11 +0800 schrieb She, Kerry: > > Sent: Tuesday, September 13, 2011 6:25 PM > > Am Dienstag, den 13.09.2011, 12:08 +0200 schrieb Kerry Sheh: > > > Kerry Sheh ([email protected]) just uploaded a new patch set to > > > gerrit, which you can find at http://review.coreboot.org/206 > > > > is there a typo in your Gerrit user information: »Sheh«? > I have update the profile :) I thought it is written without the h (She) as in your AMD address. It looks like Gerrit now added an h to the end everywhere. > > > -gerrit > > > > > > commit 7f2d27b0bddf97948fff2f81fab52633e0f77709 > > > Author: Kerry She <[email protected]> > > > Date: Tue Sep 13 18:29:27 2011 +0800 > > > > > > rs780: hide unused gpp ports > > > > > > hide unused gpp ports, test on avalue/eax-785e > > > > Copying the commit summary into the commit message body does not have any > > benefits. > > > > Could you update the commit message using `git commit --amend`? > > > > 1. Why is hiding these ports needed? Did you get an error message? > the information get by Lspci -vvv may not accurate if no device on the pcie > port, such as the link speed etc. Thank you for the information and updating the commit message. To even be more elaborate a diff of lspci output could also be added next time. > > 2. Looking at the code, does this patch do more than is written in the > > commit message? What does for example `AtiPcieCfg.PortDetect |= 1 << 2; > > /* Port 2 */` do? Is that also hiding these ports? (I am sorry for this > > noob question.) If this is doing something else please add that to the > > commit message or even better split the commit up to make it even smaller. > This because the pcie training code for gfx port 2 is hardcoded, > If the training successful, we make a mark on the PortDect bitmap. > I think we will improve the magic number later. Thanks again. I do not know if a separate patch for this change would be self-contained. At least it should have been mentioned in the commit message. Mark Brown wrote a blog post about this. > Thanks for your elaborative review. Thank you for your answer and the patches! Thanks, Paul [1] http://www.sirena.org.uk/log/2011/09/09/making-patches-easy-to-review/
signature.asc
Description: This is a digitally signed message part
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
