[Precise][Patch 1/1] samsung-laptop: make the dmi check less strict
Herton Ronaldo Krzesinski
herton.krzesinski at canonical.com
Fri Jun 22 18:44:25 UTC 2012
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 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
> >>
> >
>
> 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
>
--
[]'s
Herton
More information about the kernel-team
mailing list