This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
--- Oops, if have forgotten to replace the patch file; here it is Fixed it to fulfil feedback of Uwe Hermann and Peter Stuge, related to coreboot coding conventions.
Mit freundlichen Grüßen / Best regards
Claus Gindhart SW R&D Kontron Modular Computers phone :++49 (0)8341-803-374 mailto:claus.gindhart@kontron.com http://www.kontron.com
Kontron Modular Computers GmbH Geschaeftsfuehrer / Managing Directors: Ulrich Gehrmann, Thomas Sabisch Sitz der Gesellschaft / Registered Office: Kaufbeuren, Rechtsform / Legal: GmbH Amtsgericht / Local District Court Kempten, HRB Nr. / Trade Register No. 6195
The information contained in this document is CONFIDENTIAL and property of Kontron. Any unauthorized review, use, disclosure or distribution is prohibited without express written consent of Kontron. If you are not the intended recipient, please contact the sender and destroy all copies of the original message and enclosed attachments.
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen und ist Eigentum von Kontron. Die Verwendung und Weitergabe von jeglichen Inhalten ist ohne ausdrückliche schriftliche Genehmigung von Kontron strikt untersagt. Wenn Sie diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten diese Mail und enthaltene Dokumente.
Hi Claus,
thanks for your patch. I will work on it and try to integrate it more with the existing flashrom structure.
On 13.05.2008 15:29, Claus Gindhart wrote:
This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
Expect some feedback and a patch against your tree to improve integration tomorrow.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
Hi Claus,
thanks for your patch. I will work on it and try to integrate it more with the existing flashrom structure.
On 13.05.2008 15:29, Claus Gindhart wrote:
This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
Expect some feedback and a patch against your tree to improve integration tomorrow.
I suggest we check this in and you work on the public tree. There is no reason to delay check-in.
On 13.05.2008 17:37, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
Hi Claus,
thanks for your patch. I will work on it and try to integrate it more with the existing flashrom structure.
On 13.05.2008 15:29, Claus Gindhart wrote:
This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
Expect some feedback and a patch against your tree to improve integration tomorrow.
I suggest we check this in and you work on the public tree. There is no reason to delay check-in.
Sorry, the patch Claus posted duplicates a lot of existing infrastructure because the existing SPI abstraction was not clearly visible. That means to support a SPI flash chip with ICH9 and any other chipset we have to add it twice to flashchips.c. I have posted 3 patches to improve the existing SPI abstraction and 2 of them have already been merged. Once the third patch is merged, I can eliminate all reasons for duplicate listings in flashcips.c with a fourth patch. I expect to have all patches merged tomorrow and then we can rework Claus' patch a bit more.
Claus, if I don't find time until tomorrow to post a patch against your tree, can you look into using spi.h for some of your #defines? I'm thinking of these:
+#define SPI_ST_M25P_COMMAND_WRITE 0x02 +#define SPI_ST_M25P_COMMAND_READ 0x03 +#define SPI_ST_M25P_COMMAND_ERASE 0xd8 +#define SPI_ST_M25P_COMMAND_WRITE_DISABLE 0x04 +#define SPI_ST_M25P_COMMAND_READ_STATUS 0x05 +#define SPI_ST_M25P_COMMAND_WRITE_ENABLE 0x06 +#define SPI_ST_M25P_COMMAND_JDEC 0x9F +#define SPI_ST_M25P_COMMAND_RES_PD 0xab +#define SPI_ST_M25P_COMMAND_WRITE_S_EN 0x50 +#define SPI_ST_M25P_COMMAND_WRITE_S 0x01 +#define SPI_ST_M25P_COMMAND_BULK_ERASE 0xc7
AFAICS all of them are in spi.h under a different name and it would be cool if you could reuse the definitions from spi.h
Regards, Carl-Daniel
Hi Claus,
flashrom spi.c has been greatly restructured in the last few commits and all IT87 specific routines have been moved to it87spi.c and useful #defines have been moved to spi.h. I hope spi.c is now a lot more clear and understandable. Can you check? AFAICS we can reduce the amount of code in ichspi.c by ~50% or more if we use the generic functions from spi.c.
I also merged some of your changes in flash.h (albeit at a different place) and added dozens of ATMEL flash chip IDs to flash.h.
On 13.05.2008 17:52, Carl-Daniel Hailfinger wrote:
Sorry, the patch Claus posted duplicates a lot of existing infrastructure because the existing SPI abstraction was not clearly visible. That means to support a SPI flash chip with ICH9 and any other chipset we have to add it twice to flashchips.c. I have posted 3 patches to improve the existing SPI abstraction and 2 of them have already been merged. Once the third patch is merged, I can eliminate all reasons for duplicate listings in flashcips.c with a fourth patch. I expect to have all patches merged tomorrow and then we can rework Claus' patch a bit more.
Claus, if I don't find time until tomorrow to post a patch against your tree, can you look into using spi.h for some of your #defines? I'm thinking of these:
+#define SPI_ST_M25P_COMMAND_WRITE 0x02 +#define SPI_ST_M25P_COMMAND_READ 0x03 +#define SPI_ST_M25P_COMMAND_ERASE 0xd8 +#define SPI_ST_M25P_COMMAND_WRITE_DISABLE 0x04 +#define SPI_ST_M25P_COMMAND_READ_STATUS 0x05 +#define SPI_ST_M25P_COMMAND_WRITE_ENABLE 0x06 +#define SPI_ST_M25P_COMMAND_JDEC 0x9F +#define SPI_ST_M25P_COMMAND_RES_PD 0xab +#define SPI_ST_M25P_COMMAND_WRITE_S_EN 0x50 +#define SPI_ST_M25P_COMMAND_WRITE_S 0x01 +#define SPI_ST_M25P_COMMAND_BULK_ERASE 0xc7
AFAICS all of them are in spi.h under a different name and it would be cool if you could reuse the definitions from spi.h
Regards, Carl-Daniel
Regards, Carl-Daniel
Hi Carl-Daniel,
my colleague Dominik will investigate into this; we will provide a new patch within the next few days.
Hi Claus, hi Dominik,
On 14.05.2008 08:23, Claus Gindhart wrote:
Hi Carl-Daniel,
my colleague Dominik will investigate into this; we will provide a new patch within the next few days.
Thanks. By the way, the ICH8 datasheet says your code will not work with ICH8 (wrong SPIBAR address), so you might want to remove ICH8 bits from the patch. Hardcoding the RCRB address is a problem as well.
I have sent another patch a few minutes ago which will allow you to replace is_supported_chipset() with ich9_detected and curflash->virtual_registers with *ich_spibar. That patch also calculates the correct RCRB address and the correct SPIBAR address. The subject of the mail was "[coreboot] [PATCH] flashrom: Infrastructure for ICH9 merge".
Regards, Carl-Daniel
On 14.05.2008 16:17, Carl-Daniel Hailfinger wrote:
Hi Claus, hi Dominik,
On 14.05.2008 08:23, Claus Gindhart wrote:
Hi Carl-Daniel,
my colleague Dominik will investigate into this; we will provide a new patch within the next few days.
Thanks. By the way, the ICH8 datasheet says your code will not work with ICH8 (wrong SPIBAR address), so you might want to remove ICH8 bits from the patch. Hardcoding the RCRB address is a problem as well.
I have sent another patch a few minutes ago which will allow you to replace is_supported_chipset() with ich9_detected and curflash->virtual_registers with *ich_spibar. That patch also calculates the correct RCRB address and the correct SPIBAR address. The subject of the mail was "[coreboot] [PATCH] flashrom: Infrastructure for ICH9 merge".
That patch has been merged in r3314. You can also drop munmap_ich_registers() and map_ich_registers() now.
If you can create a wrapper for run_opcode() which can be called from spi_command(), you should be able to drop ~80% of the code in ichspi.c. Such a wrapper would look up the opcode from spi_command() in the list of programmed opcodes and convert the arguments. AFAICS this should be easy.
Regards, Carl-Daniel
Hi Carl-Daniel,
On Wednesday 14 May 2008 17:11, Carl-Daniel Hailfinger wrote:
On 14.05.2008 16:17, Carl-Daniel Hailfinger wrote:
Hi Claus, hi Dominik,
On 14.05.2008 08:23, Claus Gindhart wrote:
Hi Carl-Daniel,
my colleague Dominik will investigate into this; we will provide a new patch within the next few days.
Thanks. By the way, the ICH8 datasheet says your code will not work with ICH8 (wrong SPIBAR address), so you might want to remove ICH8 bits from the patch. Hardcoding the RCRB address is a problem as well.
I have sent another patch a few minutes ago which will allow you to replace is_supported_chipset() with ich9_detected and curflash->virtual_registers with *ich_spibar. That patch also calculates the correct RCRB address and the correct SPIBAR address. The subject of the mail was "[coreboot] [PATCH] flashrom: Infrastructure for ICH9 merge".
That patch has been merged in r3314. You can also drop munmap_ich_registers() and map_ich_registers() now.
If you can create a wrapper for run_opcode() which can be called from spi_command(), you should be able to drop ~80% of the code in ichspi.c. Such a wrapper would look up the opcode from spi_command() in the list of programmed opcodes and convert the arguments. AFAICS this should be easy.
Sounds good. Your infrastructure/restructure patches are a nice approach. I will adjust our code and provide you with the patches.
Regards, Dominik
On Thu, May 15, 2008 at 09:43:23AM +0200, Dominik Geyer wrote:
Sounds good. Your infrastructure/restructure patches are a nice approach. I will adjust our code and provide you with the patches.
I'm looking forward to those!
Sorry about the extra work this has caused you, but I am very happy to get rid of this achilles heel in flashrom.
Thank you!
//Peter
Hi Dominik,
On 15.05.2008 09:43, Dominik Geyer wrote:
On Wednesday 14 May 2008 17:11, Carl-Daniel Hailfinger wrote:
On 14.05.2008 16:17, Carl-Daniel Hailfinger wrote:
On 14.05.2008 08:23, Claus Gindhart wrote:
Hi Carl-Daniel,
my colleague Dominik will investigate into this; we will provide a new patch within the next few days.
Thanks. By the way, the ICH8 datasheet says your code will not work with ICH8 (wrong SPIBAR address), so you might want to remove ICH8 bits from the patch. Hardcoding the RCRB address is a problem as well.
I have sent another patch a few minutes ago which will allow you to replace is_supported_chipset() with ich9_detected and curflash->virtual_registers with *ich_spibar. That patch also calculates the correct RCRB address and the correct SPIBAR address. The subject of the mail was "[coreboot] [PATCH] flashrom: Infrastructure for ICH9 merge".
That patch has been merged in r3314. You can also drop munmap_ich_registers() and map_ich_registers() now.
If you can create a wrapper for run_opcode() which can be called from spi_command(), you should be able to drop ~80% of the code in ichspi.c. Such a wrapper would look up the opcode from spi_command() in the list of programmed opcodes and convert the arguments. AFAICS this should be easy.
Sounds good. Your infrastructure/restructure patches are a nice approach. I will adjust our code and provide you with the patches.
Thanks! I'm looking forward to these patches.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Claus Gindhart -
Dominik Geyer -
Peter Stuge -
Stefan Reinauer