On 01/07/26 00:22, Aditya Gupta wrote:
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 ?
Thanks for the explanation. I agree with keeping it as is for this
patch. Looks good to me.
~ Nikhil
Otherwise, the patch looks good to me.
Thanks for reviewing, Nikhil !
- Aditya G
Thanks,
Nikhil
