flashrom July 2013

flashrom@flashrom.org

31 participants
142 discussions

WG: SB600, shutting down IMC failed
by Trunzer Markus 26 Jul '13

26 Jul '13

>Can you tell us more about your system's firmware? =>What kind of information do you mean concretely? >Should the IMC be active and responsive? =>I'm not sure what the consequence is in detail when the IMC is inactive. But I gues it doesn't need to be active. >Rudolf: Let's assume the IMC is set up but does not respond in time (btw I guess a longer timeout won't help? =>Can you please tell me where I can increase this time? Best regards Markus On Thu, 11 Jul 2013 13:50 >Hello together, >I'm very sorry...I didn't verified the output at the display and the logfile content. Yes, your absolutely right, the message "IMC MBOX: Timeout!" is printed after the line "EC MBOX is at 0x3e". >And at the end there are 3 more lines: > >"FATAL ERROR! >Unhandled programmer parameters: amd_imc_force=yes >Error: Programmer initialization failed." > >Best regards >Markus_T On Thu, 11 Jul 2013 12:24:47 +0200 > Hello again, > I used this source code > http://download.flashrom.org/snapshots/flashrom-0.9.6.1-r1643.tar.bz2 and patched it with patch 3894 "patch<AMD---SP5100---take-SPI-ownership-1-2.patch". The patching and compiling was without errors. > > The execution of flashrom with "flashrom -VVV -p internal:amd_imc_force=yes" resulted in following error message. > > > flashrom v0.9.6.1-r1643 on Linux 3.0.1pae (i686) flashrom is free > software, get the source code at http://www.flashrom.org > > flashrom was built with libpci 3.1.7, GCC 4.4.5, little endian Command > line (3 args): flashrom -VVV -p internal:amd_imc_force=yes Calibrating > delay loop... OS timer resolution is 1 usecs, 824M loops per second, 10 myus = 11 us, 100 myus = 100 us, 1000 myus = 1000 us, 10000 myus = 10008 us, 4 myus = 4 us, OK. > Initializing internal programmer > No coreboot table found. > DMI string system-manufacturer: "To be filled by O.E.M." > DMI string system-product-name: "To be filled by O.E.M." > DMI string system-version: "To be filled by O.E.M." > DMI string baseboard-manufacturer: "abc" > DMI string baseboard-product-name: "123" > DMI string baseboard-version: "1.1.0" > DMI string chassis-type: "Desktop" > Found chipset "AMD SB7x0/SB8x0/SB9x0" with PCI ID 1002:439d. Enabling flash write... > EC SIO is at 0x6e > EC MBOX is at 0x3e > (Shutting down IMC failed!) > FAILED! > > > Do you have an idea what the problem could be? > I have no idea what the underlying problem is, but can it be that you did only paste the output to stdout and not to stderr (which can happen if you redirect the output of flashrom with ">/some/logfile" instead of using the -o option or directing with ">/some/logfile 2>&1")? I am asking because I am pretty sure that there is "IMC MBOX: Timeout!" in the original output (to stderr). And in that case it is clearly not an error introduced by me refinement of the patch, and I would like to continue/test/fix my current version (3979) on top of current HEAD (or at least r1680) instead of the original one, because we want to merge the new version eventually of course. Can you tell us more about your system's firmware? Should the IMC be active and responsive? Rudolf: Let's assume the IMC is set up but does not respond in time (btw I guess a longer timeout won't help? how did you choose the timeout value?), should we bail out like we do or presume that it is ok to write to flash? I guess it could make quite a lot of sense to be able to rewrite the IMC image in case the current image is broken (and hence does not respond to shutdown requests :) but this hopefully wont happen on end user systems. Also, note my comments in the refinement patch about IMCEnable and EcEnable. Do you have any opinion/knowledge about that? -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

3 5

Re: [flashrom] [PATCH] Add support for Loongson-3A.
by Stefan Tauner 26 Jul '13

26 Jul '13

On Fri, 26 Jul 2013 12:47:39 -0700 David Hendricks <dhendrix(a)google.com> wrote: > Hmmm, that return statement seems to be getting rather unwieldy especially > since we can probably expect a lot more variants. Maybe you can take this > opportunity to re-factor the code a bit, make a table of known strings, and > use a loop instead? You could also annotate the table entries to indicate > whether the processor is known-working and print a warning if not. > > Alternately, perhaps it would be sufficient to use strstr() to find > "GodsonV2", "Loongson-2", and "Loongson-3"? Yes it's awful... but I want to know if it works first and even then I think I don't care enough ATM. I had enough string handling in C this week already :) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

Re: [flashrom] [PATCH] Add support for Loongson-3A.
by David Hendricks 26 Jul '13

26 Jul '13

Hmmm, that return statement seems to be getting rather unwieldy especially since we can probably expect a lot more variants. Maybe you can take this opportunity to re-factor the code a bit, make a table of known strings, and use a loop instead? You could also annotate the table entries to indicate whether the processor is known-working and print a warning if not. Alternately, perhaps it would be sufficient to use strstr() to find "GodsonV2", "Loongson-2", and "Loongson-3"? On Thu, Jul 25, 2013 at 8:33 AM, Stefan Tauner < stefan.tauner(a)student.tuwien.ac.at> wrote: > Also be less restrictive regarding Loongson 2, because these strings seem > to be rather volatile: > http://www.linux-mips.org/archives/linux-mips/2013-04/msg00179.html > > http://www.loongson.cn/dev/gitweb/?p=linux-loongson-all;a=history;f=arch/mi… > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > --- > > Let's pretend I know what I am doing here... if I read the chinese > data sheets correctly then Loongson-3B has moved to use SPI primarily > for its firmware but there is always a chipset attached too... > In any case I would rather not touch it yet. 3A OTOH seems to be > more similar to Loongson-2 so this might actually work. > > This patch applies w/o conflicts to r1565 and later. > > processor_enable.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/processor_enable.c b/processor_enable.c > index 1361dd5..92c6b18 100644 > --- a/processor_enable.c > +++ b/processor_enable.c > @@ -58,8 +58,9 @@ static int is_loongson(void) > while (*ptr && isspace((unsigned char)*ptr)) > ptr++; > fclose(cpuinfo); > - return (strncmp(ptr, "ICT Loongson-2 V0.3", strlen("ICT > Loongson-2 V0.3")) == 0) || > - (strncmp(ptr, "Godson2 V0.3 FPU V0.1", > strlen("Godson2 V0.3 FPU V0.1")) == 0); > + return (strncmp(ptr, "ICT Loongson-2", strlen("ICT > Loongson-2")) == 0) || > + (strncmp(ptr, "Godson2 V0.3 FPU V0.1", > strlen("Godson2 V0.3 FPU V0.1")) == 0) || > + (strncmp(ptr, "ICT Loongson-3A", strlen("ICT > Loongson-3A")) == 0); > } > fclose(cpuinfo); > return 0; > -- > Kind regards, Stefan Tauner > > > _______________________________________________ > flashrom mailing list > flashrom(a)flashrom.org > http://www.flashrom.org/mailman/listinfo/flashrom > -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.

1 0

Re: [flashrom] A quick hack to support AMD Family 16h SOC
by Carl-Daniel Hailfinger 26 Jul '13

26 Jul '13

