Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
soc/amd/common: Remove Picasso display HDA from list
Change-Id: I306a806dc510e3a4ee3d9c0663306dc93b1d936d Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/hda/hda.c 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/35272/1
diff --git a/src/soc/amd/common/block/hda/hda.c b/src/soc/amd/common/block/hda/hda.c index 49fc1a6..df3dc5e 100644 --- a/src/soc/amd/common/block/hda/hda.c +++ b/src/soc/amd/common/block/hda/hda.c @@ -21,7 +21,6 @@ static const unsigned short pci_device_ids[] = { PCI_DEVICE_ID_AMD_SB900_HDA, PCI_DEVICE_ID_AMD_CZ_HDA, - PCI_DEVICE_ID_AMD_PCO_HDA0, PCI_DEVICE_ID_AMD_PCO_HDA1, 0 };
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
Patch Set 1:
Maybe say why it's being removed?
Hello build bot (Jenkins), Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35272
to look at the new patch set (#2).
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
soc/amd/common: Remove Picasso display HDA from list
The PCO_HDA0 device contains the "ATI" vendor ID 0x1002 and was incorrectly added to this file. It isn't anticipated that the device will need special handling, so remove it from the list of supported IDs.
Change-Id: I306a806dc510e3a4ee3d9c0663306dc93b1d936d Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/hda/hda.c 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/35272/2
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... File src/soc/amd/common/block/hda/hda.c:
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... PS2, Line 24: PCI_DEVICE_ID_AMD_PCO_HDA1 If there's only one now, does it needs a number at the end? Should it not be simply PCI_DEVICE_ID_AMD_PCO_HDA?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... File src/soc/amd/common/block/hda/hda.c:
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... PS2, Line 24: PCI_DEVICE_ID_AMD_PCO_HDA1
If there's only one now, does it needs a number at the end? Should it not be simply PCI_DEVICE_ID_AM […]
I could go either way but I lean toward keeping it this way. The PPR lists two and we need to make sure we have the correct one here. Both are listed in pci_ids.h. If anything, that file is possibly misleading now, but it seems way worse to rename AMD_PCO_HDA0 TO ATI_PCO_HDA (or 0).
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... File src/soc/amd/common/block/hda/hda.c:
https://review.coreboot.org/c/coreboot/+/35272/2/src/soc/amd/common/block/hd... PS2, Line 24: PCI_DEVICE_ID_AMD_PCO_HDA1
I could go either way but I lean toward keeping it this way. […]
Ack
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35272 )
Change subject: soc/amd/common: Remove Picasso display HDA from list ......................................................................
soc/amd/common: Remove Picasso display HDA from list
The PCO_HDA0 device contains the "ATI" vendor ID 0x1002 and was incorrectly added to this file. It isn't anticipated that the device will need special handling, so remove it from the list of supported IDs.
Change-Id: I306a806dc510e3a4ee3d9c0663306dc93b1d936d Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35272 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/soc/amd/common/block/hda/hda.c 1 file changed, 0 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Richard Spiegel: Looks good to me, approved
diff --git a/src/soc/amd/common/block/hda/hda.c b/src/soc/amd/common/block/hda/hda.c index 49fc1a6..df3dc5e 100644 --- a/src/soc/amd/common/block/hda/hda.c +++ b/src/soc/amd/common/block/hda/hda.c @@ -21,7 +21,6 @@ static const unsigned short pci_device_ids[] = { PCI_DEVICE_ID_AMD_SB900_HDA, PCI_DEVICE_ID_AMD_CZ_HDA, - PCI_DEVICE_ID_AMD_PCO_HDA0, PCI_DEVICE_ID_AMD_PCO_HDA1, 0 };
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)