| Message ID | CAJCLVRCyp0ASdWTx-PxsrDC9zFBPw0U2AtPip+_Hpj2r5gUPwA@mail.gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [RESEND] platform/x86: intel-hid: fix _DSM function index handling | expand |
Hi, On 11/8/20 10:23 AM, Zhen Gong wrote: > According to the ACPI spec 9.1.1 _DSM (Device Specific Method), > intel_hid_dsm_fn_mask, acquired from function index 0, is "a buffer > containing one bit for each function index". When validitaing fn_index, > it should be compared with corresponding bit. > > This buffer is usually longer than a byte. Depending on whether > INTEL_HID_DSM_HEBC_V2_FN exist, it could be either > "Buffer (0x02) { 0xFF, 0x01 }" or "Buffer (0x02) { 0xFF, 0x03 }". > Probably it won't grow larger according to the description. On older > platforms, available functions could be fewer or not supported at all, > i.e., "Buffer (One) { 0x00 }". > > Signed-off-by: Zhen Gong <zhengong@usc.edu> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Note your mail client completely mangled the patch, eating whitespace and replacing tabs with spaces. I've fixed this up manually for this time. Next time please use git send-email and do NOT invoke an editor to change e.g. the subject, instead use the command line options to set a patch subject prefix. Regards, Hans > --- > drivers/platform/x86/intel-hid.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c > index 86261970bd8f..9a52e56f75da 100644 > --- a/drivers/platform/x86/intel-hid.c > +++ b/drivers/platform/x86/intel-hid.c > @@ -141,7 +141,7 @@ static bool intel_hid_execute_method(acpi_handle handle, > > method_name = (char *)intel_hid_dsm_fn_to_method[fn_index]; > > - if (!(intel_hid_dsm_fn_mask & fn_index)) > + if (!(intel_hid_dsm_fn_mask & BIT(fn_index))) > goto skip_dsm_exec; > > /* All methods expects a package with one integer element */ > @@ -214,7 +214,19 @@ static void intel_hid_init_dsm(acpi_handle handle) > obj = acpi_evaluate_dsm_typed(handle, &intel_dsm_guid, 1, 0, NULL, > ACPI_TYPE_BUFFER); > if (obj) { > - intel_hid_dsm_fn_mask = *obj->buffer.pointer; > + switch (obj->buffer.length) { > + default: > + case 2: > + intel_hid_dsm_fn_mask = *(u16 *)obj->buffer.pointer; > + break; > + case 1: > + intel_hid_dsm_fn_mask = *obj->buffer.pointer; > + break; > + case 0: > + acpi_handle_warn(handle, "intel_hid_dsm_fn_mask length is zero\n"); > + intel_hid_dsm_fn_mask = 0; > + break; > + } > ACPI_FREE(obj); > } >
diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index 86261970bd8f..9a52e56f75da 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -141,7 +141,7 @@ static bool intel_hid_execute_method(acpi_handle handle, method_name = (char *)intel_hid_dsm_fn_to_method[fn_index]; - if (!(intel_hid_dsm_fn_mask & fn_index)) + if (!(intel_hid_dsm_fn_mask & BIT(fn_index))) goto skip_dsm_exec; /* All methods expects a package with one integer element */ @@ -214,7 +214,19 @@ static void intel_hid_init_dsm(acpi_handle handle) obj = acpi_evaluate_dsm_typed(handle, &intel_dsm_guid, 1, 0, NULL, ACPI_TYPE_BUFFER); if (obj) { - intel_hid_dsm_fn_mask = *obj->buffer.pointer; + switch (obj->buffer.length) { + default: + case 2: + intel_hid_dsm_fn_mask = *(u16 *)obj->buffer.pointer; + break; + case 1: + intel_hid_dsm_fn_mask = *obj->buffer.pointer; + break; + case 0: + acpi_handle_warn(handle, "intel_hid_dsm_fn_mask length is zero\n"); + intel_hid_dsm_fn_mask = 0; + break; + } ACPI_FREE(obj); }
According to the ACPI spec 9.1.1 _DSM (Device Specific Method), intel_hid_dsm_fn_mask, acquired from function index 0, is "a buffer containing one bit for each function index". When validitaing fn_index, it should be compared with corresponding bit. This buffer is usually longer than a byte. Depending on whether INTEL_HID_DSM_HEBC_V2_FN exist, it could be either "Buffer (0x02) { 0xFF, 0x01 }" or "Buffer (0x02) { 0xFF, 0x03 }". Probably it won't grow larger according to the description. On older platforms, available functions could be fewer or not supported at all, i.e., "Buffer (One) { 0x00 }". Signed-off-by: Zhen Gong <zhengong@usc.edu> --- drivers/platform/x86/intel-hid.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)