Hi Wei, thanks for the link. I didn't search for processor BKDG, only for chipset BKDG. That's why I missed the Family 16h BKDG which includes the SPI controller documentation. Am 26.07.2013 07:18 schrieb Wei Hu: > Have you checked the public BKDG > http://support.amd.com/us/Processor_TechDocs/48751_BKDG_Fam_16h_Mod_00h-0Fh… > It actually does talk about the SPI registers in Section 3.26.9.2. Thanks. > I'm not very familiar with AMD's code names, but I think Kabini's FCH > is called Yangtze? Hudson seems to be previous generations. Yes, I will have to fix this. > I'll give you modified patch a test tomorrow. Please note that I expect problems with my patch. Not all of the FIFO management has been changed, and this might be the cause of error messages. Updated patch with more debugging at the end of this mail. > On Thu, Jul 25, 2013 at 7:21 PM, Carl-Daniel Hailfinger > <c-d.hailfinger.devel.2006(a)gmx.net> wrote: >> Hello Wei Hu, >> >> thanks for your patch! >> >> Am 27.06.2013 02:34 schrieb Wei Hu: >>> In the past I posted a question at >>> http://www.flashrom.org/pipermail/flashrom/2013-April/010924.html >>> about failure to run flashrom on the new AMD SOC. It turns out the SPI >>> flash interface has introduced incompatible changes from SB600. I have >>> created a quick hack and made it available at >>> http://blogs.coreboot.org/blog/2013/06/26/gsoc-2013-flashrom-week-1-while1/. >>> Based on the discussion there I'm posting the patch here as well. Hope >>> it will be helpful. >>> >>> A quick hack to support AMD Family 16h SOC (Kabini). >>> This patch is just a proof of concept and incompatible with previous generations of AMD chipset. >>> Tested reading and writing on ASRock IMB-A180. >>> >>> Signed-off-by: Wei Hu <wei(a)aristanetworks.com> >> I have tried to change the patch in a way which keeps compatibility with >> older AMD chipsets. >> Please test with and without USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER >> #defined. If any errors crop up, please send them to the mailing list in >> reply to this mail. I still have to search/replace Hudson with Yangtze, but this patch will help a lot finding out more about the various FIFO related registers. Signed-off-by: Wei Hu <wei(a)aristanetworks.com> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-kabini/sb600spi.c =================================================================== --- flashrom-kabini/sb600spi.c (Revision 1704) +++ flashrom-kabini/sb600spi.c (Arbeitskopie) @@ -45,6 +45,8 @@ static uint8_t *sb600_spibar = NULL; +static int hudson_method = 0; + static void reset_internal_fifo_pointer(void) { mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2); @@ -58,14 +60,29 @@ { uint8_t tmp; + if (hudson_method) { + uint8_t tmp1; + mmio_writeb(7, sb600_spibar + 0x1e); + tmp = mmio_readb(sb600_spibar + 0x1f); + msg_pspew("Kabini SPIDataFifoPtr %d/%d, ", tmp, want); + tmp = mmio_readb(sb600_spibar + 0xd) & 0x07; + msg_pspew("FifoPtr %d/%d, ", tmp, want & 0x07); + tmp = mmio_readb(sb600_spibar + 0x4d) & 0x7f; + tmp1 = mmio_readb(sb600_spibar + 0x4e) & 0x7f; + msg_pspew("FifoWrPtr/FifoRdPtr %d/%d\n", tmp, tmp1); + } + +#ifdef USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER + mmio_writeb(7, sb600_spibar + 0x1e); + tmp = mmio_readb(sb600_spibar + 0x1f); +#else /* either way works */ tmp = mmio_readb(sb600_spibar + 0xd) & 0x07; want &= 0x7; +#endif if (want != tmp) { msg_perr("FIFO pointer corruption! Pointer is %d, wanted %d\n", tmp, want); - msg_perr("Something else is accessing the flash chip and " - "causes random corruption.\nPlease stop all " - "applications and drivers and IPMI which access the " - "flash chip.\n"); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash chip.\n"); return 1; } else { msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want); @@ -99,7 +116,7 @@ /* First byte is cmd which can not being sent through FIFO. */ unsigned char cmd = *writearr++; unsigned int readoffby1; - unsigned char readwrite; + unsigned char readwrite = 0; writecnt--; @@ -118,15 +135,21 @@ return SPI_INVALID_LENGTH; } - /* This is a workaround for a bug in SB600 and SB700. If we only send - * an opcode and no additional data/address, the SPI controller will - * read one byte too few from the chip. Basically, the last byte of - * the chip response is discarded and will not end up in the FIFO. - * It is unclear if the CS# line is set high too early as well. - */ - readoffby1 = (writecnt) ? 0 : 1; - readwrite = (readcnt + readoffby1) << 4 | (writecnt); - mmio_writeb(readwrite, sb600_spibar + 1); + if (hudson_method) { + /* Use the extended TxByteCount and RxByteCount register. */ + mmio_writeb(writecnt, sb600_spibar + 0x48); + mmio_writeb(readcnt, sb600_spibar + 0x4b); + } else { + /* This is a workaround for a bug in SB600 and SB700. If we only send + * an opcode and no additional data/address, the SPI controller will + * read one byte too few from the chip. Basically, the last byte of + * the chip response is discarded and will not end up in the FIFO. + * It is unclear if the CS# line is set high too early as well. + */ + readoffby1 = (writecnt) ? 0 : 1; + readwrite = (readcnt + readoffby1) << 4 | (writecnt); + mmio_writeb(readwrite, sb600_spibar + 1); + } mmio_writeb(cmd, sb600_spibar + 0); /* Before we use the FIFO, reset it first. */ @@ -171,27 +194,58 @@ msg_pspew("[%02x]", cmd); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt)) - return SPI_PROGRAMMER_ERROR; + /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Hudson, but it does so on all + * chipsets before Hudson. + */ + if (hudson_method) { + if (compare_internal_fifo_pointer(0)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_fifo_pointer(writecnt)) + return SPI_PROGRAMMER_ERROR; + } + msg_pspew("Reading: "); for (count = 0; count < readcnt; count++, readarr++) { *readarr = mmio_readb(sb600_spibar + 0xC); msg_pspew("[%02x]", *readarr); } msg_pspew("\n"); - if (reset_compare_internal_fifo_pointer(readcnt + writecnt)) - return SPI_PROGRAMMER_ERROR; - if (mmio_readb(sb600_spibar + 1) != readwrite) { - msg_perr("Unexpected change in SB600 read/write count!\n"); - msg_perr("Something else is accessing the flash chip and " - "causes random corruption.\nPlease stop all " - "applications and drivers and IPMI which access the " - "flash chip.\n"); - return SPI_PROGRAMMER_ERROR; + /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Hudson, but it does so on all + * chipsets before Hudson. + */ + if (hudson_method) { + if (compare_internal_fifo_pointer(0)) + return SPI_PROGRAMMER_ERROR; + } else { + if (reset_compare_internal_fifo_pointer(readcnt + writecnt)) + return SPI_PROGRAMMER_ERROR; } + if (hudson_method) { + uint8_t tmp_wc = mmio_readb(sb600_spibar + 0x48); + uint8_t tmp_rc = mmio_readb(sb600_spibar + 0x4b); + if ((tmp_rc != readcnt) || (tmp_wc != writecnt)) { + msg_perr("Unexpected change in Hudson read/write count: %i/%i instead of %i/%i!\n", + tmp_rc, tmp_wc, readcnt, writecnt); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash " + "chip.\n"); + return SPI_PROGRAMMER_ERROR; + } + } else { + uint8_t tmp_rw = mmio_readb(sb600_spibar + 1); + if (tmp_rw != readwrite) { + msg_perr("Unexpected change in SB600 read/write count: %i instead of %i!\n", tmp_rw, + readwrite); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash " + "chip.\n"); + return SPI_PROGRAMMER_ERROR; + } + } return 0; } @@ -310,6 +364,14 @@ tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]); + /* AMD Hudson apparently requires use of a different SPI registers for read/write counts. */ + if (dev->device_id == 0x438d) { + msg_pdbg("Using AMD Hudson SPI access method.\n"); + hudson_method = 1; + } else { + hudson_method = 0; + } + /* Look for the SMBus device. */ smbus_dev = pci_dev_find(0x1002, 0x4385); -- http://www.hailfinger.org/

