Skip to content

Commit c16c205

Browse files
committed
ACPI: button: Rework device verification during probe
Instead of manually comparing the primary ID of the device (retuned by _HID) with each of the device IDs supported by the driver, use acpi_match_acpi_device() (which includes the ACPI companion device pointer check against NULL) and store the ACPI button type as driver_data in button_device_ids[], which allows a multi-branch conditional statement to be replaced with a switch () one. However, to continue preventing successful probing of devices that only have one of the supported device IDs in their _CID lists, compare the matched device ID with the primary ID of the device and return an error if they don't match. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Link: https://patch.msgid.link/7960518.EvYhyI6sBW@rafael.j.wysocki [ rjw: Fixed button memory leak on probe failure ] Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
1 parent 1ad8cdd commit c16c205

1 file changed

Lines changed: 41 additions & 37 deletions

File tree

drivers/acpi/button.c

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ MODULE_DESCRIPTION("ACPI Button Driver");
5959
MODULE_LICENSE("GPL");
6060

6161
static const struct acpi_device_id button_device_ids[] = {
62-
{ACPI_BUTTON_HID_LID, 0},
63-
{ACPI_BUTTON_HID_SLEEP, 0},
64-
{ACPI_BUTTON_HID_SLEEPF, 0},
65-
{ACPI_BUTTON_HID_POWER, 0},
66-
{ACPI_BUTTON_HID_POWERF, 0},
62+
{ACPI_BUTTON_HID_LID, ACPI_BUTTON_TYPE_LID},
63+
{ACPI_BUTTON_HID_SLEEP, ACPI_BUTTON_TYPE_SLEEP},
64+
{ACPI_BUTTON_HID_SLEEPF, ACPI_BUTTON_TYPE_SLEEP},
65+
{ACPI_BUTTON_HID_POWER, ACPI_BUTTON_TYPE_POWER},
66+
{ACPI_BUTTON_HID_POWERF, ACPI_BUTTON_TYPE_POWER},
6767
{"", 0},
6868
};
6969
MODULE_DEVICE_TABLE(acpi, button_device_ids);
@@ -542,22 +542,23 @@ static int acpi_lid_input_open(struct input_dev *input)
542542
static int acpi_button_probe(struct platform_device *pdev)
543543
{
544544
struct device *dev = &pdev->dev;
545+
struct acpi_device *device = ACPI_COMPANION(dev);
546+
const struct acpi_device_id *id;
545547
acpi_notify_handler handler;
546-
struct acpi_device *device;
547548
struct acpi_button *button;
548549
struct input_dev *input;
549550
acpi_status status;
550551
char *name, *class;
551-
const char *hid;
552+
u8 button_type;
552553
int error = 0;
553554

554-
device = ACPI_COMPANION(dev);
555-
if (!device)
556-
return -ENODEV;
555+
id = acpi_match_acpi_device(button_device_ids, device);
556+
if (!id || strcmp(acpi_device_hid(device), id->id))
557+
return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
557558

558-
hid = acpi_device_hid(device);
559-
if (!strcmp(hid, ACPI_BUTTON_HID_LID) &&
560-
lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)
559+
button_type = id->driver_data;
560+
if (button_type == ACPI_BUTTON_TYPE_LID &&
561+
lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)
561562
return -ENODEV;
562563

563564
button = kzalloc_obj(struct acpi_button);
@@ -568,57 +569,60 @@ static int acpi_button_probe(struct platform_device *pdev)
568569

569570
button->dev = dev;
570571
button->adev = device;
571-
button->input = input = input_allocate_device();
572+
input = input_allocate_device();
572573
if (!input) {
573574
error = -ENOMEM;
574575
goto err_free_button;
575576
}
577+
button->input = input;
578+
button->type = button_type;
576579

577580
class = acpi_device_class(device);
578581

579-
if (!strcmp(hid, ACPI_BUTTON_HID_POWER) ||
580-
!strcmp(hid, ACPI_BUTTON_HID_POWERF)) {
581-
button->type = ACPI_BUTTON_TYPE_POWER;
582+
switch (button_type) {
583+
case ACPI_BUTTON_TYPE_LID:
584+
handler = acpi_lid_notify;
585+
name = ACPI_BUTTON_DEVICE_NAME_LID;
586+
sprintf(class, "%s/%s",
587+
ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
588+
input->open = acpi_lid_input_open;
589+
break;
590+
591+
case ACPI_BUTTON_TYPE_POWER:
582592
handler = acpi_button_notify;
583593
name = ACPI_BUTTON_DEVICE_NAME_POWER;
584594
sprintf(class, "%s/%s",
585595
ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
586-
} else if (!strcmp(hid, ACPI_BUTTON_HID_SLEEP) ||
587-
!strcmp(hid, ACPI_BUTTON_HID_SLEEPF)) {
588-
button->type = ACPI_BUTTON_TYPE_SLEEP;
596+
break;
597+
598+
case ACPI_BUTTON_TYPE_SLEEP:
589599
handler = acpi_button_notify;
590600
name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
591601
sprintf(class, "%s/%s",
592602
ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_SLEEP);
593-
} else if (!strcmp(hid, ACPI_BUTTON_HID_LID)) {
594-
button->type = ACPI_BUTTON_TYPE_LID;
595-
handler = acpi_lid_notify;
596-
name = ACPI_BUTTON_DEVICE_NAME_LID;
597-
sprintf(class, "%s/%s",
598-
ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
599-
input->open = acpi_lid_input_open;
600-
} else {
601-
pr_info("Unsupported hid [%s]\n", hid);
602-
error = -ENODEV;
603-
}
603+
break;
604604

605-
if (!error)
606-
error = acpi_button_add_fs(button);
605+
default:
606+
input_free_device(input);
607+
error = dev_err_probe(dev, -ENODEV, "Unrecognized button type\n");
608+
goto err_free_button;
609+
}
607610

611+
error = acpi_button_add_fs(button);
608612
if (error) {
609613
input_free_device(input);
610614
goto err_free_button;
611615
}
612616

613-
snprintf(button->phys, sizeof(button->phys), "%s/button/input0", hid);
617+
snprintf(button->phys, sizeof(button->phys), "%s/button/input0", id->id);
614618

615619
input->name = name;
616620
input->phys = button->phys;
617621
input->id.bustype = BUS_HOST;
618-
input->id.product = button->type;
622+
input->id.product = button_type;
619623
input->dev.parent = dev;
620624

621-
switch (button->type) {
625+
switch (button_type) {
622626
case ACPI_BUTTON_TYPE_POWER:
623627
input_set_capability(input, EV_KEY, KEY_POWER);
624628
input_set_capability(input, EV_KEY, KEY_WAKEUP);
@@ -679,7 +683,7 @@ static int acpi_button_probe(struct platform_device *pdev)
679683
goto err_input_unregister;
680684
}
681685

682-
if (button->type == ACPI_BUTTON_TYPE_LID) {
686+
if (button_type == ACPI_BUTTON_TYPE_LID) {
683687
/*
684688
* This assumes there's only one lid device, or if there are
685689
* more we only care about the last one...

0 commit comments

Comments
 (0)