Change in coreboot[master]: vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redun...

List overview All Threads
Download

newer

older

[S] Change in coreboot[master]:...

[M] Change in coreboot[master]:...

Name of user not set (Code Review)

13 Oct 2019 13 Oct '19

11:14 p.m.

awokd@danwin1210.me has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36001 )

Change subject: vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code ......................................................................

vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code

From looking at neighboring code, disabling FIS based switching appears intentional, possibly due to errata. Clean and comment code so it is apparent.

Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Signed-off-by: Joe Moore awokd@danwin1210.me --- M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c 1 file changed, 4 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/36001/1

diff --git a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c index 9f526d8..293ea57 100644 --- a/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c +++ b/src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c @@ -367,11 +367,8 @@ AndMaskDword |= BIT12; }

- if ( FchSataFisBasedSwitching ) { - AndMaskDword |= BIT10; - } else { - AndMaskDword |= BIT10; - } + // FchSataFisBasedSwitching + AndMaskDword |= BIT10;

if ( FchSataAggrLinkPmCap ) { OrMaskDword |= BIT11; @@ -431,11 +428,8 @@ RwMem ((Bar5 + 0x0FC), AccessWidth32, (UINT32)~(BIT20), 0x00); }

- if ( FchSataFisBasedSwitching ) { - RwMem ((Bar5 + 0x0F8), AccessWidth32, 0x00FFFFFF, 0x00); - } else { - RwMem ((Bar5 + 0x0F8), AccessWidth32, 0x00FFFFFF, 0x00); - } + // FchSataFisBasedSwitching + RwMem ((Bar5 + 0x0F8), AccessWidth32, 0x00FFFFFF, 0x00);

if ( LocalCfgPtr->Sata.BiosOsHandOff == 1 ) { RwMem ((Bar5 + 0x24), AccessWidth8, (UINT32)~BIT0, BIT0);

Attachments:

attachment.htm (text/html — 4.0 KB)

Show replies by date

Paul Menzel (Code Review)

14 Oct 14 Oct

4:56 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36001 )

Change subject: vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36001/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36001/1//COMMIT_MSG@12 PS1, Line 12: If it’s found by Coverity, please add the appropriate tag line.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 1 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Mon, 14 Oct 2019 14:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Name of user not set (Code Review)

5:22 p.m.

Hello Jacob Garber, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36001

to look at the new patch set (#2).

Change subject: vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code ......................................................................

vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code

From looking at neighboring code, disabling FIS based switching appears intentional, possibly due to errata. Clean and comment code so it is apparent.

Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Signed-off-by: Joe Moore awokd@danwin1210.me Found-by: Coverity CID 1241913 --- M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c 1 file changed, 4 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/36001/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 2 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

22 Oct 22 Oct

2:53 p.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36001 )

Change subject: vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze: Remove redundant FIS code ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36001/2/src/vendorcode/amd/agesa/f1... File src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c:

https://review.coreboot.org/c/coreboot/+/36001/2/src/vendorcode/amd/agesa/f1... PS2, Line 370: // FchSataFisBasedSwitching If I'd read the comment without seeing this commit, I'd think that the line below is because that flag is always set. From this change it seems that you always took the "else" path. Could you make the intent of the comment clearer here?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 2 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 22 Oct 2019 12:53:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Name of user not set (Code Review)

9:26 p.m.

New subject: Change in coreboot[master]: vc/amd/agesa/f16kb: Remove redundant FIS code

Hello Jacob Garber, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36001

to look at the new patch set (#3).

Change subject: vc/amd/agesa/f16kb: Remove redundant FIS code ......................................................................

vc/amd/agesa/f16kb: Remove redundant FIS code

From looking at neighboring code, disabling FIS based switching appears intentional, possibly due to errata. Clean and comment code so it is apparent.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/36001/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 3 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Name of user not set (Code Review)

23 Oct 23 Oct

1:42 p.m.

New subject: Change in coreboot[master]: vc/amd/agesa/f16kb: Remove redundant FIS code

Hello Jacob Garber, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36001

to look at the new patch set (#4).

Change subject: vc/amd/agesa/f16kb: Remove redundant FIS code ......................................................................

vc/amd/agesa/f16kb: Remove redundant FIS code

From looking at neighboring code, disabling FIS based switching appears intentional, possibly due to errata. Clean and comment code so it is apparent.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/36001/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 4 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: newpatchset

Name of user not set (Code Review)

1:47 p.m.

New subject: Change in coreboot[master]: vc/amd/agesa/f16kb: Remove redundant FIS code

awokd@danwin1210.me has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36001 )

Change subject: vc/amd/agesa/f16kb: Remove redundant FIS code ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36001/2/src/vendorcode/amd/agesa/f1... File src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c:

https://review.coreboot.org/c/coreboot/+/36001/2/src/vendorcode/amd/agesa/f1... PS2, Line 370: // FchSataFisBasedSwitching

...

If I'd read the comment without seeing this commit, I'd think that the line below is because that fl […]

The original vendorcode is the same either path, unlike the other capabilities. If I'm following the bit flips correctly (not a given), they're disabling it even if the device reports it is capable.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 4 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Wed, 23 Oct 2019 11:47:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Name of user not set (Code Review)

11 Jan 11 Jan

1:29 p.m.

New subject: Change in coreboot[master]: vc/amd/agesa/f16kb: Remove redundant FIS code

awokd@danwin1210.me has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36001 )

Change subject: vc/amd/agesa/f16kb: Remove redundant FIS code ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36001/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36001/1//COMMIT_MSG@12 PS1, Line 12:

...

If it’s found by Coverity, please add the appropriate tag line.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 5 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sat, 11 Jan 2020 12:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Martin L Roth (Code Review)

7 Aug 7 Aug

10:42 p.m.

New subject: [S] Change in coreboot[master]: vc/amd/agesa/f16kb: Remove redundant FIS code

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36001?usp=email )

Change subject: vc/amd/agesa/f16kb: Remove redundant FIS code ......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36001?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4529716e951a9e7b63bdee11611dd78f2d41b464 Gerrit-Change-Number: 36001 Gerrit-PatchSet: 5 Gerrit-Owner: awokd@danwin1210.me Gerrit-Reviewer: Jacob Garber jgarber1@ualberta.ca Gerrit-Reviewer: awokd@danwin1210.me Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Martin L Roth gaumless@gmail.com Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-MessageType: abandon

474

days inactive

1868

days old

coreboot-gerrit@coreboot.org

8 comments

4 participants

tags (0)

participants (4)

Martin L Roth (Code Review)
Name of user not set (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)