On Mon, Nov 6, 2017 at 8:16 PM, <Mario.Limonciello@xxxxxxxx> wrote: >> On Tue, Oct 24, 2017 at 10:59 PM, Mario Limonciello >> <Mario.Limonciello@xxxxxxxx> wrote: Mario, thanks for your comments on the subject. >> > The hope is that eventually drivers on the WMI bus don't need to be very >> > rich in code, but more intelligence comes from the bus. That would mean >> > that the bus parses the MOF and knows what types of data would be passed in >> > individual method GUIDs. The bus would know what size of data that is and >> > what fields represent what in data objects. The vendor drivers may add some >> > filtering or permissions checking, but that would be it. >> > >> > We still don't have MOF parsing in the kernel, but I think that it's good to >> > set up concepts that reflect how we want it to work until it's available. >> > That should mean that if you get the interfaces right that later your driver >> > can shrink. My patch series isn't yet accepted, so what I'm doing isn't >> > necessarily the way it will be done, I just want to let you know about it. >> > >> > The big notable differences with how we're approaching our drivers: >> > 1) GUID's that provide methods are given direct execution paths in sysfs >> > files through your patch. This means that there could be a ton of different >> > sysfs attributes that vary from vendor to vendor based on what they offer. >> > I set up a concept that method type GUID's would be handled by the WMI bus >> > by creating a character device in the /dev/wmi and copying in/out data for >> > those method objects. >> >> Wouldn't that be a little harder to use from userspace ? (But I can >> see how it makes it more generic). > > The concept that we're working with would mean that some userspace software > can read sysfs attributes to understand what data format is being communicated > and then know how to pack data into the character device when communicating > with WMI bus. > > This is closer to the Windows model too with PowerShell. You query the namespace > to determine what methods are offered via the MOF and then you can pack the > data and PowerShell will ferry it out to the ASL to execute and return the results. Interoperability with Windows is a plus in my opinion. So, Windows user or even developer finds themselves in slightly more convenient environment. >> > 2) You don't register all devices with the WMI bus. Each of your GUIDs that >> > you interact with should really be registered with the bus. Some of the >> > data you're interested in should be exposed there. >> > I can't speak on behalf of Darren and Andy here, but I would anticipate they >> > don't want "new" WMI drivers introduced that don't register and use the WMI >> > bus properly. I only see the single GUID that registered. >> >> Well, these aren't really "devices". I can register all methods to the >> BUS though, but it'll be weird for the end user. >> What is your suggestion here ? > > The WMI bus device model means that any "GUID" is a device associated to the WMI > bus. I view your driver as putting another level of granularity on top of that. > Whether you adopt a character device or sysfs attributes for communicating to the > bus, you should still have some sort of driver that packs all the GUID's into subdevices > under say a platform device. > > I think you should look for some feedback from Darren or Andy on how they want to > see this work. My preference here is to try to have as much generic and flexible interface in kernel that may allow user space application utilize customisations. Though I think Darren is more involved in WMI activity and can share his view on all of this. >> > 3) Your driver provides more granular data than mine (as that's how it is >> > exposed by Lenovo's design). I think this is a good thing, and you should >> > find a way to programattically expose your attributes to sysfs instead of >> > debugfs if possible. >> > >> > The driver looks very good to me though, a few nested comments: >> >> Thanks for the review Mario. I'm afraid I won't have much more time in >> the near future to make changes to this driver. What do you think >> would be the minimal set of changes to make this good enough to be >> merged ? > > I'm just a contributor myself who has recently worked on a WMI driver series. > I would defer to Andy and Darren to decide what they would like to accept. ... > it would be difficult to update to the > newer model we're working towards when we have the kernel learning how to > parse MOF and programmatically producing sysfs attributes for interacting with > character devices. ...and since your stuff is scheduled for v4.15 the possibility to (re-)use as much as possible from it is a plus. > Since the kernel has to keep a stable interface to userspace you may run into a > situation that one day you want to update to the new interfaces and might have a > difficult time since you have to continue to offer a configuration option to offer these > old interfaces too. This is a good point. -- With Best Regards, Andy Shevchenko
