On 2015/3/26 18:29, Mika Westerberg wrote: > On Wed, Mar 25, 2015 at 03:46:07PM -0500, 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[] = { >> { ACPI_DEVICE_CLASS(PCI_CLASS_STORAGE_SATA_AHCI, 0xFFFFFF) }, >> {}, >> }; >> >> 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/scan.c | 39 +++++++++++++++++++++++++++++++++++---- >> include/acpi/acnames.h | 1 + >> include/acpi/actypes.h | 4 +++- >> include/linux/mod_devicetable.h | 4 ++++ >> scripts/mod/devicetable-offsets.c | 2 ++ >> scripts/mod/file2alias.c | 32 ++++++++++++++++++++++++++++++-- >> 6 files changed, 75 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >> index 52a62aa..f26bf80 100644 >> --- a/drivers/acpi/scan.c >> +++ b/drivers/acpi/scan.c >> @@ -895,6 +895,32 @@ static void acpi_device_remove_files(struct acpi_device *dev) >> ACPI Bus operations >> -------------------------------------------------------------------------- */ >> >> +static bool __acpi_match_device_cls(const struct acpi_device_id *id, >> + struct acpi_hardware_id *hwid) >> +{ >> + int i, msk, byte_shift; >> + bool found = true; >> + char buf[3]; >> + >> + if (!id->cls) >> + return false; >> + >> + /* Apply class-code bitmask, before checking each class-code byte */ >> + for (i = 1; i <= 3; i++) { >> + byte_shift = 8 * (3 - i); >> + msk = (id->cls_msk >> byte_shift) & 0xFF; >> + if (!msk) >> + continue; >> + >> + sprintf(buf, "%02x", (id->cls >> byte_shift) & msk); >> + if (strncmp(buf, &hwid->id[(i - 1) * 2], 2)) { >> + found = false; >> + break; > You can return false directly here. > >> + } >> + } >> + return found; >> +} >> + >> static const struct acpi_device_id *__acpi_match_device( >> struct acpi_device *device, const struct acpi_device_id *ids) >> { >> @@ -908,11 +934,14 @@ 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 (__acpi_match_device_cls(id, hwid)) >> + return id; >> + } >> + } >> return NULL; >> } >> >> @@ -2005,6 +2034,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..9563abe 100644 >> --- a/include/linux/mod_devicetable.h >> +++ b/include/linux/mod_devicetable.h >> @@ -189,8 +189,12 @@ struct css_device_id { >> struct acpi_device_id { >> __u8 id[ACPI_ID_LEN]; >> kernel_ulong_t driver_data; >> + __u32 cls; >> + __u32 cls_msk; >> }; >> >> +#define ACPI_DEVICE_CLASS(cls, msk) "", 0, cls, msk > Consider moving this to <linux/acpi.h>, just like PCI_DEVICE_CLASS() is > defined in <linux/pci.h>. > > Also please use designated initializers here, eg: > > #define ACPI_DEVICE_CLASS(cls, msk) .cls = (cls), .cls_mask = (msk) I agree. > > Once done you can add my, > > Acked-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> Please add my Reviewed-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx> too. Thanks Hanjun -- 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