Thanks Ilpo On Mon, May 29, 2023, at 7:36 AM, Ilpo Järvinen wrote: > On Fri, 26 May 2023, Mark Pearson wrote: > >> The system password identification was incorrect. This means that if >> the password was enabled it wouldn't be detected correctly; and setting >> it would not work. >> Also updated code to use TLMI_SMP_PWD instead of TLMI_SYS_PWD to be in >> sync with Lenovo documentation. >> >> Correct these mistakes. >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx> > > Missing Fixes tag? Yes - will add. > >> --- >> Changes in v2: >> - Updated define name to be SMP_PWD instead of SYS_PWD >> - Clarified in comments what each password type is. >> Changes in v3: None. Version bump with rest of series >> >> drivers/platform/x86/think-lmi.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c >> index 2745224f62ab..c7e98fbe7c3d 100644 >> --- a/drivers/platform/x86/think-lmi.c >> +++ b/drivers/platform/x86/think-lmi.c >> @@ -168,11 +168,11 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support"); >> */ >> #define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4" >> >> -#define TLMI_POP_PWD (1 << 0) >> -#define TLMI_PAP_PWD (1 << 1) >> -#define TLMI_HDD_PWD (1 << 2) >> -#define TLMI_SYS_PWD (1 << 3) >> -#define TLMI_CERT (1 << 7) >> +#define TLMI_POP_PWD (1 << 0) /* Supervisor */ >> +#define TLMI_PAP_PWD (1 << 1) /* Power-on */ >> +#define TLMI_HDD_PWD (1 << 2) /* HDD/NVME */ >> +#define TLMI_SMP_PWD (1 << 6) /* System Management */ >> +#define TLMI_CERT (1 << 7) /* Certificate Based */ > > Whe you're adding Fixes tag, please make this change minimal by just > adding TLMI_SMP_PWD. > > The rest of these define changes are a good too but it's unrelated to the > actual fix so they should be in a separate patch. And once you move it > into own change, convert to BIT() while at it. > I was asked previously to clarify what SMP stood for so added the comment and it seemed odd to only clarify one and not the others. Can I push back on this request. Doing two separate patches for just that doesn't make sense to me. Thanks for the review Mark
