Re: [PATCH v3 3/5] platform/x86: think-lmi: Correct NVME password handling

[Ilpo Järvinen]

On Mon, 29 May 2023, Mark Pearson wrote:
> On Mon, May 29, 2023, at 8:03 AM, Ilpo Järvinen wrote:
> > On Fri, 26 May 2023, Mark Pearson wrote:
> >
> >> NVME passwords identifier have been standardised across the Lenovo
> >> systems and now use udrp and adrp (user and admin level) instead of
> >> unvp and mnvp.
> >> 
> >> This should apparently be backwards compatible.
> >> 
> >> Also cleaned up so the index is set to a default of 1 rather than 0
> >> as this just makes more sense (there is no device 0).
> >
> > These two sound entirely separate changes. If that's the case, please 
> > make own patch from the send change.
> 
> Ack. It was all related to the index setting and seemed trivial so I 
> lumped together but I can split. This patch series is turning into a 
> good learning exercise for my git skills :) (which are limited)
>
> > Hmm, index_store() still allows 0, is that also related here? Please check 
> > also ABI documentation as index default seems to be mentioned there as 
> > well.
> >
> 
> I'd rather not limit it so 0 isn't allowed in case our BIOS team does 
> something weird in the future; but right now 1 is the default so it 
> makes more sense.

Sure, do what you feel makes sense here. I was just pointing out the 
perceived inconsistency in case it wasn't intentional.

It might be useful to add one sentence into changelog about the reasoning 
so it can be found easier later on (effectively the paragraph you wrote 
above with small tweaks is enough I think).


-- 
 i.