Hi Hans
On 2021-11-16 09:07, Hans de Goede wrote:
> Hi Mark,
>
> On 11/9/21 00:25, Mark Pearson wrote:
>> Implement Opcode support.
>> This is available on ThinkCenter and ThinkStations platforms and
>> gives improved password setting capabilities
>>
>> Add options to configure System, HDD & NVMe passwords.
>> HDD & NVMe passwords need a user level (user/master) along with
>> drive index.
>>
>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
>
> The change you are making to tlmi_probe() is already in my
> review-hans branch and the line numbers also do not seem to
> match in various places, please rebase this for v2.
Yeah - I had this written before that patch came through and wasn't sure
what to do. I'll rebase
>
> I also have some remarks inline.
>
>> ---
>> drivers/platform/x86/think-lmi.c | 303 +++++++++++++++++++++++++++----
>> drivers/platform/x86/think-lmi.h | 28 ++-
>> 2 files changed, 296 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
>> index 3671b5d20613..04810c5ced93 100644
>> --- a/drivers/platform/x86/think-lmi.c
>> +++ b/drivers/platform/x86/think-lmi.c
>> @@ -116,8 +116,23 @@
>> */
>> #define LENOVO_GET_BIOS_SELECTIONS_GUID "7364651A-132F-4FE7-ADAA-40C6C7EE2E3B"
>>
<snip>
>> +
>> +static struct kobj_attribute auth_level = __ATTR_RW(level);
>> +
>> static struct attribute *auth_attrs[] = {
>> &auth_is_pass_set.attr,
>> &auth_min_pass_length.attr,
>> @@ -473,6 +606,8 @@ static struct attribute *auth_attrs[] = {
>> &auth_mechanism.attr,
>> &auth_encoding.attr,
>> &auth_kbdlang.attr,
>> + &auth_index.attr,
>> + &auth_level.attr,
>
> This will add the index and level attr to all auth dirs,
> but they should only be added to the NVMe and HDD dirs,
> right ?
>
> Please add an is_visible callback (see recent thinkpad_acpi changes)
> and hide these for the other auth dirs.
Good idea - I hadn't thought of that. Will do
>
<snip>
>
> Except for the one remark and the need to renase this
> looks good overall, thank you.
>
Excellent - thanks!
Mark
