Re: [PATCH v3 4/8] platform/x86/intel/ifs: Introduce Array Scan test to IFS

[Hans de Goede]

Hi,

On 3/15/23 20:29, Joseph, Jithu wrote:
> 
> 
> On 3/13/2023 10:21 AM, Luck, Tony wrote:
> 
>>
>>
>> I'm not sure this is better than splitting the tests into different directories.
>>
> 
> To provide a bit more context to Tony's response - 
> There are similarities between tests (one of the test inputs is cpu number , which is 
> common among different tests , so are the test outputs described via status / details attributes)
> and some differences too (distinct to each test are the test patterns, some tests needs to
> load test patterns appropriate for that test type and some does not)
> 
> Current approach of keeping each test's attributes in its own directory (intel_ifs_n)
> allows the driver to account for the differences naturally, for e.g load test file
> suited for a specific test. (I.e if the load is issued from intel_ifs_<n> , the driver
> will load test patterns from /lib/firmware/intel/ifs/ifs_<n>/ff-mm-ss-xx.<type_n>).
> If load is not applicable for a test type , test directory doesn’t show load and image_version attributes). 
> 
> As Tony mentioned, similar effect might be achieved using distinct load / run (and image_version)
> files for each test type, but the current approach of organizing files per test feels a little
> bit more intuitive. 
> 
> Grouping attributes per test was the original design intent , when the first test was introduced,
> as indicated by the existing ABI doc (Documentation/ABI/testing/sysfs-platform-intel-ifs),
> wherein attributes are described under /sys/devices/virtual/misc/intel_ifs_<N>/ …

Ok I see, lets go with 1 intel_ifs device per test-type then.

If I understood things correctly esp. also with the /lib/firmware path then the <N> in intel_ifs_<N> basically specifies the test-type, correct ?

If I have that correct please add this to the ABI documentation in the form of a list with

intel_ifs_<N> <-> test-type

mappings. And also add documentation to each attribute for which test-types the attribute is valid (this can be "all" for e.g. status, to avoid churn when adding more test types).

> Hans, Shall I revise the series incorporating the rest of your comments ?

Yes please.

Regards,

Hans