On Friday 25 December 2015 02:23:04 Andy Lutomirski wrote: > On Thu, Dec 24, 2015 at 1:18 PM, Pali Rohár <pali.ro...@gmail.com> > wrote: > > According to Dell WMI document mentioned in ML dicussion archived > > at http://www.spinics.net/lists/platform-driver-x86/msg07220.html > > OS should check Dell WMI descriptor structure. Structure also > > provide Dell WMI interface version which is used later. > > I will rebase my big series on top of this. It'll give me a good > excuse to test that I got the probe ordering right. (The code is > explicitly intended to support use cases like this, and now I'll have > a real-world test for it.) I'll also test this in a bit.
Ok!
> > +MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID);
>
> I don't think this is necessary. The driver will only work if both
> wmi devices and, hence, modaliases are present, so there's no need to
> cause just one or the other to trigger dell-wmi autoloading.
Maybe now when you are working on big WMI patch series is time to change
modalias support in WMI to support AND-conjunction (&&) on WMI aliases,
not just OR (one alias match).
Something like: load dell-wmi.ko driver if system provides both WMI
GUIDs.
> > +/**
>
> > + * Descriptor buffer is 128 byte long and contains:
> This isn't kerneldoc format, so I think this should just be "/*".
>
Ok, I will fix this in next version.
> > + if (obj->buffer.length != 128) {
> > + pr_err("Dell descriptor buffer has invalid length
> > (%d)\n", + obj->buffer.length);
> > + kfree(obj);
> > + return -EINVAL;
> > + }
>
> I would advocate for being more permissive: a buffer that is actually
> too short for the fields we need would result in -EINVAL, but a
> buffer that isn't 128 bytes would just be a warning and not cause
> module load to fail.
>
> --Andy
Sounds good, I will change this part.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
