Change in flashrom[master]: nicintel: add I350 support
Martijn Berger has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... nicintel: add I350 support While trying to flash I350 Intel NIC's on arm64 linux I had to change some things to make it work well Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M nicintel_eeprom.c M nicintel_spi.c 2 files changed, 39 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/39963/1 diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 4d45f79..be03e41 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -27,6 +27,9 @@ * 8.4.3: EEPROM-Mode Read Register * 8.4.6: EEPROM-Mode Write Register * Write process inspired on kernel e1000_i210.c + * + * Intel I350 Gigabit controller Datasheet + * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/ethe... */ #include <stdlib.h> @@ -75,8 +78,6 @@ static struct pci_dev *nicintel_pci; static bool done_i20_write = false; -#define UNPROG_DEVICE 0x1509 - /* * Warning: is_i210() below makes assumptions on these PCI ids. * It may have to be updated when this list is extended. @@ -87,7 +88,7 @@ {PCI_VENDOR_ID_INTEL, 0x1510, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Backplane)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Ext. PHY)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Dual Gigabit Ethernet Controller (Copper)"}, - {PCI_VENDOR_ID_INTEL, UNPROG_DEVICE, OK, "Intel", "Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller"}, + {PCI_VENDOR_ID_INTEL, 0x1509, OK, "Intel", "Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller"}, {PCI_VENDOR_ID_INTEL, 0x1531, NT, "Intel", "I210 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1532, NT, "Intel", "I211 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1533, OK, "Intel", "I210 Gigabit Network Connection"}, @@ -95,9 +96,26 @@ {PCI_VENDOR_ID_INTEL, 0x1537, NT, "Intel", "I210 Gigabit Network Connection SERDES Backplane"}, {PCI_VENDOR_ID_INTEL, 0x1538, NT, "Intel", "I210 Gigabit Network Connection SGMII"}, {PCI_VENDOR_ID_INTEL, 0x1539, NT, "Intel", "I211 Gigabit Network Connection"}, + {PCI_VENDOR_ID_INTEL, 0x151f, OK, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, + {PCI_VENDOR_ID_INTEL, 0x1521, OK, "Intel", "I350 Gigabit Network Connection Copper"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, + {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, {0}, }; +static inline bool is_unprog_device(uint16_t device_id) +{ + switch (device_id) + { + case 0x1509: // Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller + case 0x151f: // I350 Gigabit Network Connection Unprogrammed + return true; + default: + return false; + } +} + static inline bool is_i210(uint16_t device_id) { return (device_id & 0xfff0) == 0x1530; @@ -118,7 +136,7 @@ static int nicintel_ee_probe_82580(struct flashctx *flash) { - if (nicintel_pci->device_id == UNPROG_DEVICE) + if (is_unprog_device(nicintel_pci->device_id)) flash->chip->total_size = 16; /* Fall back to minimum supported size. */ else { uint32_t tmp = pci_mmio_readl(nicintel_eebar + EEC); @@ -456,7 +474,7 @@ if (!dev) return 1; - uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); if (!io_base_addr) return 1; @@ -466,7 +484,7 @@ return 1; nicintel_pci = dev; - if ((dev->device_id != UNPROG_DEVICE)) { + if (!is_unprog_device(dev->device_id)) { uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); /* C.f. 3.3.1.5 for the detection mechanism (maybe? contradicting diff --git a/nicintel_spi.c b/nicintel_spi.c index 1173ef7..09a37dc 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -28,6 +28,9 @@ * * Intel 82599 10 GbE Controller Datasheet (331520) * http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82599... + * + * Intel I350 Gigabit controller Datasheet + * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/ethe... */ #include <stdlib.h> @@ -104,6 +107,11 @@ {PCI_VENDOR_ID_INTEL, 0x1538, NT, "Intel", "I210 Gigabit Network Connection SGMII"}, {PCI_VENDOR_ID_INTEL, 0x1539, NT, "Intel", "I211 Gigabit Network Connection"}, + {PCI_VENDOR_ID_INTEL, 0x151f, NT, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, + {PCI_VENDOR_ID_INTEL, 0x1521, NT, "Intel", "I350 Gigabit Network Connection Copper"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, + {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, {0}, }; @@ -252,7 +260,7 @@ if (!dev) return 1; - uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); if (!io_base_addr) return 1; @@ -266,6 +274,12 @@ MEMMAP_SIZE); if (!nicintel_spibar || nicintel_spi_82599_enable_flash()) return 1; + } else if ((dev->device_id & 0xfff0) == 0x1510 || + (dev->device_id & 0xfff0) == 0x1520) { + nicintel_spibar = rphysmap("Intel I350 Gigabit w/ SPI flash", io_base_addr, + MEMMAP_SIZE); + if (!nicintel_spibar || nicintel_spi_i210_enable_flash()) + return 1; } else { nicintel_spibar = rphysmap("Intel 10 Gigabit NIC w/ SPI flash", io_base_addr + 0x10000, MEMMAP_SIZE); -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-MessageType: newchange
Peter Lemenkov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@103 PS1, Line 103: {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, 0x1522? Should be 0x1524? -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Tue, 31 Mar 2020 12:30:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@99 PS1, Line 99: Unprogrammed Please put this at the beginning of the string https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@109 PS1, Line 109: switch (device_id) Why use a switch? This could be simply: return device_id == 0x1509 || device_id == 0x151f; https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@477 PS1, Line 477: uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); This change is probably needed because most PCI devices on x86_64 use 32-bit BARs https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c@277 PS1, Line 277: (dev->device_id & 0xfff0) == 0x1510 This check will be a false positive for 0x1517 and 0x151c -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Tue, 31 Mar 2020 13:56:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Martijn Berger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@103 PS1, Line 103: {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
0x1522? Should be 0x1524? yes good catch
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@477 PS1, Line 477: uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
This change is probably needed because most PCI devices on x86_64 use 32-bit BARs it is needed as my platform is arm64 and the BAR0 has an address above the 32 bit boundary even if the pci device might not see it that way. Maybe a fixed size 64 bit would be more appropriate ?
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Wed, 01 Apr 2020 08:41:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Lemenkov <lemenkov@gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@477 PS1, Line 477: uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
it is needed as my platform is arm64 and the BAR0 has an address above the 32 bit boundary even if t […] Well, there may be many more instances of uint32_t to store addresses, so it would be a good idea to take care of them all at once
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Wed, 01 Apr 2020 08:43:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martijn Berger Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/39963 to look at the new patch set (#3). Change subject: nicintel: add I350 support ...................................................................... nicintel: add I350 support While trying to flash I350 Intel NIC's on arm64 linux I had to change some things to make it work well Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M nicintel_eeprom.c M nicintel_spi.c 2 files changed, 32 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/39963/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 3 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/39963 to look at the new patch set (#4). Change subject: nicintel: add I350 support ...................................................................... nicintel: add I350 support While trying to flash I350 Intel NIC's on arm64 linux I had to change some things to make it work well Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M nicintel_eeprom.c M nicintel_spi.c 2 files changed, 32 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/39963/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 4 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/39963 to look at the new patch set (#5). Change subject: nicintel: add I350 support ...................................................................... nicintel: add I350 support While trying to flash I350 Intel NIC's on arm64 linux I had to change some things to make it work well Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Signed-off-by: Martijn Berger <martijn.berger@gmail.com> --- M nicintel_eeprom.c M nicintel_spi.c 2 files changed, 32 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/39963/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 5 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-MessageType: newpatchset
Peter Lemenkov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@99 PS1, Line 99: Unprogrammed
Please put this at the beginning of the string Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 5 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Thu, 02 Apr 2020 13:39:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Martijn Berger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 5: (3 comments) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@103 PS1, Line 103: {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
yes good catch Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@109 PS1, Line 109: switch (device_id)
Why use a switch? This could be simply: […] Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c@277 PS1, Line 277: (dev->device_id & 0xfff0) == 0x1510
This check will be a false positive for 0x1517 and 0x151c Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 5 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Fri, 03 Apr 2020 08:53:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Lemenkov <lemenkov@gmail.com> Comment-In-Reply-To: Martijn Berger Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Martijn Berger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 5: (4 comments) https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@103 PS1, Line 103: {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
Done Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@109 PS1, Line 109: switch (device_id)
Done Done
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_eeprom.c@477 PS1, Line 477: uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
Well, there may be many more instances of uint32_t to store addresses, so it would be a good idea to […] Ack
https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/1/nicintel_spi.c@277 PS1, Line 277: (dev->device_id & 0xfff0) == 0x1510
Done Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 5 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Sat, 04 Apr 2020 13:22:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Lemenkov <lemenkov@gmail.com> Comment-In-Reply-To: Martijn Berger Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 6: Code-Review+1 (2 comments) https://review.coreboot.org/c/flashrom/+/39963/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/39963/6//COMMIT_MSG@10 PS6, Line 10: some things Mention it was necessary to change the type of the base address variables? https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c@110 PS6, Line 110: {PCI_VENDOR_ID_INTEL, 0x151f, NT, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, : {PCI_VENDOR_ID_INTEL, 0x1521, NT, "Intel", "I350 Gigabit Network Connection Copper"}, : {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, : {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, : {PCI_VENDOR_ID_INTEL, 0x1524, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, None of these are tested? -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 6 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Tue, 14 Jul 2020 08:33:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Martijn Berger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 6: (2 comments) I will augment the commit message https://review.coreboot.org/c/flashrom/+/39963/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/39963/6//COMMIT_MSG@10 PS6, Line 10: some things
Mention it was necessary to change the type of the base address variables? Ack
https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c@110 PS6, Line 110: {PCI_VENDOR_ID_INTEL, 0x151f, NT, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, : {PCI_VENDOR_ID_INTEL, 0x1521, NT, "Intel", "I350 Gigabit Network Connection Copper"}, : {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, : {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, : {PCI_VENDOR_ID_INTEL, 0x1524, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
None of these are tested? Yes, we abandoned the SPI for our product and I do not have the time to add the chips back to my Intel produced PCIe I350 that I used as a reference.
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 6 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Mon, 20 Jul 2020 09:59:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c File nicintel_spi.c: https://review.coreboot.org/c/flashrom/+/39963/6/nicintel_spi.c@110 PS6, Line 110: {PCI_VENDOR_ID_INTEL, 0x151f, NT, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, : {PCI_VENDOR_ID_INTEL, 0x1521, NT, "Intel", "I350 Gigabit Network Connection Copper"}, : {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, : {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, : {PCI_VENDOR_ID_INTEL, 0x1524, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"},
Yes, we abandoned the SPI for our product and I do not have the time to add the chips back to my Int […] Ack
-- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 6 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-Comment-Date: Mon, 20 Jul 2020 10:02:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martijn Berger Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/39963?usp=email ) Change subject: nicintel: add I350 support ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/39963?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 6 Gerrit-Owner: Martijn Berger Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-CC: Peter Lemenkov <lemenkov@gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org>
participants (4)
-
Anastasia Klimchuk (Code Review) -
Angel Pons (Code Review)
-
Martijn Berger (Code Review)
-
Peter Lemenkov (Code Review)