| Message ID | 1517388005-14852-1-git-send-email-alex.hung@canonical.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Alex, On Wed, 31 Jan 2018 00:40:04 -0800, Alex Hung wrote: > OEM strings are defined by each OEM and they contain customized and > useful OEM information. Supporting it provides more flexible uses of > the dmi_matches function. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > drivers/firmware/dmi_scan.c | 8 ++++++++ > include/linux/mod_devicetable.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c > index 7830419..e534d1b 100644 > --- a/drivers/firmware/dmi_scan.c > +++ b/drivers/firmware/dmi_scan.c > @@ -797,11 +797,19 @@ static bool dmi_matches(const struct dmi_system_id *dmi) > else if (dmi->matches[i].exact_match && > !strcmp(dmi_ident[s], dmi->matches[i].substr)) > continue; > + } else if (s == DMI_OEM_STRING) { > + const struct dmi_device *valid; > + > + valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, > + dmi->matches[i].substr, NULL); > + if (valid) > + continue; > } > > /* No match */ > return false; > } > + > return true; > } > > diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h > index abb6dc2..5739c4c 100644 > --- a/include/linux/mod_devicetable.h > +++ b/include/linux/mod_devicetable.h > @@ -482,6 +482,7 @@ enum dmi_field { > DMI_CHASSIS_VERSION, > DMI_CHASSIS_SERIAL, > DMI_CHASSIS_ASSET_TAG, > + DMI_OEM_STRING, > DMI_STRING_MAX, > }; > This is kind of a hack, because there are more than one OEM string, so they don't fit in dmi_ident[], but I see the value. However, reserving one entry for them in dmi_ident[], which you will never use, is potentially confusing, so it would have to be documented. Did you consider adding DMI_OEM_STRING after DMI_STRING_MAX? It would avoid the memory waste (small but still) and shouldn't be a problem if you test this specific case early enough in dmi_matches()? It should also be documented that only exact matches are supported for DMI_OEM_STRING (dmi_strmatch.exact_match is ignored and considered true always.) Lastly you will have to rebase your patch on top of Linus' latest tree and in particular: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cf4e6a04f734e831c2ac7f405071d1cde690ba8 Thanks,
On Mon, Feb 5, 2018 at 2:26 AM, Jean Delvare <jdelvare@suse.de> wrote: > Hi Alex, > > On Wed, 31 Jan 2018 00:40:04 -0800, Alex Hung wrote: >> OEM strings are defined by each OEM and they contain customized and >> useful OEM information. Supporting it provides more flexible uses of >> the dmi_matches function. >> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> drivers/firmware/dmi_scan.c | 8 ++++++++ >> include/linux/mod_devicetable.h | 1 + >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c >> index 7830419..e534d1b 100644 >> --- a/drivers/firmware/dmi_scan.c >> +++ b/drivers/firmware/dmi_scan.c >> @@ -797,11 +797,19 @@ static bool dmi_matches(const struct dmi_system_id *dmi) >> else if (dmi->matches[i].exact_match && >> !strcmp(dmi_ident[s], dmi->matches[i].substr)) >> continue; >> + } else if (s == DMI_OEM_STRING) { >> + const struct dmi_device *valid; >> + >> + valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, >> + dmi->matches[i].substr, NULL); >> + if (valid) >> + continue; >> } >> >> /* No match */ >> return false; >> } >> + >> return true; >> } >> >> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h >> index abb6dc2..5739c4c 100644 >> --- a/include/linux/mod_devicetable.h >> +++ b/include/linux/mod_devicetable.h >> @@ -482,6 +482,7 @@ enum dmi_field { >> DMI_CHASSIS_VERSION, >> DMI_CHASSIS_SERIAL, >> DMI_CHASSIS_ASSET_TAG, >> + DMI_OEM_STRING, >> DMI_STRING_MAX, >> }; >> > > This is kind of a hack, because there are more than one OEM string, so > they don't fit in dmi_ident[], but I see the value. However, reserving > one entry for them in dmi_ident[], which you will never use, is > potentially confusing, so it would have to be documented. > > Did you consider adding DMI_OEM_STRING after DMI_STRING_MAX? It would > avoid the memory waste (small but still) and shouldn't be a problem if > you test this specific case early enough in dmi_matches()? > > It should also be documented that only exact matches are supported for > DMI_OEM_STRING (dmi_strmatch.exact_match is ignored and considered true > always.) It is a good idea. I will refine, test and submit a V2. > > Lastly you will have to rebase your patch on top of Linus' latest tree > and in particular: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cf4e6a04f734e831c2ac7f405071d1cde690ba8 Will do > > Thanks, > -- > Jean Delvare > SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index 7830419..e534d1b 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -797,11 +797,19 @@ static bool dmi_matches(const struct dmi_system_id *dmi) else if (dmi->matches[i].exact_match && !strcmp(dmi_ident[s], dmi->matches[i].substr)) continue; + } else if (s == DMI_OEM_STRING) { + const struct dmi_device *valid; + + valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, + dmi->matches[i].substr, NULL); + if (valid) + continue; } /* No match */ return false; } + return true; } diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index abb6dc2..5739c4c 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -482,6 +482,7 @@ enum dmi_field { DMI_CHASSIS_VERSION, DMI_CHASSIS_SERIAL, DMI_CHASSIS_ASSET_TAG, + DMI_OEM_STRING, DMI_STRING_MAX, };
OEM strings are defined by each OEM and they contain customized and useful OEM information. Supporting it provides more flexible uses of the dmi_matches function. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- drivers/firmware/dmi_scan.c | 8 ++++++++ include/linux/mod_devicetable.h | 1 + 2 files changed, 9 insertions(+)