[Precise][Patch 1/1] samsung-laptop: make the dmi check less strict
Herton Ronaldo Krzesinski
herton.krzesinski at canonical.com
Fri Jun 22 17:51:03 UTC 2012
Hi, just some nits, and my opinion:
On Fri, Jun 22, 2012 at 12:41:40PM -0400, joseph.salisbury at canonical.com wrote:
> From: Corentin Chary <corentincj at iksaif.net>
>
> BugLink: http://bugs.launchpad.net/bugs/1012284
> (backported from commit 3be324a94df0c3f032178d04549dbfbf6cccb09a)
Move this backport line below, to the sign off area, seems better.
>
> This enable the driver for everything that look like
> a laptop and is from vendor "SAMSUNG ELECTRONICS CO., LTD.".
> Note that laptop supported by samsung-q10 seem to have a different
> vendor strict.
>
> Also remove every log output until we know that we have a SABI interface
> (except if the driver is forced to load, or debug is enabled).
>
> Keeping a whitelist of laptop with a model granularity is something that can't
> work without close vendor cooperation (and we don't have that).
>
> Signed-off-by: Corentin Chary <corentincj at iksaif.net>
> Acked-by: Greg Kroah-Hartman <gregkh at suse.de>
> Signed-off-by: Matthew Garrett <mjg at redhat.com>
> (cherry picked from commit 3be324a94df0c3f032178d04549dbfbf6cccb09a)
Remove this cherry picked line, and no need I think to keep Conflicts
information below.
>
> Conflicts:
>
> drivers/platform/x86/samsung-laptop.c
>
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
Ok so what the patch does seems reasonable. For precise we don't need
really the log output changes. And instead of continuing with an ever
growing whitelist, it just checks for the vendor string and filters
through dmi chassis type.
This change in DMI filter could have a potential for regression, eg.
triggering samsung-laptop to load or not load anymore if there is some
samsung machine with botched dmi info, related to the chassis type
(which probably is unlikely to happen...). But as the potential is
isolated to samsung machines with vendor="SAMSUNG ELECTRONICS CO., LTD."
and this specific driver, also being succesfuly tested by the bug
reporter, I would say it's fine to have it, Ack.
> ---
> drivers/platform/x86/samsung-laptop.c | 230 +--------------------------------
> 1 file changed, 4 insertions(+), 226 deletions(-)
>
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index 8c021cf..c351309 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -539,256 +539,34 @@ static ssize_t set_performance_level(struct device *dev,
> static DEVICE_ATTR(performance_level, S_IWUSR | S_IRUGO,
> get_performance_level, set_performance_level);
>
> -
> -static int __init dmi_check_cb(const struct dmi_system_id *id)
> -{
> - pr_info("found laptop model '%s'\n",
> - id->ident);
> - return 1;
> -}
> -
> static struct dmi_system_id __initdata samsung_dmi_table[] = {
> {
> - .ident = "N128",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N128"),
> - DMI_MATCH(DMI_BOARD_NAME, "N128"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "N130",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N130"),
> - DMI_MATCH(DMI_BOARD_NAME, "N130"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "N510",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N510"),
> - DMI_MATCH(DMI_BOARD_NAME, "N510"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "N150P",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N150P"),
> - DMI_MATCH(DMI_BOARD_NAME, "N150P"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "X125",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "X125"),
> - DMI_MATCH(DMI_BOARD_NAME, "X125"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "X120/X170",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "X120/X170"),
> - DMI_MATCH(DMI_BOARD_NAME, "X120/X170"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "NC10",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "NC10"),
> - DMI_MATCH(DMI_BOARD_NAME, "NC10"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "NP-Q45",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "SQ45S70S"),
> - DMI_MATCH(DMI_BOARD_NAME, "SQ45S70S"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "X360",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "X360"),
> - DMI_MATCH(DMI_BOARD_NAME, "X360"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R410 Plus",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R410P"),
> - DMI_MATCH(DMI_BOARD_NAME, "R460"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R518",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R518"),
> - DMI_MATCH(DMI_BOARD_NAME, "R518"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R519/R719",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R519/R719"),
> - DMI_MATCH(DMI_BOARD_NAME, "R519/R719"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "N150/N210/N220",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR,
> - "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N150/N210/N220"),
> - DMI_MATCH(DMI_BOARD_NAME, "N150/N210/N220"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "N220",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR,
> "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N220"),
> - DMI_MATCH(DMI_BOARD_NAME, "N220"),
> + DMI_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
> },
> - .callback = dmi_check_cb,
> },
> {
> - .ident = "N150/N210/N220/N230",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR,
> "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N150/N210/N220/N230"),
> - DMI_MATCH(DMI_BOARD_NAME, "N150/N210/N220/N230"),
> + DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
> },
> - .callback = dmi_check_cb,
> },
> {
> - .ident = "N150P/N210P/N220P",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR,
> "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N150P/N210P/N220P"),
> - DMI_MATCH(DMI_BOARD_NAME, "N150P/N210P/N220P"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R700",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "SR700"),
> - DMI_MATCH(DMI_BOARD_NAME, "SR700"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R530/R730",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R530/R730"),
> - DMI_MATCH(DMI_BOARD_NAME, "R530/R730"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "NF110/NF210/NF310",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "NF110/NF210/NF310"),
> - DMI_MATCH(DMI_BOARD_NAME, "NF110/NF210/NF310"),
> + DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
> },
> - .callback = dmi_check_cb,
> },
> {
> - .ident = "N145P/N250P/N260P",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "N145P/N250P/N260P"),
> - DMI_MATCH(DMI_BOARD_NAME, "N145P/N250P/N260P"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R70/R71",
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR,
> "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R70/R71"),
> - DMI_MATCH(DMI_BOARD_NAME, "R70/R71"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "P460",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "P460"),
> - DMI_MATCH(DMI_BOARD_NAME, "P460"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "R528/R728",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "R528/R728"),
> - DMI_MATCH(DMI_BOARD_NAME, "R528/R728"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "NC210/NC110",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "NC210/NC110"),
> - DMI_MATCH(DMI_BOARD_NAME, "NC210/NC110"),
> - },
> - .callback = dmi_check_cb,
> - },
> - {
> - .ident = "X520",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_PRODUCT_NAME, "X520"),
> - DMI_MATCH(DMI_BOARD_NAME, "X520"),
> + DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
> },
> - .callback = dmi_check_cb,
> },
> { },
> };
> --
> 1.7.9.5
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
--
[]'s
Herton
More information about the kernel-team
mailing list