[M] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE

[Anastasia Klimchuk (Code Review)]

Attention is currently required from: Edward O'Callaghan, Jes Klinke, Nikolai Artemiev, Thomas Heijligen.

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email )

Change subject: raiden: Support target index with generic REQ_ENABLE
......................................................................

Patch Set 5: Code-Review+1

(4 comments)

Patchset:

PS2:
> I have added a few more cases to the test. […]

Yes, existing test is running `run_basic_lifecycle` which expects the lifecycle to run successfully.

There is another method in our test framework, which tests initialisation error (invalid value for programmer param for example). This is `run_init_error_path` here https://review.coreboot.org/plugins/gitiles/flashrom/+/refs/heads/main/tests...

If you would like to add a test to verify that init fails with invalid value of param, you can use run_init_error_path for that. I suspect `raiden_debug_io` would not need that many mocks, maybe even no mocks at all: if init fails early those mocks are never called.

File doc/classic_cli_manpage.rst:

https://review.coreboot.org/c/flashrom/+/77999/comment/7c68030b_c99e4595 :
PS4, Line 873:
> I have attempted to merge your suggested paragraph into the existing documentation.

Looks really good

File doc/classic_cli_manpage.rst:

https://review.coreboot.org/c/flashrom/+/77999/comment/a1844fc0_7b8a005a :
PS5, Line 871:   
trailing whitespace

Otherwise the text is very good!

File tests/raiden_debug_spi.c:

https://review.coreboot.org/c/flashrom/+/77999/comment/3154708d_7bf2511b :
PS5, Line 112: 
             : 	snprintf(raiden_debug_param, sizeof(raiden_debug_param),
             :                  "address=%d,target=AP", USB_DEVICE_ADDRESS);
             : 	run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param);
             : 
             : 	snprintf(raiden_debug_param, sizeof(raiden_debug_param),
             :                  "address=%d,target=ec", USB_DEVICE_ADDRESS);
             : 	run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param);
             : 
             : 	snprintf(raiden_debug_param, sizeof(raiden_debug_param),
             :                  "address=%d,target=0", USB_DEVICE_ADDRESS);
             : 	run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param);
             : 
             : 	snprintf(raiden_debug_param, sizeof(raiden_debug_param),
             :                  "address=%d,target=1", USB_DEVICE_ADDRESS);
             : 	run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param);
This is great, thank you so much for adding test coverage!

The only thing: let's make each test case a separate test. Which means, initial test case stays in `raiden_debug_basic_lifecycle_test_success` function, and the other 4 you add into their own test functions.

I don't mind long names for test functions, so you can name them for example:
raiden_debug_targetAP_basic_lifecycle_test_success
raiden_debug_targetEC_basic_lifecycle_test_success
raiden_debug_target0_basic_lifecycle_test_success
raiden_debug_target1_basic_lifecycle_test_success

The definitions of `raiden_debug_fallback_open_state` and `raiden_debug_io` at the beginning of each function you can just copy-paste.

Then you need to add 4 new functions into:
tests/tests.c
tests/tests.h
and at the bottom of this file add SKIP_TEST macro.

And that should be it!