On Fri, Mar 06, 2015 at 12:11:41AM -0600, Suravee Suthikulpanit wrote: > Device drivers typically use ACPI _HIDs/_CIDs listed in struct device_driver > acpi_match_table to match devices. However, for generic drivers, we do not > want to list _HID for all supported devices. Also, certain classes of devices > do not have _CID (e.g. SATA, USB). Instead, we can leverage ACPI _CLS, > which specifies PCI-defined class code (i.e. base-class, subclass and > programming interface). This patch adds support for matching ACPI devices using > the _CLS method. > > To support loadable module, current design uses _HID or _CID to match device's > modalias. With the new way of matching with _CLS this would requires modification > to the current ACPI modalias key to include _CLS. This patch appends PCI-defined > class-code to the existing ACPI modalias as following. > > acpi:<HID>:<CID1>:<CID2>:..:<CIDn>:<bbsspp>: > E.g: > # cat /sys/devices/platform/AMDI0600:00/modalias > acpi:AMDI0600:010601: > > where bb is th base-class code, ss is te sub-class code, and pp is the > programming interface code > > Since there would not be _HID/_CID in the ACPI matching table of the driver, > this patch adds a field to acpi_device_id to specify the matching _CLS. > > static const struct acpi_device_id ahci_acpi_match[] = { > { "", 0, PCI_CLASS_STORAGE_SATA_AHCI }, How about introducing macro like PCI already has and then do it like: { ACPI_DEVICE_CLASS(PCI_CLASS_STORAGE_SATA_AHCI) }, Also should we allow mask here as well? This would allow partial match if, for example we are only interested in class and subclass. > {}, > }; > > In this case, the corresponded entry in modules.alias file would be: > > alias acpi*:010601:* ahci_platform > > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx> > --- > drivers/acpi/acpica/acutils.h | 3 ++ > drivers/acpi/acpica/nsxfname.c | 21 ++++++++++-- > drivers/acpi/acpica/utids.c | 71 +++++++++++++++++++++++++++++++++++++++ I think you need to split the ACPICA part to be a separate patch. Those are merged through ACPICA. > drivers/acpi/scan.c | 17 ++++++++-- > include/acpi/acnames.h | 1 + > include/acpi/actypes.h | 4 ++- > include/linux/mod_devicetable.h | 1 + > scripts/mod/devicetable-offsets.c | 1 + > scripts/mod/file2alias.c | 13 +++++-- > 9 files changed, 124 insertions(+), 8 deletions(-) > > diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h > index c2f03e8..2aef850 100644 > --- a/drivers/acpi/acpica/acutils.h > +++ b/drivers/acpi/acpica/acutils.h > @@ -430,6 +430,9 @@ acpi_status > acpi_ut_execute_CID(struct acpi_namespace_node *device_node, > struct acpi_pnp_device_id_list ** return_cid_list); > > +acpi_status > +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node, > + struct acpi_pnp_device_id **return_id); > /* > * utlock - reader/writer locks > */ > diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c > index d66c326..590ef06 100644 > --- a/drivers/acpi/acpica/nsxfname.c > +++ b/drivers/acpi/acpica/nsxfname.c > @@ -276,11 +276,12 @@ acpi_get_object_info(acpi_handle handle, > struct acpi_pnp_device_id *hid = NULL; > struct acpi_pnp_device_id *uid = NULL; > struct acpi_pnp_device_id *sub = NULL; > + struct acpi_pnp_device_id *cls = NULL; > char *next_id_string; > acpi_object_type type; > acpi_name name; > u8 param_count = 0; > - u8 valid = 0; > + u16 valid = 0; > u32 info_size; > u32 i; > acpi_status status; > @@ -320,7 +321,7 @@ acpi_get_object_info(acpi_handle handle, > if ((type == ACPI_TYPE_DEVICE) || (type == ACPI_TYPE_PROCESSOR)) { > /* > * Get extra info for ACPI Device/Processor objects only: > - * Run the Device _HID, _UID, _SUB, and _CID methods. > + * Run the Device _HID, _UID, _SUB, _CID and _CLS methods. > * > * Note: none of these methods are required, so they may or may > * not be present for this device. The Info->Valid bitfield is used > @@ -351,6 +352,14 @@ acpi_get_object_info(acpi_handle handle, > valid |= ACPI_VALID_SUB; > } > > + /* Execute the Device._CLS method */ > + > + status = acpi_ut_execute_CLS(node, &cls); > + if (ACPI_SUCCESS(status)) { > + info_size += cls->length; > + valid |= ACPI_VALID_CLS; > + } > + > /* Execute the Device._CID method */ > > status = acpi_ut_execute_CID(node, &cid_list); > @@ -468,6 +477,11 @@ acpi_get_object_info(acpi_handle handle, > sub, next_id_string); > } > > + if (cls) { > + next_id_string = acpi_ns_copy_device_id(&info->cls, > + cls, next_id_string); > + } > + > if (cid_list) { > info->compatible_id_list.count = cid_list->count; > info->compatible_id_list.list_size = cid_list->list_size; > @@ -507,6 +521,9 @@ cleanup: > if (sub) { > ACPI_FREE(sub); > } > + if (cls) { > + ACPI_FREE(cls); > + } > if (cid_list) { > ACPI_FREE(cid_list); > } > diff --git a/drivers/acpi/acpica/utids.c b/drivers/acpi/acpica/utids.c > index 27431cf..a64b5d1 100644 > --- a/drivers/acpi/acpica/utids.c > +++ b/drivers/acpi/acpica/utids.c > @@ -416,3 +416,74 @@ cleanup: > acpi_ut_remove_reference(obj_desc); > return_ACPI_STATUS(status); > } > + > +/******************************************************************************* > + * > + * FUNCTION: acpi_ut_execute_CLS > + * > + * PARAMETERS: device_node - Node for the device > + * return_id - Where the string UID is returned > + * > + * RETURN: Status > + * > + * DESCRIPTION: Executes the _CLS control method that returns PCI-defined > + * class code of the device. The ACPI spec define _CLS as a > + * package with three integers. The returned string has format: > + * > + * "bbsspp" > + * where: > + * bb = Base-class code > + * ss = Sub-class code > + * pp = Programming Interface code > + * > + ******************************************************************************/ > + > +acpi_status > +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node, > + struct acpi_pnp_device_id **return_id) > +{ > + struct acpi_pnp_device_id *cls; > + union acpi_operand_object *obj_desc; > + union acpi_operand_object **cls_objects; > + acpi_status status; > + > + ACPI_FUNCTION_TRACE(ut_execute_CLS); > + status = acpi_ut_evaluate_object(device_node, METHOD_NAME__CLS, > + ACPI_BTYPE_PACKAGE, &obj_desc); > + if (ACPI_FAILURE(status)) > + return_ACPI_STATUS(status); > + > + cls_objects = obj_desc->package.elements; > + > + if (obj_desc->common.type == ACPI_TYPE_PACKAGE && > + obj_desc->package.count == 3 && > + cls_objects[0]->common.type == ACPI_TYPE_INTEGER && > + cls_objects[1]->common.type == ACPI_TYPE_INTEGER && > + cls_objects[2]->common.type == ACPI_TYPE_INTEGER) { > + > + /* Allocate a buffer for the CLS */ > + cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) + > + (acpi_size) 7); > + if (!cls) { > + status = AE_NO_MEMORY; > + goto cleanup; > + } > + > + cls->string = > + ACPI_ADD_PTR(char, cls, sizeof(struct acpi_pnp_device_id)); > + > + sprintf(cls->string, "%02x%02x%02x", > + (u8)ACPI_TO_INTEGER(cls_objects[0]->integer.value), > + (u8)ACPI_TO_INTEGER(cls_objects[1]->integer.value), > + (u8)ACPI_TO_INTEGER(cls_objects[2]->integer.value)); > + cls->length = 7; > + *return_id = cls; > + } > + > +cleanup: > + > + /* On exit, we must delete the return object */ > + > + acpi_ut_remove_reference(obj_desc); > + return_ACPI_STATUS(status); > +} > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index bbca783..f6ecbd1 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -907,10 +907,19 @@ static const struct acpi_device_id *__acpi_match_device( > if (!device->status.present) > return NULL; > > - for (id = ids; id->id[0]; id++) > - list_for_each_entry(hwid, &device->pnp.ids, list) > - if (!strcmp((char *) id->id, hwid->id)) > + for (id = ids; id->id[0] || id->cls; id++) { > + list_for_each_entry(hwid, &device->pnp.ids, list) { > + if (id->id[0] && !strcmp((char *) id->id, hwid->id)) { > return id; > + } else if (id->cls) { > + char buf[7]; > + > + sprintf(buf, "%06x", id->cls); > + if (!strcmp(buf, hwid->id)) > + return id; > + } > + } > + } > > return NULL; > } > @@ -1974,6 +1983,8 @@ static void acpi_set_pnp_ids(acpi_handle handle, struct acpi_device_pnp *pnp, > if (info->valid & ACPI_VALID_UID) > pnp->unique_id = kstrdup(info->unique_id.string, > GFP_KERNEL); > + if (info->valid & ACPI_VALID_CLS) > + acpi_add_id(pnp, info->cls.string); > > kfree(info); > > diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h > index 273de70..b52c0dc 100644 > --- a/include/acpi/acnames.h > +++ b/include/acpi/acnames.h > @@ -51,6 +51,7 @@ > #define METHOD_NAME__BBN "_BBN" > #define METHOD_NAME__CBA "_CBA" > #define METHOD_NAME__CID "_CID" > +#define METHOD_NAME__CLS "_CLS" > #define METHOD_NAME__CRS "_CRS" > #define METHOD_NAME__DDN "_DDN" > #define METHOD_NAME__HID "_HID" > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h > index b034f10..ab3dac8 100644 > --- a/include/acpi/actypes.h > +++ b/include/acpi/actypes.h > @@ -1148,7 +1148,7 @@ struct acpi_device_info { > u32 name; /* ACPI object Name */ > acpi_object_type type; /* ACPI object Type */ > u8 param_count; /* If a method, required parameter count */ > - u8 valid; /* Indicates which optional fields are valid */ > + u16 valid; /* Indicates which optional fields are valid */ > u8 flags; /* Miscellaneous info */ > u8 highest_dstates[4]; /* _sx_d values: 0xFF indicates not valid */ > u8 lowest_dstates[5]; /* _sx_w values: 0xFF indicates not valid */ > @@ -1157,6 +1157,7 @@ struct acpi_device_info { > struct acpi_pnp_device_id hardware_id; /* _HID value */ > struct acpi_pnp_device_id unique_id; /* _UID value */ > struct acpi_pnp_device_id subsystem_id; /* _SUB value */ > + struct acpi_pnp_device_id cls; /* _CLS value */ > struct acpi_pnp_device_id_list compatible_id_list; /* _CID list <must be last> */ > }; > > @@ -1174,6 +1175,7 @@ struct acpi_device_info { > #define ACPI_VALID_CID 0x20 > #define ACPI_VALID_SXDS 0x40 > #define ACPI_VALID_SXWS 0x80 > +#define ACPI_VALID_CLS 0x100 > > /* Flags for _STA return value (current_status above) */ > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h > index e530533..9a42522 100644 > --- a/include/linux/mod_devicetable.h > +++ b/include/linux/mod_devicetable.h > @@ -189,6 +189,7 @@ struct css_device_id { > struct acpi_device_id { > __u8 id[ACPI_ID_LEN]; > kernel_ulong_t driver_data; > + __u32 cls; It would be nice if we could change ordering here but I understand that it breaks quite many drivers. Perhaps we should consider creating ACPI_DEVICE() macro and convert existing drivers to that at some point. > }; > > #define PNP_ID_LEN 8 > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > index f282516..7f68268 100644 > --- a/scripts/mod/devicetable-offsets.c > +++ b/scripts/mod/devicetable-offsets.c > @@ -63,6 +63,7 @@ int main(void) > > DEVID(acpi_device_id); > DEVID_FIELD(acpi_device_id, id); > + DEVID_FIELD(acpi_device_id, cls); > > DEVID(pnp_device_id); > DEVID_FIELD(pnp_device_id, id); > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > index e614ef6..ba5998c 100644 > --- a/scripts/mod/file2alias.c > +++ b/scripts/mod/file2alias.c > @@ -511,12 +511,21 @@ static int do_serio_entry(const char *filename, > } > ADD_TO_DEVTABLE("serio", serio_device_id, do_serio_entry); > > -/* looks like: "acpi:ACPI0003 or acpi:PNP0C0B" or "acpi:LNXVIDEO" */ > +/* looks like: "acpi:ACPI0003" or "acpi:PNP0C0B" or "acpi:LNXVIDEO" or > + * "acpi:bbsspp" (bb=base-class, ss=sub-class, pp=prog-if) > + * > + * NOTE: * Each driver should use one of the following : _HID, _CIDs or _CLS. > + */ > static int do_acpi_entry(const char *filename, > void *symval, char *alias) > { > DEF_FIELD_ADDR(symval, acpi_device_id, id); > - sprintf(alias, "acpi*:%s:*", *id); > + DEF_FIELD_ADDR(symval, acpi_device_id, cls); > + > + if (id && strlen((const char *)*id)) > + sprintf(alias, "acpi*:%s:*", *id); > + else if (cls) > + sprintf(alias, "acpi*:%06x:*", *cls); > return 1; > } > ADD_TO_DEVTABLE("acpi", acpi_device_id, do_acpi_entry); > -- > 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html