RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-22 Thread Limonciello, Mario
> So I've been thinking more about this and to me this whole argument > sounds a lot like we just want to have our own little corner to > play in, without needing to worry about what other vendors do. > > And then Lenovo, and HP and who knows else will all want the same > and we and up with at lea

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-22 Thread Hans de Goede
Hi, On 9/22/20 10:57 AM, Hans de Goede wrote: Hi, On 9/21/20 5:26 PM, Limonciello, Mario wrote: I will do another more detailed reply in another email, but I would like to focus at the main pain point here, which is the using a generic sysfs-ABI/class vs using a Dell specific sysfs-ABI.

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-22 Thread Hans de Goede
Hi, On 9/21/20 5:26 PM, Limonciello, Mario wrote: Well if different schemes are supported and each scheme has its own type, then I would expect there to be say / e.g.: /sys/class/firmware-attributes/dell/authentication/admin-password (with a type of "password") and: /sys/class/firmware-attribu

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-22 Thread Hans de Goede
Hi, On 9/21/20 5:26 PM, Limonciello, Mario wrote: I will do another more detailed reply in another email, but I would like to focus at the main pain point here, which is the using a generic sysfs-ABI/class vs using a Dell specific sysfs-ABI. I guess a could way to look at the generic sysfs

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-21 Thread Limonciello, Mario
> > Well if different schemes are supported and each scheme has its own type, > then I would expect there to be say / e.g.: > > /sys/class/firmware-attributes/dell/authentication/admin-password > (with a type of "password") and: > /sys/class/firmware-attributes/dell/authentication/admin-hotp > (w

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-21 Thread Hans de Goede
Hi, On 9/17/20 6:18 PM, Limonciello, Mario wrote: Those are very different flows to get to and change the same "types" of data. By Dell's interface being Dell specific we can guarantee that the documented flow works how it should. Documenting the flow could be part of the documentation for

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-17 Thread Limonciello, Mario
> > Those are very different flows to get to and change the same "types" of > data. By Dell's interface > > being Dell specific we can guarantee that the documented flow works how it > should. > > Documenting the flow could be part of the documentation for the > different passwd types. In the c

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-17 Thread Hans de Goede
Hi, On 9/14/20 6:06 PM, Limonciello, Mario wrote: So my thinking here is as follows: 1. AFAIK other vendors may want to do something similar in the near future 2. The interface you (Dell) have come up with looks pretty generic / complete to me Dell sets precedent here by having the first d

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Limonciello, Mario
> So my thinking here is as follows: > > 1. AFAIK other vendors may want to do something similar in the near future > 2. The interface you (Dell) have come up with looks pretty generic / complete > to me > > > Would those possible options > > be hardcoded in their kernel driver? > > Maybe, so th

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Hans de Goede
Hi, On 9/3/20 4:27 PM, Limonciello, Mario wrote: + /* look up if user set a password for the requests */ + current_password = get_current_password("Admin"); + if (!current_password) + return -ENODEV; Can we instead of passing "Admin" and "System" to this function jus

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Hans de Goede
Hi, On 9/1/20 4:17 PM, Limonciello, Mario wrote: So first of all some comments on the userspace (sysfs) API for that. Getting this part right is the most important part of this patch, as that will be set in stone once merged. My first reaction to the suggest API is that I find the sorting by

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Hans de Goede
Hi, On 9/1/20 4:17 PM, Limonciello, Mario wrote: So first of all some comments on the userspace (sysfs) API for that. Getting this part right is the most important part of this patch, as that will be set in stone once merged. My first reaction to the suggest API is that I find the sorting by

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Hans de Goede
Hi, On 9/14/20 10:45 AM, Hans de Goede wrote: +   lower_bound:    A file that can be read to obtain the lower +   bound value of the + +   modifier:   A file that can be read to obtain attribute-level +   dependency rule which has to be met to configure +

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-14 Thread Hans de Goede
Hi, Sorry for being slow with getting back to you on this. On 9/1/20 4:22 PM, Limonciello, Mario wrote: "A read-only attribute enumerating if a reboot is pending on any BIOS attribute change." does not really seem to make much sense. I guess what this is trying to say is: "This read-only attr

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-03 Thread Limonciello, Mario
Andy, Thanks for your feedback. > > > > +bool get_pending_changes(void) > > > +{ > > > + struct wmi_interface_priv *priv; > > > + > > > + priv = get_first_interface_priv(); > > > + if (priv) > > > + return priv->pending_changes; > > > > + return 0; > > 0 is not boole

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-02 Thread Andy Shevchenko
On Tue, Sep 1, 2020 at 5:58 PM Hans de Goede wrote: > On 7/30/20 4:31 PM, Divya Bharathi wrote: ... > > +bool get_pending_changes(void) > > +{ > > + struct wmi_interface_priv *priv; > > + > > + priv = get_first_interface_priv(); > > + if (priv) > > + return priv->pending_

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-01 Thread Hans de Goede
Hi, So as promised a third mail with a review focused some more on the code. On 7/30/20 4:31 PM, Divya Bharathi wrote: From: Divya Bharathi The Dell WMI Systems Management Driver provides a sysfs interface for systems management to enable BIOS configuration capability on certain Dell Systems.

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-01 Thread Limonciello, Mario
> Going forward I will be helping Andy and Darren with maintaining the > drivers/platform/x86/* drivers. > > So one of the first things which I'm doing with that hat on, > is review this patch. Congrats on the new responsibilities :) > > So first of all some comments on the userspace (sysfs) AP

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-01 Thread Limonciello, Mario
> > "A read-only attribute enumerating if a reboot is pending on any BIOS > attribute > change." > does not really seem to make much sense. I guess what this is trying to say > is: > > "This read-only attribute reads 1 if a reboot is necessary to apply pending > BIOS > attribute changes"? > >

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-01 Thread Hans de Goede
Hi, I promised a second review focussed on the code, but first a bit more detailed review of the sysfs-files for the different attribute types... On 7/30/20 4:31 PM, Divya Bharathi wrote: From: Divya Bharathi The Dell WMI Systems Management Driver provides a sysfs interface for systems manage

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-09-01 Thread Hans de Goede
Hi All, Going forward I will be helping Andy and Darren with maintaining the drivers/platform/x86/* drivers. So one of the first things which I'm doing with that hat on, is review this patch. On 7/30/20 4:31 PM, Divya Bharathi wrote: From: Divya Bharathi The Dell WMI Systems Management Drive

Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-08-10 Thread Andy Shevchenko
On Sat, Aug 08, 2020 at 06:37:44PM +, Limonciello, Mario wrote: > +Andy > > Andy, can you please review this one? Yes, at some point. It's anyway material to at least v5.10. > Thanks, > > > -Original Message- > > From: platform-driver-x86-ow...@vger.kernel.org > ow...@vger.kernel.o

RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

2020-08-08 Thread Limonciello, Mario
+Andy Andy, can you please review this one? Thanks, > -Original Message- > From: platform-driver-x86-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf Of Divya Bharathi > Sent: Thursday, July 30, 2020 9:31 AM > To: dvh...@infradead.org > Cc: LKML; platform-driver-...@vger.kernel.or