| Message ID | 90827a6af24bc8cfe09ca079f8e70eda681fffa8.1340380967.git.joseph.salisbury@canonical.com |
|---|---|
| State | New |
| Headers | show |
Hi, just some nits, and my opinion: On Fri, Jun 22, 2012 at 12:41:40PM -0400, joseph.salisbury@canonical.com wrote: > From: Corentin Chary <corentincj@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@iksaif.net> > Acked-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Matthew Garrett <mjg@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@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@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team >
On 06/22/2012 01:51 PM, Herton Ronaldo Krzesinski wrote: > Hi, just some nits, and my opinion: > > On Fri, Jun 22, 2012 at 12:41:40PM -0400, joseph.salisbury@canonical.com wrote: >> From: Corentin Chary <corentincj@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@iksaif.net> >> Acked-by: Greg Kroah-Hartman <gregkh@suse.de> >> Signed-off-by: Matthew Garrett <mjg@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@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@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team >> > Thanks for the feedback, Herton. Do you want me to fix up the things you pointed out and re-submit the patch to the list? Thanks, Joe
On Fri, Jun 22, 2012 at 02:38:19PM -0400, Joseph Salisbury wrote: > On 06/22/2012 01:51 PM, Herton Ronaldo Krzesinski wrote: > > Hi, just some nits, and my opinion: > > > > On Fri, Jun 22, 2012 at 12:41:40PM -0400, joseph.salisbury@canonical.com wrote: > >> From: Corentin Chary <corentincj@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@iksaif.net> > >> Acked-by: Greg Kroah-Hartman <gregkh@suse.de> > >> Signed-off-by: Matthew Garrett <mjg@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@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@lists.ubuntu.com > >> https://lists.ubuntu.com/mailman/listinfo/kernel-team > >> > > > > Thanks for the feedback, Herton. Do you want me to fix up the things > you pointed out and re-submit the patch to the list? No, I think probably who is going to apply will fix it (Tim). > > Thanks, > > Joe >
On 06/22/2012 02:44 PM, Herton Ronaldo Krzesinski wrote: > On Fri, Jun 22, 2012 at 02:38:19PM -0400, Joseph Salisbury wrote: >> On 06/22/2012 01:51 PM, Herton Ronaldo Krzesinski wrote: >>> Hi, just some nits, and my opinion: >>> >>> On Fri, Jun 22, 2012 at 12:41:40PM -0400, joseph.salisbury@canonical.com wrote: >>>> From: Corentin Chary <corentincj@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@iksaif.net> >>>> Acked-by: Greg Kroah-Hartman <gregkh@suse.de> >>>> Signed-off-by: Matthew Garrett <mjg@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@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@lists.ubuntu.com >>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team >>>> >>> >> >> Thanks for the feedback, Herton. Do you want me to fix up the things >> you pointed out and re-submit the patch to the list? > > No, I think probably who is going to apply will fix it (Tim). Ok, thanks. I'll make a note of your comments for next time. > >> >> Thanks, >> >> Joe >> >
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, }, { }, };