[coreboot-gerrit] [S] Change in coreboot[main]: sb/intel/common/spi: Fix I/O alignment

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78090?usp=email )

Change subject: sb/intel/common/spi: Fix I/O alignment
......................................................................

sb/intel/common/spi: Fix I/O alignment

On ICH9 the SPI control register is not naturally aligned
and a word write might be split into smaller naturally aligned
I/O transactions.

As the first byte starts a new SPI transfer, replace the existing
word write with two byte writes and write the second byte first.

This is required for platforms that do not support unaligned
word I/O instructions and would start a SPI transfer while the
second byte hasn't reached the control register yet.

TEST: Virtual SPI controller on qemu 8.0 doesn't start a transfer
      early.

Change-Id: Id05b1a080911b71b94ef781c6e26d98165f02f67
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78090
Tested-by: build bot (Jenkins) 
Reviewed-by: Kyösti Mälkki 
---
M src/southbridge/intel/common/spi.c
1 file changed, 16 insertions(+), 6 deletions(-)

Approvals:
  Kyösti Mälkki: Looks good to me, approved
  build bot (Jenkins): Verified

diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index c984c6d4..93185d4 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -93,7 +93,7 @@
    uint8_t *data;
    unsigned int databytes;
    uint8_t *status;
-	uint16_t *control;
+	uint8_t *control;	/* Unaligned on ICH9 */
    uint32_t *bbar;
    uint32_t *fpr;
    uint8_t fpr_max;
@@ -300,7 +300,7 @@
    	cntlr.data = (uint8_t *)ich7_spi->spid;
    	cntlr.databytes = sizeof(ich7_spi->spid);
    	cntlr.status = (uint8_t *)&ich7_spi->spis;
-		cntlr.control = &ich7_spi->spic;
+		cntlr.control = (uint8_t *)&ich7_spi->spic;
    	cntlr.bbar = &ich7_spi->bbar;
    	cntlr.preop = &ich7_spi->preop;
    	cntlr.fpr = &ich7_spi->pbr[0];
@@ -317,7 +317,7 @@
    	cntlr.data = (uint8_t *)ich9_spi->fdata;
    	cntlr.databytes = sizeof(ich9_spi->fdata);
    	cntlr.status = &ich9_spi->ssfs;
-		cntlr.control = (uint16_t *)ich9_spi->ssfc;
+		cntlr.control = ich9_spi->ssfc;
    	cntlr.bbar = &ich9_spi->bbar;
    	cntlr.preop = &ich9_spi->preop;
    	cntlr.fpr = &ich9_spi->pr[0];
@@ -579,8 +579,13 @@
    	 * bitesout size was 1, decremented to zero while executing
    	 * spi_setup_opcode() above. Tell the chip to send the
    	 * command.
+		 *
+		 * On ICH9 the control register is not natually aligned.
+		 * Write the high byte first to prevent triggering a transfer
+		 * when writing two bytes on the bus using outw instruction.
    	 */
-		writew_(control, cntlr.control);
+		writeb_(control >> 8, &cntlr.control[1]);
+		writeb_(control, &cntlr.control[0]);

/* wait for the result */
    	status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);
@@ -636,8 +641,13 @@
    	control |= SPIC_DS;
    	control |= (data_length - 1) << 8;

-		/* write it */
-		writew_(control, cntlr.control);
+		/*
+		 * On ICH9 the control register is not natually aligned.
+		 * Write the high byte first to prevent triggering a transfer
+		 * when writing two bytes on the bus using outw instruction.
+		 */
+		writeb_(control >> 8, &cntlr.control[1]);
+		writeb_(control, &cntlr.control[0]);

/* Wait for Cycle Done Status or Flash Cycle Error. */
    	status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);