Message-ID: <363aa89e-de06-6fc4-e396-d8cc311017b9@intel.com>
Date:   Wed, 15 Mar 2023 12:29:36 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v3 4/8] platform/x86/intel/ifs: Introduce Array Scan test
 to IFS
Content-Language: en-US
To:     "Luck, Tony" <tony.luck@intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        "markgross@kernel.org" <markgross@kernel.org>
CC:     "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "rostedt@goodmis.org" <rostedt@goodmis.org>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "platform-driver-x86@vger.kernel.org" 
        <platform-driver-x86@vger.kernel.org>,
        "patches@lists.linux.dev" <patches@lists.linux.dev>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        "Macieira, Thiago" <thiago.macieira@intel.com>,
        "Jimenez Gonzalez, Athenas" <athenas.jimenez.gonzalez@intel.com>,
        "Mehta, Sohil" <sohil.mehta@intel.com>
References: <20230214234426.344960-1-jithu.joseph@intel.com>
 <20230301015942.462799-1-jithu.joseph@intel.com>
 <20230301015942.462799-5-jithu.joseph@intel.com>
 <7f82f241-39ee-15e0-1ae7-e98e50730c95@redhat.com>
 <275d2f04-782f-2c9d-187a-7a510bf34f41@redhat.com>
 <SJ1PR11MB60831C996C2EC462262948CDFCB99@SJ1PR11MB6083.namprd11.prod.outlook.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <SJ1PR11MB60831C996C2EC462262948CDFCB99@SJ1PR11MB6083.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WDkwQytsbjgyM0tNaU4rc1VRWkhHVUc2VW8yKytJeHZWSFVkVDhpazU4Vk5H?=
 =?utf-8?B?WjdJeExWQ0FjeUd1WEZ3NFhkcnNRbWZ6WStURG1ESUtHV0MveDViYlBvMTVJ?=
 =?utf-8?B?S3Rydjltc2JDMHEyMUhudlhEZE5iem1TWi9CYmVITjBDMVJKRnNWZXVOaXNa?=
 =?utf-8?B?SDhONUx0OGsxQ1lGdWl5allCSEJkdHg2RXIrL0hKL3NvNWlyRk5mY2FvTzNM?=
 =?utf-8?B?c3pFZFUyYzNQeTRpQ0lJdUlScStLb2Nqc3lXVEdRWFlZS0tpdTBkY0hHbWRi?=
 =?utf-8?B?TzRUYkFzYWJhekFVaVpkc29kdjdnZm9ScGthNDd2UEJqai82YmFsdHVBOGY3?=
 =?utf-8?B?YXlnVUkvei9KVjRJbmV5allSUmVSeDREelJJWEdqWXhvdFdJOE9ONnh0VW1i?=
 =?utf-8?B?SmdYelB0dlJGY0xoMUVyNERMTDVnak9BckFubUNWUmF1Ujh6eW5jbm5TVTlR?=
 =?utf-8?B?NUNKMjJhUDd6R3YyRzRMWjZvaFczdjZRcjl2emxjR3d3cUF4cG00OFVxU1pT?=
 =?utf-8?B?c1hKVU9Tdm14elRxaWhwbXIxZEQvVEZJMEhlczhnV0JMTGQvN2JqT2xUS21o?=
 =?utf-8?B?TS9QSmF0eWNRRTNoblFOODJORkxYd2ZXT3RJWisrdHRaSXJ1N1VwenRQQklM?=
 =?utf-8?B?TWo2dkF3cEZoNmhwMWI0OWpRUUx4Q3F4aUdRNTNtVmxVbDlPeld3SE9ySGwx?=
 =?utf-8?B?ZTZxOEtZUk5PaXNDcVRkblZxTXUxRFdXaVBqcUpRREI0WEFqOVdhSWVQSEFY?=
 =?utf-8?B?TjFXcVBGTVE5YUtTY3JiMWpKWThWaUdURmNaVjJQaE9mcFZvcWRlN0tYY0po?=
 =?utf-8?B?K2FQZzZFYUxIYWlMbkJ4cUdZNVp3UmZzaUNtMlhDQUdvUTRQNXZlbzZxSVNj?=
 =?utf-8?B?NjFJVjFDSXltVUVkVmswdVdzUXdiL2ZDUTIwRHhiMWNqSGdwd2hsUlNIVW9G?=
 =?utf-8?B?RXphOUR1TWx6YXQ2VTF1eGljb1AranBtUXBqTjl6Z1BOSFR4a1UrVTNvcHJR?=
 =?utf-8?B?L2Y0amZueWh1V3puaGxLY1dsbFlOZGdlY25xRk5OZmQxazNJOG9wRFU4RWV3?=
 =?utf-8?B?aWdDQnlQOHhJMzZ6L2l2Q3RlWGJJUHE0bURMblJramVzbkZvRVJVL054SEJk?=
 =?utf-8?B?N3UraDFqcHduSm8rQWZqMzF1Vit4dlZ3WEJkSFdJbDRoMjFOOHBIUGt0MDBD?=
 =?utf-8?B?OGxaRk5yUzZoQmtPRldXUjh4VGYzenRjZHZJc3FRbWVNT0RuSnU0ZDdQUXJP?=
 =?utf-8?B?RGU4OXM3alBKajlmUGRRa01DVzBPSlJuVUIrK0g2ZTltUW1meEhtSGJ1a29v?=
 =?utf-8?B?QmFiZDBTcFh5cklNRU4rWEc4OGZuSzhXN2Rubkc3a2xMdFNYdTNHOHFSbzJj?=
 =?utf-8?B?MGpIL3hoYXpCaTlDMVVGZzNlVGNiN0lPWnVGQ0taRjJ4UXpMS1lvM3YxbVdB?=
 =?utf-8?B?YUpjTFZOWkxCT0J3NjZOdDR3Sm0xNzExd24rUlpOZTdTWHBKRENMdXdIUzNP?=
 =?utf-8?B?UlRCYzhGMkZyakl6dUZBQXEwbTlFdWVvZGpqR1BudlBhZG9GdkFjT25FdWpU?=
 =?utf-8?B?MG9tSzBxSEpXcXdlTC9BNmdHb3lFa3VkbjJSTmFLdG1tYzgrbVg1UkNXNWZm?=
 =?utf-8?B?RFpxYkM2QWJheTdRMGJnZk4yemtMemZwTWxYTW5HWGFsWURjUHB6bzZsaUkr?=
 =?utf-8?B?YWhvY0NkVWFjRjM2WUJxSktYRjhEbDc1WTFad3daUkxpUi9NY1IzTWVEZEcr?=
 =?utf-8?B?VUY3Zk5vSHRhWXAyK29YL1JEL2lWN0FkTytuTmx0TEhEL2JjcVpJVmtxcksr?=
 =?utf-8?B?WVBVOTFtWkJxTXpha3JtaFNDdHRmcHVKeUpvdjVxOFlHYVgvbkc2N3g1L3h2?=
 =?utf-8?B?NXEva1AyblkvQkVjdGhPdXk3WUpmRjRwdnZ1Wmdzd0lxZ2Z2TGxJeFlTaThV?=
 =?utf-8?B?M2lqTjhWeTl3VUVEK2MwdVg5VXlUM2dNK0FDSzVjQzhTVzJ4Q0ptd0lDWFFn?=
 =?utf-8?B?ZVBnM216ZGxsU1puTExCRFlnQkVxaDJ0c2phSGZmSGt4Sm8wZU9mdUFhV0Vl?=
 =?utf-8?B?SkJLbDN4dUpteFJFbDVqSGZXK0ljcm01cng4aFhnT1JGRUZ5RmpaY2hQTmVq?=
 =?utf-8?B?Y3RZbmo5RXN0SnZQMEhUUXl5RVVXYUU3ZVlZQjBaUjZvVFJNUnlQUWJIcWpj?=
 =?utf-8?B?YUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b1d5c939-e5fc-4509-c1c2-08db258b9eaf
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3771.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2023 19:29:39.8401
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: hF8sye2k9jKyauSAsZPH10nAdDt3G+vXMWhvYkfTJWeT91WwkB2rfdm/vcYTLUuRWM+okXDOmmsq7qvmlVsDYg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7091
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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>/ …

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

Jithu