[M] Change in flashrom[master]: flashrom.c: Rewrite prepare_chip_access() 4BA enablement

[Jonathon Hall (Code Review)]

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons.

Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71742 )

Change subject: flashrom.c: Rewrite prepare_chip_access() 4BA enablement
......................................................................

Patch Set 1:

(2 comments)

File flashrom.c:

https://review.coreboot.org/c/flashrom/+/71742/comment/e4a27f2c_8a507ddc 
PS1, Line 1916: 	if (!spi_master_no_4ba_modes(flash)) {
I don't really understand this check - the comment on SPI_MASTER_NO_4BA_MODES says "Compatibility modes (..., 4BA mode switch) don't work" - would this flag mean the master has 4BA but the mode switch doesn't work?

I would assume it is not just missing 4BA mode since that would be the same as !spi_master_4ba(), there would probably be a reason for two distinct flags right?  It looks like 'dediprog' could set both flags, I don't have any device to test that though.

Do you know what this flag means?  It used to possibly enter/exit 4ba in this state depending on spi_chip_4ba()/spi_master_4ba().

https://review.coreboot.org/c/flashrom/+/71742/comment/a3bbedc4_6afea3fb 
PS1, Line 1938: 	int ret = spi_master_4ba(flash) ? spi_enter_4ba(flash): spi_exit_4ba(flash);
It's not possible to reach spi_exit_4ba() here due to returning if spi_master_4ba() was false above.

I think the intent was that if the chip supports 4ba but doesn't require it (<= 16M), and master doesn't support 4ba, it would exit 4ba here.  I'm not sure if that makes sense though, why would such a chip be in 4ba in the first place?  Maybe if some buggy program changed it?

So I'm not sure if the answer is to delete the unreachable code or restore the disable path, what do you think?