On Mon, May 29, 2023, at 11:50 AM, Ilpo Järvinen wrote: > On Mon, 29 May 2023, Mark Pearson wrote: > >> 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. > > I did not mean removing TLMI_SMP_PWD's comment from this patch just to add > it in the another but the comments to the other bits which should go into > their own patch. The thing here is that fixes should be made minimal to > comply with stable rules. > OK....seems odd to me to be honest, but not something I'd lose sleep over. I'll do that in amongst all the other changes. Thanks Mark