[S] Change in coreboot[main]: block/fast_spi: Use read32p/write32p for spi rw

[Arthur Heymans (Code Review)]

Attention is currently required from: Ashish Kumar Mishra, Saurabh Mishra, Subrata Banik.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82079?usp=email )

Change subject: block/fast_spi: Use read32p/write32p for spi rw
......................................................................


Patch Set 2:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/82079/comment/df7cb749_1f70679f :
PS2, Line 9: The memcpy impl. for
           : x86_64 changed to 8 byte copy and due to 4 byte limit, spi rw fails. MRC
           : cache rw regression observed in existing X86_64 platforms. Hence update
           : rw ops to use read32p/write32p.
Maybe change this to say that the hardware only accepts at most dword transactions at most which won't work with the current 64bit implementation of memcpy?


File src/soc/intel/common/block/fast_spi/fast_spi_flash.c:

https://review.coreboot.org/c/coreboot/+/82079/comment/cfbf4466_bb5eabfe :
PS2, Line 80: const uint8_t *byte_ptr = (const uint8_t *)data;
            : 	size_t bytes_to_copy;
            : 	union {
            : 		uint32_t full;
            : 		uint8_t bytes[4];
            : 	} dword;
            : 
            : 	for (size_t i = 0; i < len; i += 4) {
            : 		dword.full = 0;
            : 
            : 		bytes_to_copy = (len - i < 4) ? len - i : 4;
            : 		for (size_t j = 0; j < bytes_to_copy; j++)
            : 			dword.bytes[j] = byte_ptr[i + j];
            : 
            : 		write32p(ctx->mmio_base + SPIBAR_FDATA(i >> 2), dword.full);
            : 	}
I think the HW accepts at most dword transactions, but also word and byte ones. Which is why the 32bit memcpy worked.

This code isn't efficient. Just loop over dwords and put them into SPIBAR_FDATA, then for the remaining bytes do it bytewise (just like 32bit memcpy)?



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/82079?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I317c7160bf192dd2aeacebf6029a809bc97f3420
Gerrit-Change-Number: 82079
Gerrit-PatchSet: 2
Gerrit-Owner: Ashish Kumar Mishra <[e-mail]>
Gerrit-Reviewer: Saurabh Mishra <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Arthur Heymans <[e-mail]>
Gerrit-Attention: Ashish Kumar Mishra <[e-mail]>
Gerrit-Attention: Saurabh Mishra <[e-mail]>
Gerrit-Attention: Subrata Banik <[e-mail]>
Gerrit-Comment-Date: Wed, 24 Apr 2024 12:18:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment