[Precise][Patch 1/1] samsung-laptop: make the dmi check less strict
Joseph Salisbury
joseph.salisbury at canonical.com
Fri Jun 22 18:55:16 UTC 2012
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 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).
Ok, thanks. I'll make a note of your comments for next time.
>
>>
>> Thanks,
>>
>> Joe
>>
>
More information about the kernel-team
mailing list