Change in coreboot[master]: i82371eb: Drop ops entry for i82371sb

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Keith Hui (Code Review)

27 Jan 2020 27 Jan '20

11:27 p.m.

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38598 )

Change subject: i82371eb: Drop ops entry for i82371sb ......................................................................

i82371eb: Drop ops entry for i82371sb

All boards with this chipset use i82371eb (that shares PCI ID with i82371ab).

Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/isa.c 1 file changed, 0 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/38598/1

diff --git a/src/southbridge/intel/i82371eb/isa.c b/src/southbridge/intel/i82371eb/isa.c index 00fbb77..1442470 100644 --- a/src/southbridge/intel/i82371eb/isa.c +++ b/src/southbridge/intel/i82371eb/isa.c @@ -158,9 +158,3 @@ .vendor = PCI_VENDOR_ID_INTEL, .device = PCI_DEVICE_ID_INTEL_82371AB_ISA, }; - -static const struct pci_driver isa_SB_driver __pci_driver = { - .ops = &isa_ops, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371SB_ISA, -};

Attachments:

attachment.htm (text/html — 2.6 KB)

Show replies by date

Paul Menzel (Code Review)

28 Jan 28 Jan

12:45 p.m.

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

Change subject: i82371eb: Drop ops entry for i82371sb ......................................................................

Patch Set 2: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG@10 PS2, Line 10: PCI ID with i82371ab). More words fit on the first line. (75 character limit)

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG@11 PS2, Line 11: Does it fix anything, or is it just cosmetical?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 28 Jan 2020 12:45:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

2 Feb 2 Feb

12:16 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38598 )

Change subject: i82371eb: Drop ops entry for i82371sb ......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG@11 PS2, Line 11:

...

Does it fix anything, or is it just cosmetical?

As I understand it, some unused code is dropped

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 02 Feb 2020 12:16:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Keith Hui (Code Review)

4:49 p.m.

Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38598 )

Change subject: i82371eb: Drop ops entry for i82371sb ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG@10 PS2, Line 10: PCI ID with i82371ab).

...

More words fit on the first line. […]

Done

https://review.coreboot.org/c/coreboot/+/38598/2//COMMIT_MSG@11 PS2, Line 11:

...

As I understand it, some unused code is dropped

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 02 Feb 2020 16:49:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Keith Hui (Code Review)

4:49 p.m.

New subject: Change in coreboot[master]: i82371eb: Drop support for older PIIX chips

Hello Patrick Rudolph, Angel Pons, Paul Menzel, build bot (Jenkins),

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

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

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

Change subject: i82371eb: Drop support for older PIIX chips ......................................................................

i82371eb: Drop support for older PIIX chips

All boards using this code use i82371eb (that shares PCI ID with i82371ab). Dropping the code lightens compressed ramstage by a few dozen bytes.

Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/i82371eb.c M src/southbridge/intel/i82371eb/ide.c M src/southbridge/intel/i82371eb/isa.c M src/southbridge/intel/i82371eb/usb.c 4 files changed, 2 insertions(+), 83 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/38598/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 3 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Patrick Georgi (Code Review)

3 Mar 3 Mar

7:42 a.m.

New subject: Change in coreboot[master]: i82371eb: Drop support for older PIIX chips

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

Change subject: i82371eb: Drop support for older PIIX chips ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 03 Mar 2020 07:42:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

7:42 a.m.

New subject: Change in coreboot[master]: i82371eb: Drop support for older PIIX chips

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38598 )

Change subject: i82371eb: Drop support for older PIIX chips ......................................................................

i82371eb: Drop support for older PIIX chips

All boards using this code use i82371eb (that shares PCI ID with i82371ab). Dropping the code lightens compressed ramstage by a few dozen bytes.

Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Signed-off-by: Keith Hui buurin@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38598 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/southbridge/intel/i82371eb/i82371eb.c M src/southbridge/intel/i82371eb/ide.c M src/southbridge/intel/i82371eb/isa.c M src/southbridge/intel/i82371eb/usb.c 4 files changed, 2 insertions(+), 83 deletions(-)

Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved

diff --git a/src/southbridge/intel/i82371eb/i82371eb.c b/src/southbridge/intel/i82371eb/i82371eb.c index 898cdff..02812ce 100644 --- a/src/southbridge/intel/i82371eb/i82371eb.c +++ b/src/southbridge/intel/i82371eb/i82371eb.c @@ -14,22 +14,9 @@ * GNU General Public License for more details. */

-/* Note: This code supports the 82371FB/SB/MX/AB/EB/MB and 82437MX. */ +/* Note: This code supports the 82371AB/EB/MB. */

/* Datasheets: - * - Name: 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR - * - URL: http://www.intel.com/design/intarch/datashts/290550.htm - * - PDF: http://download.intel.com/design/intarch/datashts/29055002.pdf - * - Date: April 1997 - * - Order Number: 290550-002 - * - * - Name: 82371FB (PIIX) and 82371SB (PIIX3) PCI ISA IDE Xcelerator - * Specification Update - * - URL: http://www.intel.com/design/chipsets/specupdt/297658.htm - * - PDF: http://download.intel.com/design/chipsets/specupdt/29765801.pdf - * - Date: March 1998 - * - Order Number: 297658-004 - * * - Name: 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4) * (applies to 82371AB/EB/MB, a.k.a. PIIX4/PIIX4E/PIIX4M) * - URL: http://www.intel.com/design/intarch/datashts/290562.htm @@ -44,10 +31,8 @@ * - Order Number: 297738-017 */

-/* TODO: List the other datasheets. */ - #include <device/device.h>

const struct chip_operations southbridge_intel_i82371eb_ops = { - CHIP_NAME("Intel 82371FB/SB/MX/AB/EB/MB Southbridge") + CHIP_NAME("Intel 82371AB/EB/MB Southbridge") }; diff --git a/src/southbridge/intel/i82371eb/ide.c b/src/southbridge/intel/i82371eb/ide.c index 7a72a65..1b8136a 100644 --- a/src/southbridge/intel/i82371eb/ide.c +++ b/src/southbridge/intel/i82371eb/ide.c @@ -119,18 +119,6 @@ }

/** - * IDE init for the Intel 82371FB/SB IDE controller. - * - * These devices do not support UDMA/33, so don't attempt to enable it. - * - * @param dev The device to use. - */ -static void ide_init_i82371fb_sb(struct device *dev) -{ - ide_init_enable(dev); -} - -/** * IDE init for the Intel 82371AB/EB/MB IDE controller. * * @param dev The device to use. @@ -141,17 +129,6 @@ ide_init_udma33(dev); }

-/* Intel 82371FB/SB */ -static const struct device_operations ide_ops_fb_sb = { - .read_resources = pci_dev_read_resources, - .set_resources = pci_dev_set_resources, - .enable_resources = pci_dev_enable_resources, - .init = ide_init_i82371fb_sb, - .scan_bus = 0, - .enable = 0, - .ops_pci = 0, /* No subsystem IDs on 82371XX! */ -}; - /* Intel 82371AB/EB/MB */ static const struct device_operations ide_ops_ab_eb_mb = { .read_resources = pci_dev_read_resources, @@ -163,34 +140,6 @@ .ops_pci = 0, /* No subsystem IDs on 82371XX! */ };

-/* Intel 82371FB (PIIX) */ -static const struct pci_driver ide_driver_fb __pci_driver = { - .ops = &ide_ops_fb_sb, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371FB_IDE, -}; - -/* Intel 82371SB (PIIX3) */ -static const struct pci_driver ide_driver_sb __pci_driver = { - .ops = &ide_ops_fb_sb, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371SB_IDE, -}; - -/* Intel 82371MX (MPIIX) */ -static const struct pci_driver ide_driver_mx __pci_driver = { - .ops = &ide_ops_fb_sb, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371MX_ISA_IDE, -}; - -/* Intel 82437MX (part of the 430MX chipset) */ -static const struct pci_driver ide_driver_82437mx __pci_driver = { - .ops = &ide_ops_fb_sb, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82437MX_ISA_IDE, -}; - /* Intel 82371AB/EB/MB */ static const struct pci_driver ide_driver_ab_eb_mb __pci_driver = { .ops = &ide_ops_ab_eb_mb, diff --git a/src/southbridge/intel/i82371eb/isa.c b/src/southbridge/intel/i82371eb/isa.c index a57d61a..fefead0 100644 --- a/src/southbridge/intel/i82371eb/isa.c +++ b/src/southbridge/intel/i82371eb/isa.c @@ -157,9 +157,3 @@ .vendor = PCI_VENDOR_ID_INTEL, .device = PCI_DEVICE_ID_INTEL_82371AB_ISA, }; - -static const struct pci_driver isa_SB_driver __pci_driver = { - .ops = &isa_ops, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371SB_ISA, -}; diff --git a/src/southbridge/intel/i82371eb/usb.c b/src/southbridge/intel/i82371eb/usb.c index 80b19a1..38ab167 100644 --- a/src/southbridge/intel/i82371eb/usb.c +++ b/src/southbridge/intel/i82371eb/usb.c @@ -43,15 +43,6 @@ .ops_pci = 0, /* No subsystem IDs on 82371EB! */ };

-/* Note: No USB on 82371FB/MX (PIIX/MPIIX) and 82437MX. */ - -/* Intel 82371SB (PIIX3) */ -static const struct pci_driver usb_driver_sb __pci_driver = { - .ops = &usb_ops, - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_82371SB_USB, -}; - /* Intel 82371AB/EB/MB (PIIX4/PIIX4E/PIIX4M) */ /* The 440MX (82443MX) consists of 82443BX + 82371EB (uses same PCI IDs). */ static const struct pci_driver usb_driver_ab_eb_mb __pci_driver = {

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 5 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

Paul Menzel (Code Review)

4:11 p.m.

New subject: Change in coreboot[master]: i82371eb: Drop support for older PIIX chips

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

Change subject: i82371eb: Drop support for older PIIX chips ......................................................................

Patch Set 5:

This breaks detecting the disk in QEMU i440fx. SeaBIOS is able to load GRUB from the MBR, but Linux 5.4.19 does not recognize the disk anymore. Reverting this commit makes it boot again.

Keith, Patrick, Angel, any suggestions?

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 5 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 03 Mar 2020 16:11:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Patrick Georgi (Code Review)

5:05 p.m.

New subject: Change in coreboot[master]: i82371eb: Drop support for older PIIX chips

Patrick Georgi has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/38598 )

Change subject: i82371eb: Drop support for older PIIX chips ......................................................................

-- To view, visit https://review.coreboot.org/c/coreboot/+/38598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab1e83b8f5fff44a33619c7925e5448169a2a87c Gerrit-Change-Number: 38598 Gerrit-PatchSet: 5 Gerrit-Owner: Keith Hui buurin@gmail.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Keith Hui buurin@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: revert

1537

days inactive

1573

days old

coreboot-gerrit@coreboot.org

8 comments

4 participants

tags (0)

participants (4)

Angel Pons (Code Review)
Keith Hui (Code Review)
Patrick Georgi (Code Review)
Paul Menzel (Code Review)