Message-ID: <74058e98-2bbc-3f36-7e3d-538fc2323264@intel.com>
Date:   Thu, 16 Mar 2023 11:11:22 -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 6/8] platform/x86/intel/ifs: Implement Array BIST test
Content-Language: en-US
To:     Hans de Goede <hdegoede@redhat.com>, <markgross@kernel.org>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <gregkh@linuxfoundation.org>, <rostedt@goodmis.org>,
        <ashok.raj@intel.com>, <tony.luck@intel.com>,
        <linux-kernel@vger.kernel.org>,
        <platform-driver-x86@vger.kernel.org>, <patches@lists.linux.dev>,
        <ravi.v.shankar@intel.com>, <thiago.macieira@intel.com>,
        <athenas.jimenez.gonzalez@intel.com>, <sohil.mehta@intel.com>
References: <20230214234426.344960-1-jithu.joseph@intel.com>
 <20230301015942.462799-1-jithu.joseph@intel.com>
 <20230301015942.462799-7-jithu.joseph@intel.com>
 <628e092c-793e-96ce-71f5-80392ad69569@redhat.com>
 <9dde72aa-a28d-2b0e-7b90-6b1996dbf202@intel.com>
 <f45c26ae-865b-b7b6-0f2a-5906d9c0a349@redhat.com>
From:   "Joseph, Jithu" <jithu.joseph@intel.com>
In-Reply-To: <f45c26ae-865b-b7b6-0f2a-5906d9c0a349@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Rk52VEsydzRhME02bytaTFRvUEhreEw0dTJXWm1QK29DNW5KOHVveDVUZzlT?=
 =?utf-8?B?eGtBVVdzQThQenpidXlzWS9PRnNPL1JvblZIWklhRVNSYU1wZlQ5MlB1WFI4?=
 =?utf-8?B?MnpxMFcwM1VkakRLLzltMHhITGhlUEJqa0d1M0NNUStrT2RkQ2NDU1JlQ2Jv?=
 =?utf-8?B?THkxMnl2ZTdoUVFGQmtSUlA0eCszdXFobFBDRlY1TGtwNGYrZk1Zd2V0K3NL?=
 =?utf-8?B?UGN1WWZRRGJNK2p5Yi9IOUxXNWNhcDF0YzNQc1ZFbDVWRFAxYm45Y0l5N3ZG?=
 =?utf-8?B?c2pKcHRBMXl0TzQ2VDRzK0VBSlJMV211VytvQTZsSlFnVU5TUkc0MnVmNDkz?=
 =?utf-8?B?TG41QnlKOUJTYlFqMmZ2eFRET011RGl5MEk3SjJKVWIrT3RkVnBkQTd0bU10?=
 =?utf-8?B?SEJCbmFtdXorbEpxVytlUmFRbDVubXNNWllla3IvSXR2a2lmWTJjcGxqNDU1?=
 =?utf-8?B?TEdSTGVBYWF6Vi95WDVsRm96VWhxNTdhMyt4N1BvTW42cFhlMGN1WE5TVWR3?=
 =?utf-8?B?TVk5ZVNycy9mczdmYzNPR2ozZWpNbkFvUThnUC81UERZNExrRDJBWjdPeHlw?=
 =?utf-8?B?TC9jWGdrUHRsMEM2M2I0bDBJQTdnU0E3cU1tUXhtcUlZWk94cUp0OEdnWnAy?=
 =?utf-8?B?Y0FDUmFVZWo0L1ZQV3V0Z0dwNFNXajBWREdHM21hK1RDTzYrRm43YUpKZG4z?=
 =?utf-8?B?M1JkS1pJcFZFUU9td1FtcGpkSnNhSjlSNGpOMVhSeUsvL1NSMnJtcVVIby92?=
 =?utf-8?B?WnNpNzlFbUVFbk93cUJoNnB4Skx3Q1lTVjBRd1dQek43b3h6WXRpTlNPQ0RL?=
 =?utf-8?B?VlJKcTYzTXg3Y0dPZFdPemVkak1PWkhZSzdHOE9PbVlONGxiMkQvMGJBbTc3?=
 =?utf-8?B?alpuL2toK2l3NVlnRXlNYWRCZU9xaDBnNUVGWUdHM3Zka3FabjdLMGtSMXkr?=
 =?utf-8?B?dEpQMTQ3Y0EvRFE4OG5xZ3ZnQXZQejBrRHJqTlVJUkhjcmhQVkFGTENZbXFs?=
 =?utf-8?B?cGVmTkEzbDUwTnFHQW9MTUF1eGhLVHYydjdOK2NXWHdHWGduNDIzZDFieENa?=
 =?utf-8?B?a0dFV1A1REczbGI4Ym52NjNIRHczakFEaWNIK25hRXlrWGRYc1lETFpCS3BC?=
 =?utf-8?B?SUxDbGcvaVhkNGlxSUNVc2RSZ3ZkNHM0SGVXckV2M3VJV3ZmMHQxSjhZeW9H?=
 =?utf-8?B?R1UzM1lLYmNSbFB1ZFR2bk9BOHZwNXg5NkZTQ3A1RG9aTGlaS0syZzVxQUJt?=
 =?utf-8?B?TkF3bjVtaVNSN1BnclZFWmVWT0s2dE9MbGtBSENjVElYWWpmOEF2d0dSanR2?=
 =?utf-8?B?MVVkQlVyOFdhdFdtM20xVzZSTlBnenZSQzA3Tlg2TGN3a3g4L2Z2Tmt2OGg4?=
 =?utf-8?B?bmk4VWJKVGFDYW5abG5aSjErOG9QMDlOUGFKTjRnSVM1d0thWWdVcDFJNlU2?=
 =?utf-8?B?cDZFdVpjeTBNd01Gd0w1TkVHVHgyNjIrZldtbldqb0FOWFhNNnpWdlJYUDlr?=
 =?utf-8?B?MTMzeVdCN2xwMDhCOE9kVEQxZFNrQ3JNaFc1WW02Y2dDMEx0ZkFwSm9SRW9h?=
 =?utf-8?B?a2NlU25oVjRKS3A2d3NrSDNnNDZiRHJqSll1Z25tRGJDZVlrbzhZQXdHQURn?=
 =?utf-8?B?ZlUrZU5PT0E0dGV3ZXMxMHhkU0x5bXk3a3lSMVI1MDNZbmNsQmJXV0d6dmFK?=
 =?utf-8?B?TmF3OThneWhNSHhiS0xVVUlZc2dxRHBtTS8vT1J1OHA3M0RYMy96N3E2Ymcy?=
 =?utf-8?B?L3V4bDlnNnRzRDRRWHEvUlR0VG1zSEtuSTlSaGFFSnNJN1FLL3VOSzdueFVQ?=
 =?utf-8?B?RnRUM1BjZ1Fib2NXdDRtMUxEVEZoUmhjbVFSOXZWWTVRRWx6UEt4L2x6TG13?=
 =?utf-8?B?WUM3VExjTlFRTFFzTWhIbGxham8zY0Y3aWFSaGtvSkVZNk9JNEppSm5JZDdC?=
 =?utf-8?B?cHV6K3pGdElnQXNody8yUUg1RFdvVkw5VHJ3eitQaW5TZnkxcEY1MG9qRzFj?=
 =?utf-8?B?VFoycFdyTDdzc09RbkczQzZBRmhGT1ZVMldKazJ3OUE3c0dFRnBRSnhwNU81?=
 =?utf-8?B?blluMERraHNLVEtZQmIwMFJYNGdWUmdTemRLQW5TdGVDNkZuc1pKSloxSG5i?=
 =?utf-8?B?cHIwVHg0eU1Nb2dVVTZWc3V0b1MzeERWWnlZcG95Q216OTloUUlLK1dyMTZQ?=
 =?utf-8?B?SEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a87b09c9-ef54-4a0f-784a-08db2649da22
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3768.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Mar 2023 18:11:23.9248
 (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: MKcYJdZUm/jzT4rvtq3xrN1mzAQNEy1ZvKHR331WiwBHBM4oQECYAtyACntgq+98hh2uzaC+kMkekAIvcQtIWQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7244
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 3/16/2023 2:59 AM, Hans de Goede wrote:

> 
> After taking a closer look I do see one unrelated issue with this patch
> sysfs.c: run_test_store() does:
> 
>         if (!ifsd->loaded)
>                 rc = -EPERM;
>         else
>                 rc = do_core_test(cpu, dev);
> 
> But AFAICT the loaded check really only applies to the first (intel_ifs_0 device) test type and the 
> Array BIST test should work fine when loaded is false.
> 
> So I think that the if (!ifsd->loaded) error check should be moved to 
> ifs_test_core() ?
> 

It is possible that I misinterpreted your comment in my earlier reply. (Thanks Tony for pointing it out)
Yes I think moving the load check into ifs_test_core() is better than doing it in run_test_store()

Jithu