3 5

Re: [flashrom] FCH code names (was: A quick hack to support AMD Family 16h SOC)
by Stefan Tauner 26 Jul '13

26 Jul '13

On Fri, 26 Jul 2013 11:39:19 -0700 Wei Hu <wei(a)aristanetworks.com> wrote: > Stefan, > > AMD internal documents also suggest A45/A50M/A55E FCH (Hudson-1) use > device ID 438D. Not sure if it's wrong since I don't have a test > system. I am. :) The ASRock E350M1 uses the A50M and has 439D (it is well supported by coreboot and it would be easy to find someone to test it): http://www.coreboot.org/pipermail/coreboot/2011-June/065421.html > According to AMD document A55/A60M/A68/A68M/A70M/A75/A85X use device > ID 780E. There's also this A77E FCH (codename Bolton) which also uses > 780E. OK, thanks > (Please cc me on your future emails since I'm not subscribed to the list) sorry! -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

Re: [flashrom] FCH code names (was: A quick hack to support AMD Family 16h SOC)
by Wei Hu 26 Jul '13

26 Jul '13

Stefan, AMD internal documents also suggest A45/A50M/A55E FCH (Hudson-1) use device ID 438D. Not sure if it's wrong since I don't have a test system. According to AMD document A55/A60M/A68/A68M/A70M/A75/A85X use device ID 780E. There's also this A77E FCH (codename Bolton) which also uses 780E. (Please cc me on your future emails since I'm not subscribed to the list) On Fri, Jul 26, 2013 at 10:29 AM, Wei Hu <wei(a)aristanetworks.com> wrote: > On Fri, Jul 26, 2013 at 6:12 AM, Stefan Tauner > <stefan.tauner(a)student.tuwien.ac.at> wrote: >> The code names are complicated. >> >> Jaguar describes the CPU(APU) architecture (succeeding Bobcat), while >> Kabini (embedded, htpc, laptops) and Temash (tablets) describe SoC >> families with those cores. >> >> Hudson seems to be the generic code name(!?) for all FCH aka >> southbridges since sb9xx which came to be when they started with their >> fusion aka APU development, hence there are multiple generations of >> Hudson (3 AFAIK, where Yangtze/Salton are #3). >> >> The integrated FCH for Kabini and Temash are codenamed "Yangtze" and >> "Salton" respectively. I am not aware of the previous code names... >> The only other FCH name i could find is Yuba but that was never >> released because it was cancelled together with Krishna/Wichita. > > Is it right to call out Hudson for incompatible SPI changes though? We > might need to call it hudson3 or something. What I seem to see in > coreboot is they try to bundle compatible changes into hudson and for > incompatible changes just call it yangtze.

1 0

Re: [flashrom] [PATCH] sbxxx: Handle active IMCs in AMD chipsets.
by Carl-Daniel Hailfinger 26 Jul '13

26 Jul '13

Hi Stefan, sorry for the late sending of this review (falling asleep a few times due to the intense heat didn't really help me get this sent). I believe the issues I raise are only cosmetic and the patch is technically solid. Am 18.07.2013 23:50 schrieb Stefan Tauner: > Detect and temporarily disable the IMC while accessing the flash. > Disable writes on default, but allow the user to enforce it. > > Signed-off-by: Rudolf Marek <r.marek(a)assembler.cz> > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Thanks a lot for the patch! I believe it will fix pretty much every case of IMC interference we saw. > diff --git a/amd_imc.c b/amd_imc.c > new file mode 100644 > index 0000000..b05390c > --- /dev/null > +++ b/amd_imc.c > @@ -0,0 +1,159 @@ > [...] > +static uint16_t get_sio_port(struct pci_dev *dev) > +{ > + uint16_t ec_port; > + > + if (!dev) { > + return 0; > + } > + > + ec_port = pci_read_word(dev, 0xa4); > + > + /* EcPortActive? */ > + if (!(ec_port & 0x1)) > + return 0; > + > + ec_port &= ~0x1; > + > + return ec_port; > +} > + > [...] > +static uint16_t mbox_get_port(uint16_t sio_port) > +{ > + uint16_t mbox_port; > + > + enter_conf_mode_ec(sio_port); > + > + /* Go to LDN 9, mailbox */ > + sio_write(sio_port, 7, 9); > + > + /* MBOX inactive? */ > + if ((sio_read(sio_port, 0x30) & 1) == 0) { > + exit_conf_mode_ec(sio_port); > + return 0; > + } > + > + mbox_port = sio_read(sio_port, 0x60) << 8; > + mbox_port |= sio_read(sio_port, 0x61); > + > + exit_conf_mode_ec(sio_port); > + return mbox_port; > +} > + > +/* Returns negative values when IMC is inactive, positive values on errors */ > +static int imc_send_cmd(struct pci_dev *dev, uint8_t cmd) > +{ > + uint16_t sio_port; > + uint16_t mbox_port; > + > + /* IntegratedEcPresent? */ > + if (!(pci_read_byte(dev, 0x40) & (1 << 7))) > + return -1; > + > + sio_port = get_sio_port(dev); > + if (!sio_port) > + return -1; > + > + msg_pdbg2("IMC SIO is at 0x%x.\n", sio_port); > + mbox_port = mbox_get_port(sio_port); > + if (!mbox_port) > + return -1; > + msg_pdbg2("IMC MBOX is at 0x%x.\n", mbox_port); > + > + sio_write(mbox_port, 0x82, 0x0); > + sio_write(mbox_port, 0x83, cmd); > + sio_write(mbox_port, 0x84, 0x0); > + /* trigger transfer 0x96 with subcommand cmd */ > + sio_write(mbox_port, 0x80, 0x96); > + > + return mbox_wait_ack(mbox_port); > +} > + > [...] > +int amd_imc_shutdown(struct pci_dev *dev) > +{ > + /* Try to put IMC to sleep */ > + int ret = imc_send_cmd(dev, 0xb4); > + > + /* No IMC activity detectable, assume we are fine */ > + if (ret < 0) { > + msg_pdbg2("No IMC found.\n"); The message above is printed after the message "Writes have been disabled for safety reasons because the IMC is active" from sb600_handle_imc(). AFAICS this mesage can only happen if the IMC is marked as present but any of the SIO port or MBOX port are zero, i.e. inaccessible. May I suggest the following message instead? "IMC inactive or unreachable.\n" > + return 0; > + } > + > + if (ret != 0) { > + msg_perr("Shutting down IMC failed.\n"); > + return ret; > + } > + msg_pdbg2("Shutting down IMC successful.\n"); > + > + if (register_shutdown(imc_resume, dev)) > + return 1; > + > + return ret; > +} > + > +#endif > diff --git a/programmer.h b/programmer.h > index db914cb..f03eac3 100644 > --- a/programmer.h > +++ b/programmer.h > @@ -572,6 +572,9 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, > enum ich_chipset ich_generation); > int via_init_spi(struct pci_dev *dev, uint32_t mmio_base); > > +/* imc.c */ Should be amd_imc.c > +int amd_imc_shutdown(struct pci_dev *dev); > + > /* it85spi.c */ > int it85xx_spi_init(struct superio s); > > diff --git a/sb600spi.c b/sb600spi.c > index a5c00d8..e76c04a 100644 > --- a/sb600spi.c > +++ b/sb600spi.c > @@ -194,6 +195,39 @@ static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, > return 0; > } > > +static int sb600_handle_imc(struct pci_dev *dev, bool amd_imc_force) > +{ > + /* Handle IMC everywhere but sb600 which does not have one. */ > + if (dev->device_id == 0x438d) > + return 0; > + > + /* TODO: we should not only look at IntegratedImcPresent (LPC Dev 20, Func 3, 40h) but also at > + * IMCEnable(Strap) and Override EcEnable(Strap) (sb8xx, sb9xx?, a50: Misc_Reg: 80h-87h; > + * sb7xx, sp5100: PM_Reg: B0h-B1h) etc. */ > + uint8_t reg = pci_read_byte(dev, 0x40); > + if ((reg & (1 << 7)) == 0) { > + msg_pdbg("IMC is not active.\n"); > + return 0; > + } This is the point where we assume the IMC is active. Given that we only check for the IntegratedImcPresent bit, would "IMC is not present" be a better message? Or do we want to keep the message and eventually look at the straps as well? > + > + if (!amd_imc_force) > + programmer_may_write = 0; > + msg_pinfo("Writes have been disabled for safety reasons because the IMC is active\n" It's present, but does it have to be active? > + "and it could interfere with accessing flash memory. Flashrom will try\n" > + "to disable it temporarily but even then this might not be safe:\n" > + "when it is reenabled and after a reboot it expects to find working code\n" > + "in the flash and it is unpredictable what happens if there is none.\n" > + "\n" > + "To be safe make sure that there is a working IMC firmware at the right\n" > + "location in the image you intend to write and do not attempt to erase.\n" > + "\n" > + "You can enforce write support with the amd_imc_force programmer option.\n"); > + if (amd_imc_force) > + msg_pinfo("Continuing with write support because the user forced us to!\n"); > + > + return amd_imc_shutdown(dev); Call to shutdown the IMC, done after printing that the IMC is active, but inside amd_imc_shutdown() we may print the message that no IMC was found. This is a bit contradictory in some cases. > +} > + > static const struct spi_programmer spi_programmer_sb600 = { > .type = SPI_CONTROLLER_SB600, > .max_data_read = 8, I just noticed the patch has been committed an hour ago, but throwing away the review seemed a waste. Regards, Carl-Daniel -- http://www.hailfinger.org/

2 1

Re: [flashrom] A quick hack to support AMD Family 16h SOC
by Carl-Daniel Hailfinger 25 Jul '13

25 Jul '13

Hello Wei Hu, thanks for your patch! Am 27.06.2013 02:34 schrieb Wei Hu: > In the past I posted a question at > http://www.flashrom.org/pipermail/flashrom/2013-April/010924.html > about failure to run flashrom on the new AMD SOC. It turns out the SPI > flash interface has introduced incompatible changes from SB600. I have > created a quick hack and made it available at > http://blogs.coreboot.org/blog/2013/06/26/gsoc-2013-flashrom-week-1-while1/. > Based on the discussion there I'm posting the patch here as well. Hope > it will be helpful. > > A quick hack to support AMD Family 16h SOC (Kabini). > This patch is just a proof of concept and incompatible with previous generations of AMD chipset. > Tested reading and writing on ASRock IMB-A180. > > Signed-off-by: Wei Hu <wei(a)aristanetworks.com> I have tried to change the patch in a way which keeps compatibility with older AMD chipsets. Please test with and without USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER #defined. If any errors crop up, please send them to the mailing list in reply to this mail. The biggest problem was the missing register reference guide (RRG) for AMD FCH. I had to guess the way to identify affected chipsets, and I now test for PCI device ID 0x438d (Hudson FCH, probably too broad match). I really hope this is the right ID. It would be great if you could confirm that the FIFO pointer management change affects all FCH with this PCI device ID. Is there a way to get access to the RRG for Kabini or the integrated FCH? Right now my changes are guesses Signed-off-by: Wei Hu <wei(a)aristanetworks.com> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-kabini/sb600spi.c =================================================================== --- flashrom-kabini/sb600spi.c (Revision 1704) +++ flashrom-kabini/sb600spi.c (Arbeitskopie) @@ -45,6 +45,8 @@ static uint8_t *sb600_spibar = NULL; +static int hudson_method = 0; + static void reset_internal_fifo_pointer(void) { mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2); @@ -58,14 +60,17 @@ { uint8_t tmp; +#ifdef USE_THE_EXTENDED_SPIDataFifoPtr_REGISTER + mmio_writeb(7, sb600_spibar + 0x1e); + tmp = mmio_readb(sb600_spibar + 0x1f); +#else /* either way works */ tmp = mmio_readb(sb600_spibar + 0xd) & 0x07; want &= 0x7; +#endif if (want != tmp) { msg_perr("FIFO pointer corruption! Pointer is %d, wanted %d\n", tmp, want); - msg_perr("Something else is accessing the flash chip and " - "causes random corruption.\nPlease stop all " - "applications and drivers and IPMI which access the " - "flash chip.\n"); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash chip.\n"); return 1; } else { msg_pspew("SB600 FIFO pointer is %d, wanted %d\n", tmp, want); @@ -99,7 +104,7 @@ /* First byte is cmd which can not being sent through FIFO. */ unsigned char cmd = *writearr++; unsigned int readoffby1; - unsigned char readwrite; + unsigned char readwrite = 0; writecnt--; @@ -118,15 +123,21 @@ return SPI_INVALID_LENGTH; } - /* This is a workaround for a bug in SB600 and SB700. If we only send - * an opcode and no additional data/address, the SPI controller will - * read one byte too few from the chip. Basically, the last byte of - * the chip response is discarded and will not end up in the FIFO. - * It is unclear if the CS# line is set high too early as well. - */ - readoffby1 = (writecnt) ? 0 : 1; - readwrite = (readcnt + readoffby1) << 4 | (writecnt); - mmio_writeb(readwrite, sb600_spibar + 1); + if (hudson_method) { + /* Use the extended TxByteCount and RxByteCount register. */ + mmio_writeb(writecnt, sb600_spibar + 0x48); + mmio_writeb(readcnt, sb600_spibar + 0x4b); + } else { + /* This is a workaround for a bug in SB600 and SB700. If we only send + * an opcode and no additional data/address, the SPI controller will + * read one byte too few from the chip. Basically, the last byte of + * the chip response is discarded and will not end up in the FIFO. + * It is unclear if the CS# line is set high too early as well. + */ + readoffby1 = (writecnt) ? 0 : 1; + readwrite = (readcnt + readoffby1) << 4 | (writecnt); + mmio_writeb(readwrite, sb600_spibar + 1); + } mmio_writeb(cmd, sb600_spibar + 0); /* Before we use the FIFO, reset it first. */ @@ -171,27 +182,58 @@ msg_pspew("[%02x]", cmd); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt)) - return SPI_PROGRAMMER_ERROR; + /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Hudson, but it does so on all + * chipsets before Hudson. + */ + if (hudson_method) { + if (compare_internal_fifo_pointer(0)) + return SPI_PROGRAMMER_ERROR; + } else { + if (compare_internal_fifo_pointer(writecnt)) + return SPI_PROGRAMMER_ERROR; + } + msg_pspew("Reading: "); for (count = 0; count < readcnt; count++, readarr++) { *readarr = mmio_readb(sb600_spibar + 0xC); msg_pspew("[%02x]", *readarr); } msg_pspew("\n"); - if (reset_compare_internal_fifo_pointer(readcnt + writecnt)) - return SPI_PROGRAMMER_ERROR; - if (mmio_readb(sb600_spibar + 1) != readwrite) { - msg_perr("Unexpected change in SB600 read/write count!\n"); - msg_perr("Something else is accessing the flash chip and " - "causes random corruption.\nPlease stop all " - "applications and drivers and IPMI which access the " - "flash chip.\n"); - return SPI_PROGRAMMER_ERROR; + /* Reading from the FIFO apparently doesn't advance the FIFO pointer on Hudson, but it does so on all + * chipsets before Hudson. + */ + if (hudson_method) { + if (compare_internal_fifo_pointer(0)) + return SPI_PROGRAMMER_ERROR; + } else { + if (reset_compare_internal_fifo_pointer(readcnt + writecnt)) + return SPI_PROGRAMMER_ERROR; } + if (hudson_method) { + uint8_t tmp_wc = mmio_readb(sb600_spibar + 0x48); + uint8_t tmp_rc = mmio_readb(sb600_spibar + 0x4b); + if ((tmp_rc != readcnt) || (tmp_wc != writecnt)) { + msg_perr("Unexpected change in Hudson read/write count: %i/%i instead of %i/%i!\n", + tmp_rc, tmp_wc, readcnt, writecnt); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash " + "chip.\n"); + return SPI_PROGRAMMER_ERROR; + } + } else { + uint8_t tmp_rw = mmio_readb(sb600_spibar + 1); + if (tmp_rw != readwrite) { + msg_perr("Unexpected change in SB600 read/write count: %i instead of %i!\n", tmp_rw, + readwrite); + msg_perr("Something else is accessing the flash chip and causes random corruption.\n" + "Please stop all applications and drivers and IPMI which access the flash " + "chip.\n"); + return SPI_PROGRAMMER_ERROR; + } + } return 0; } @@ -310,6 +352,14 @@ tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; msg_pdbg("NormSpeed is %s MHz\n", speed_names[tmp]); + /* AMD Hudson apparently requires use of a different SPI registers for read/write counts. */ + if (dev->device_id == 0x438d) { + msg_pdbg("Using AMD Hudson SPI access method.\n"); + hudson_method = 1; + } else { + hudson_method = 0; + } + /* Look for the SMBus device. */ smbus_dev = pci_dev_find(0x1002, 0x4385); -- http://www.hailfinger.org/

2 1

flashrom 0.9.7
by Carl-Daniel Hailfinger 25 Jul '13

25 Jul '13

First of all, a big thank you to Stefan Tauner for single-handedly getting all required patches for 0.9.7 in. While there was a discussion about serial port fixes over the last few days (mostly on IRC), I believe the risk/benefit balance of any serial port changes right now is definitely on the risk side and we should postpone that. The only critical thing that possibly remains would be the Kabini fix/workaround proposed by Wei Hu. With its #ifdefs it's not mergeable right now, but it should be possible to use if() instead if we're lucky enough to make that decision dependent on detecting Kabini. I was unable to find a relevant Register Reference Guide for the affected southbridge. I sent a modified patch for Kabini a few seconds ago in the hope it will help us fix/debug this issue further. Another option with probably less impact would be to refuse flash access on the affected chipset. I'm a bit torn here. Right now I hope my patch will either work or at least give us more insight on what to expect. Stefan, if you have any other patches you deem relevant for the release (e.g. board enables currently waiting for a test), please commit them once you think they are ready. I'd like to have some band-aid solution for the Kabini problem before it causes corruption somewhere, but if you want to release now (without any Kabini related patch), I agree. Stefan, if you make a release now, I'll sign it. If you decide to wait, OK. You are in charge of the release as agreed upon. If there are any problems with svn or the release process, holler. Regards, Carl-Daniel -- http://www.hailfinger.org/

1 0

Re: [flashrom] [PATCH 09/10] Rename Numonyx and ST (SGS/Thomson) chips to Micron.
by Carl-Daniel Hailfinger 25 Jul '13

25 Jul '13

Am 10.07.2013 21:17 schrieb Stefan Tauner: > Micron acquired Numonyx and asked us to change the vendor names to > "Micron". For the chips clearly emerging from the former > manufacturers we (will) use "Micron/Numonyx/ST" and the original > name for the rest. Resorting the chip entries makes the diff > unreadable, hence the stand-alone commit. > > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> That's a lot of churn and verifying it is difficult. Can we postpone this? Regards, Carl-Daniel -- http://www.hailfinger.org/

2 2

← Newer
1
2
3
4
5
6
7
8
...
15
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom July 2013