Skip to contentSkip to conversation

compal-laptop/fujitsu-laptop/msi-laptop: make dmi_check_cb to return 1 instead of 0

PATCH In review Axel Lin 2 participants 4 messages Jul 2, 2010 – Jul 6, 2010

Subsystems (4)
x86 platform drivers Fujitsu laptop extras Compal laptop support MSI laptop support
lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 compal-laptop/fujitsu-laptop/msi-laptop: make dmi_check_cb to return 1 instead of 0

dmi_check_system() walks the table running matching functions until someone returns non zero or we hit the end.

This patch makes dmi_check_cb to return 1 so dmi_check_system() return immediately when a match is found.

Signed-off-by Axel Lin <[email protected]>
---
 drivers/platform/x86/compal-laptop.c  |    2 +-
 drivers/platform/x86/fujitsu-laptop.c |    6 +++---
 drivers/platform/x86/msi-laptop.c     |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/platform/x86/compal-laptop.c +1 -1
index 71ff154..dc31a08 100644
@@ -190,7 +190,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 	printk(KERN_INFO "compal-laptop: Identified laptop model '%s'.\n",
 		id->ident);
 
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata compal_dmi_table[] = {
drivers/platform/x86/fujitsu-laptop.c +3 -3
index dab1a0e..1859d7f 100644
@@ -603,7 +603,7 @@ static int dmi_check_cb_s6410(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_s6420(const struct dmi_system_id *id)

@@ -611,7 +611,7 @@ static int dmi_check_cb_s6420(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_p8010(const struct dmi_system_id *id)

@@ -620,7 +620,7 @@ static int dmi_check_cb_p8010(const struct dmi_system_id *id)
 	fujitsu->keycode1 = KEY_HELP;	/* "Support" */
 	fujitsu->keycode3 = KEY_SWITCHVIDEOMODE;	/* "Presentation" */
 	fujitsu->keycode4 = KEY_WWW;	/* "Internet" */
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id fujitsu_dmi_table[] = {
drivers/platform/x86/msi-laptop.c +1 -1
index afd762b..c67a74c 100644
@@ -434,7 +434,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 {
 	printk(KERN_INFO "msi-laptop: Identified laptop model '%s'.\n",
 	       id->ident);
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata msi_dmi_table[] = {
Signature · 1 line
1.5.4.3
#1 lore raw

Re: [PATCH] compal-laptop/fujitsu-laptop/msi-laptop: make dmi_check_cb

Is there no possibility of some other matching function existing after ours in the tree which might need to also be called? If so, this change would prevent that.

Regards
jonathan

Quoted 75 lines

dmi_check_system() walks the table running matching functions until someone returns non zero or we hit the end.

This patch makes dmi_check_cb to return 1 so dmi_check_system() return immediately when a match is found.

Signed-off-by Axel Lin <[email protected]>
---
 drivers/platform/x86/compal-laptop.c  |    2 +-
 drivers/platform/x86/fujitsu-laptop.c |    6 +++---
 drivers/platform/x86/msi-laptop.c     |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/platform/x86/compal-laptop.c +1 -1
index 71ff154..dc31a08 100644
@@ -190,7 +190,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 	printk(KERN_INFO "compal-laptop: Identified laptop model '%s'.\n",
 		id->ident);
 
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata compal_dmi_table[] = {
drivers/platform/x86/fujitsu-laptop.c +3 -3
index dab1a0e..1859d7f 100644
@@ -603,7 +603,7 @@ static int dmi_check_cb_s6410(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_s6420(const struct dmi_system_id *id)

@@ -611,7 +611,7 @@ static int dmi_check_cb_s6420(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_p8010(const struct dmi_system_id *id)

@@ -620,7 +620,7 @@ static int dmi_check_cb_p8010(const struct dmi_system_id *id)
 	fujitsu->keycode1 = KEY_HELP;	/* "Support" */
 	fujitsu->keycode3 = KEY_SWITCHVIDEOMODE;	/* "Presentation" */
 	fujitsu->keycode4 = KEY_WWW;	/* "Internet" */
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id fujitsu_dmi_table[] = {
drivers/platform/x86/msi-laptop.c +1 -1
index afd762b..c67a74c 100644
@@ -434,7 +434,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 {
 	printk(KERN_INFO "msi-laptop: Identified laptop model '%s'.\n",
 	       id->ident);
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata msi_dmi_table[] = {
Signature · 1 line
1.5.4.3
Signature · 6 lines
* Jonathan Woithe    [email protected]                        *
*                    http://www.physics.adelaide.edu.au/~jwoithe            *
***-----------------------------------------------------------------------***
** "Time is an illusion; lunchtime doubly so"                              **
*  "...you wouldn't recognize a subtle plan if it painted itself purple and *
*   danced naked on a harpsichord singing 'subtle plans are here again'"    *
author
#2 lore raw

Re: [PATCH] compal-laptop/fujitsu-laptop/msi-laptop: make dmi_check_cb

2010/7/5 Jonathan Woithe <[email protected]>:

Quoted 3 lines

Is there no possibility of some other matching function existing after ours in the tree which might need to also be called?  If so, this change would prevent that.

hi Jonathan,

I checked the implementation of dmi_check_system() and the useage cases in compal-laptop/fujitsu-laptop/msi-laptop, it should be no problem with this change.

In compal-laptop and msi-laptop, we call dmi_check_system() to identify the laptop model.
(The callback simply print the identified laptop model) It should have zero or exactly one successful match.

In fujitsu-laptop, we call dmi_check_system() to identify the laptop model and set parameters accordingly.
The callback print the identified laptop model and set parameters (use_alt_lcd_levels / fujitsu->keycode[1|2|3|4|5] ).
It should have zero or exactly one successful match.
If it has some other matching function with successful match, then current implementation
is buggy anyway because new successful matches will override the fujitsu->keycode[1|2|3|4|5]
parameters.

Regards,
Axel

Quoted 89 lines

Regards
 jonathan

Quoted 71 lines

dmi_check_system() walks the table running matching functions until someone returns non zero or we hit the end.

This patch makes dmi_check_cb to return 1 so dmi_check_system() return immediately when a match is found.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/platform/x86/compal-laptop.c  |    2 +- drivers/platform/x86/fujitsu-laptop.c |    6 +++--- drivers/platform/x86/msi-laptop.c     |    2 +-

 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/platform/x86/compal-laptop.c +1 -1
index 71ff154..dc31a08 100644
@@ -190,7 +190,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
       printk(KERN_INFO "compal-laptop: Identified laptop model '%s'.\n",
               id->ident);
 
-     return 0;
+     return 1;
  }
 
  static struct dmi_system_id __initdata compal_dmi_table[] = {
drivers/platform/x86/fujitsu-laptop.c +0 -0
index dab1a0e..1859d7f 100644
@@ -603,7 +603,7 @@ static int dmi_check_cb_s6410(const struct dmi_system_id *id)
       dmi_check_cb_common(id);

      fujitsu->keycode1 = KEY_SCREENLOCK;     /* "Lock" */ fujitsu->keycode2 = KEY_HELP;   /* "Mobility Center" */
-     return 0;
+     return 1;
 }

 static int dmi_check_cb_s6420(const struct dmi_system_id *id)

drivers/platform/x86/fujitsu-laptop.c +0 -0
@@ -611,7 +611,7 @@ static int dmi_check_cb_s6420(const struct dmi_system_id *id)
       dmi_check_cb_common(id);

      fujitsu->keycode1 = KEY_SCREENLOCK;     /* "Lock" */ fujitsu->keycode2 = KEY_HELP;   /* "Mobility Center" */
-     return 0;
+     return 1;
 }

 static int dmi_check_cb_p8010(const struct dmi_system_id *id)

drivers/platform/x86/fujitsu-laptop.c +0 -0
@@ -620,7 +620,7 @@ static int dmi_check_cb_p8010(const struct dmi_system_id *id)

      fujitsu->keycode1 = KEY_HELP;   /* "Support" */ fujitsu->keycode3 = KEY_SWITCHVIDEOMODE;        /* "Presentation" */ fujitsu->keycode4 = KEY_WWW;    /* "Internet" */
-     return 0;
+     return 1;
 }

 static struct dmi_system_id fujitsu_dmi_table[] = {

drivers/platform/x86/msi-laptop.c +1 -2
index afd762b..c67a74c 100644
@@ -434,7 +434,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
  {
       printk(KERN_INFO "msi-laptop: Identified laptop model '%s'.\n",
              id->ident);
-     return 0;
+     return 1;
  }
 
  static struct dmi_system_id __initdata msi_dmi_table[] = {
--
Quoted 4 lines

1.5.4.3

--
* Jonathan Woithe    [email protected]                        *
*                    http://www.physics.adelaide.edu.au/~jwoithe            *
***-----------------------------------------------------------------------***
** "Time is an illusion; lunchtime doubly so"                              **
*  "...you wouldn't recognize a subtle plan if it painted itself purple and *
*   danced naked on a harpsichord singing 'subtle plans are here again'"    *

Acked-by on v1 1/1
#3 lore raw

Re: [PATCH] compal-laptop/fujitsu-laptop/msi-laptop: make dmi_check_cb

In reply to Axel Lin

Hi Axel

Quoted 14 lines

2010/7/5 Jonathan Woithe <[email protected]>:

Quoted 3 lines

Is there no possibility of some other matching function existing after ours in the tree which might need to also be called? If so, this change would prevent that.

I checked the implementation of dmi_check_system() and the useage cases in compal-laptop/fujitsu-laptop/msi-laptop, it should be no problem with this change.

In compal-laptop and msi-laptop, we call dmi_check_system() to identify the laptop model.
(The callback simply print the identified laptop model) It should have zero or exactly one successful match.

Quoted 5 lines

In fujitsu-laptop, we call dmi_check_system() to identify the laptop model and set parameters accordingly.
The callback print the identified laptop model and set parameters (use_alt_lcd_levels / fujitsu->keycode[1|2|3|4|5] ).
It should have zero or exactly one successful match.

Yes, that seems to be the situation. If that is the case then I'm happy with the fujitsu-laptop component of this patch (I can't speak for the maintainers of the other drivers touched by this patch).

Acked-by Jonathan Woithe <[email protected]>

Regards
jonathan

Quoted 75 lines

dmi_check_system() walks the table running matching functions until someone returns non zero or we hit the end.

This patch makes dmi_check_cb to return 1 so dmi_check_system() return immediately when a match is found.

Signed-off-by Axel Lin <[email protected]>
---
 drivers/platform/x86/compal-laptop.c  |    2 +-
 drivers/platform/x86/fujitsu-laptop.c |    6 +++---
 drivers/platform/x86/msi-laptop.c     |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/platform/x86/compal-laptop.c +1 -1
index 71ff154..dc31a08 100644
@@ -190,7 +190,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 	printk(KERN_INFO "compal-laptop: Identified laptop model '%s'.\n",
 		id->ident);
 
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata compal_dmi_table[] = {
drivers/platform/x86/fujitsu-laptop.c +3 -3
index dab1a0e..1859d7f 100644
@@ -603,7 +603,7 @@ static int dmi_check_cb_s6410(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_s6420(const struct dmi_system_id *id)

@@ -611,7 +611,7 @@ static int dmi_check_cb_s6420(const struct dmi_system_id *id)
 	dmi_check_cb_common(id);
 	fujitsu->keycode1 = KEY_SCREENLOCK;	/* "Lock" */
 	fujitsu->keycode2 = KEY_HELP;	/* "Mobility Center" */
-	return 0;
+	return 1;
 }
 
 static int dmi_check_cb_p8010(const struct dmi_system_id *id)

@@ -620,7 +620,7 @@ static int dmi_check_cb_p8010(const struct dmi_system_id *id)
 	fujitsu->keycode1 = KEY_HELP;	/* "Support" */
 	fujitsu->keycode3 = KEY_SWITCHVIDEOMODE;	/* "Presentation" */
 	fujitsu->keycode4 = KEY_WWW;	/* "Internet" */
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id fujitsu_dmi_table[] = {
drivers/platform/x86/msi-laptop.c +1 -1
index afd762b..c67a74c 100644
@@ -434,7 +434,7 @@ static int dmi_check_cb(const struct dmi_system_id *id)
 {
 	printk(KERN_INFO "msi-laptop: Identified laptop model '%s'.\n",
 	       id->ident);
-	return 0;
+	return 1;
 }
 
 static struct dmi_system_id __initdata msi_dmi_table[] = {
Signature · 1 line
1.5.4.3