On 26/06/30 09:55AM, Nikhil Kumar Singh wrote: > Hi Aditya, > > One comment below. > > On 23/06/26 19:49, Aditya Gupta wrote: > > @@ -77,6 +77,7 @@ static void test_spi_seeprom(const void *data) > > const PnvChip *chip = data; > > QTestState *qts = NULL; > > g_autofree char *tmp_path = NULL; > > + const char *machine = "powernv10"; > Would it make sense to introduce `pnv_get_machine_type()` in patch 2 > instead of patch 3? That would avoid hardcoding `"powernv10"` here and > allow this patch to use the helper directly.
Could be done, but i will prefer to keep it in current state, as of this patch, power11 wasn't added to this test, and the test only runs for powernv10, later as powern11 gets added in next patch this is changed to use pnv_get_machine_type. If pnv_get_machine_type gets introduced in this patch, it makes sense to modify all other tests also to use it, which is more than what this patch intends to do (just remove hardcoded index in spi test). To me, it seems okay to keep it as is, what do you say ? > > Otherwise, the patch looks good to me. Thanks for reviewing, Nikhil ! - Aditya G > > Thanks, > Nikhil
