Re: [PATCH] drivers/x86: add thinkpad-wmi

[Pavel Machek]

Hi!

> diff --git a/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi b/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi
> new file mode 100644
> index 000000000000..c3673876c5b3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-thinkpad-wmi
> @@ -0,0 +1,50 @@
> +What:		/sys/devices/platform/thinkpad-wmi/password
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		BIOS password needs to be written in this file if set
> +		to be able to change BIOS settings.

Should this go under platform/thinkpad-wmi? Seems like similar
interface is potentially useful on different machines?

> +What:		/sys/devices/platform/thinkpad-wmi/password_encoding
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Password encoding ('ascii' or 'scanmode').
> +
> +What:		/sys/devices/platform/thinkpad-wmi/password_kbd_lang
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Keyboard language used for password. One of 'us', 'fr' and 'gr'.
> +
> +What:		/sys/devices/platform/thinkpad-wmi/password_type
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Password type to be changed when password_change is written to, e.g. 'pap'.

> +What:		/sys/devices/platform/thinkpad-wmi/password_change
> +Date:		Aug 2017
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Writing to this file will set the password specified in password_type.
> +		The new password will not take effect until the next reboot.

With the different "encoding" and "keyboard language" fields, this
looks like great way to lock user out of his own machine ;-(.

> +What:		/sys/devices/platform/thinkpad-wmi/password_settings
> +Date:		Oct 2015
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Display various password settings.

Umm. We have one value per file in sysfs?

> +What:		/sys/devices/platform/thinkpad-wmi/load_default_settings
> +Date:		Oct 2015
> +KernelVersion:	4.14
> +Contact:	"Corentin Chary" <corentin.chary@xxxxxxxxx>
> +Description:
> +		Write anything to this file to load default BIOS
> settings.

Is that reasonable?

> +### password_type
> +
> +Specify the password type to be changed when password_change is written to.
> +Can be:
> +* 'pap': supervisor password
> +* 'pop': power-on-password
> +
> +Other types may be valid, e.g. for user and master disk passwords.

This is extremely weird for /sysfs interface. Sounds like you should
have supervisor_password_change and power_on_password_change, etc...

> +### password_settings
> +
> +Display password related settings. This includes:
> +
> +* password_state: which passwords are set, if any
> +  * bit 0: user password (power on password) is installed / 'pop'
> +  * bit 1: admin/supervisor password is installed / 'pap'
> +  * bit 2: hdd password(s) installed
> +* supported_encodings: supported keyboard encoding(s)
> +  * bit 0: ASCII
> +  * bit 1: scancode
> +* supported_keyboard: support keyboard language(s)
> +  * bit 0: us
> +  * bit 1: fr
> +  * bit 2: gr
> +

If this belongs to the kernel (I'm not convinced), it certainly needs
different/better interface.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature