Hi, On 09/05/2009 07:13 PM, Alistair John Strachan wrote: > Most known Abit motherboards have unique uguru chip IDs. However, > some "refresh" boards keep the same chip ID but have a different > DMI string. As our DMI board string matching is (necessarily) > strict, some boards were failing DMI detection, and as the old > probe method was also failing, the driver would not load. > > The only known boards affected by this problem are the IP35 Pro XE > (vs IP35 Pro) and the AB9 Pro (vs AB9). Is it not sufficient to > relax the match criteria, as some boards (e.g. the AB9 Quad GT) > have different uguru chip IDs. > > This patch replaces the dmi_name string with a NULL terminated > array of strings to be matched per uguru chip ID. It has been > compile and runtime tested (thanks Rune). > > References: https://bugs.launchpad.net/bugs/298798 > > Signed-off-by: Alistair John Strachan<alistair@xxxxxxxxxxxxx> > Tested-by: Rune Svendsen<runesvend@xxxxxxxxx> > Cc: Rune Svendsen<runesvend@xxxxxxxxx> > Cc: Hans de Goede<hdegoede@xxxxxxxxxx> > Cc: lm-sensors@xxxxxxxxxxxxxx > > --- > drivers/hwmon/abituguru3.c | 63 +++++++++++++++++++++++-------------------- > 1 files changed, 34 insertions(+), 29 deletions(-) > > diff --git a/drivers/hwmon/abituguru3.c b/drivers/hwmon/abituguru3.c > index 7d3f15d..5203ec2 100644 > --- a/drivers/hwmon/abituguru3.c > +++ b/drivers/hwmon/abituguru3.c <snip> > @@ -947,7 +951,7 @@ static int __devinit abituguru3_probe(struct platform_device *pdev) > "ID: %04X\n", (unsigned int)id); > > #ifdef CONFIG_DMI > - if (!abituguru3_motherboards[i].dmi_name) { > + if (!*abituguru3_motherboards[i].dmi_name) { > printk(KERN_WARNING ABIT_UGURU3_NAME ": this motherboard was " > "not detected using DMI. Please send the output of " > "\"dmidecode\" to the abituguru3 maintainer " I would prefer to see you write this as: + if (!abituguru3_motherboards[i].dmi_name[0]) { This is better readable IMHO (I always dislike people using the singleton * operator when they want to get the first member of an array. The singleton * operator is for getting the content of a pointer). Note this is in no way a blocker for this patch, just my preference. > @@ -1131,6 +1135,7 @@ static int __init abituguru3_dmi_detect(void) > { > const char *board_vendor, *board_name; > int i, err = (force) ? 1 : -ENODEV; > + const char *const *dmi_name; > size_t sublen; > > board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR); > @@ -1151,17 +1156,17 @@ static int __init abituguru3_dmi_detect(void) > sublen--; > > for (i = 0; abituguru3_motherboards[i].id; i++) { > - const char *dmi_name = abituguru3_motherboards[i].dmi_name; > - if (!dmi_name || strlen(dmi_name) != sublen) > - continue; > - if (!strncasecmp(board_name, dmi_name, sublen)) > - break; > + dmi_name = abituguru3_motherboards[i].dmi_name; > + for ( ; *dmi_name; dmi_name++) { > + if (strlen(*dmi_name) != sublen) > + continue; > + if (!strncasecmp(board_name, *dmi_name, sublen)) > + return 0; > + } > } > > - if (!abituguru3_motherboards[i].id) > - return 1; > - > - return 0; > + /* No match found */ > + return 1; > } > > #else /* !CONFIG_DMI */ Other then that the patch looks good to me. Regards, Hans _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
