All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU models
@ 2012-10-09 21:09 Guenter Roeck
  2012-10-12  9:12 ` [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU mo Jean Delvare
  2012-10-12 16:14 ` Guenter Roeck
  0 siblings, 2 replies; 3+ messages in thread
From: Guenter Roeck @ 2012-10-09 21:09 UTC (permalink / raw)
  To: lm-sensors

Make the code easier to extend and easier to adjust by using a model table
listing CPU models, stepping/mask, and associated TjMax.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/coretemp.c |   36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 1937cd4..7107096 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -208,6 +208,23 @@ static const struct tjmax __cpuinitconst tjmax_table[] = {
 	{ "CPU  330", 125000 },
 };
 
+struct tjmax_model {
+	u8 model;
+	u8 mask;
+	int tjmax;
+};
+
+static const struct tjmax_model __cpuinitconst tjmax_model_table[] = {
+	{ 0x1c, 10, 100000 },	/* D4xx, N4xx, D5xx, N5xx */
+	{ 0x1c, 0, 90000 },	/* Z5xx, N2xx, 230, 330, others	*/
+	{ 0x26, 0, 90000 },	/* Atom Tunnel Creek (Exx), Lincroft (Z6xx)
+				 * Note: TjMax for E6xxT is 110C, but CPU type
+				 * is undetectable by software
+				 */
+	{ 0x27, 0, 90000 },	/* Atom Medfield (Z2460) */
+	{ 0x36, 0, 100000 },	/* Atom Cedar Trail/Cedarview (N2xxx, D2xxx) */
+};
+
 static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
 				  struct device *dev)
 {
@@ -226,20 +243,11 @@ static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
 			return tjmax_table[i].tjmax;
 	}
 
-	/* Atom CPUs */
-
-	if (c->x86_model = 0x1c) {
-		/*
-		 * TjMax for stepping 10 CPUs (N4xx, N5xx, D4xx, D5xx)
-		 * is 100 degrees C, for all others it is 90 degrees C.
-		 */
-		if (c->x86_mask = 10)
-			return 100000;
-		return 90000;
-	} else if (c->x86_model = 0x26 || c->x86_model = 0x27) {
-		return 90000;
-	} else if (c->x86_model = 0x36) {
-		return 100000;
+	for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) {
+		const struct tjmax_model *tm = &tjmax_model_table[i];
+		if (c->x86_model = tm->model &&
+		    (!tm->mask || c->x86_mask = tm->mask))
+			return tm->tjmax;
 	}
 
 	/* Early chips have no MSR for TjMax */
-- 
1.7.9.7


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU mo
  2012-10-09 21:09 [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU models Guenter Roeck
@ 2012-10-12  9:12 ` Jean Delvare
  2012-10-12 16:14 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Jean Delvare @ 2012-10-12  9:12 UTC (permalink / raw)
  To: lm-sensors

On Tue,  9 Oct 2012 14:09:00 -0700, Guenter Roeck wrote:
> Make the code easier to extend and easier to adjust by using a model table
> listing CPU models, stepping/mask, and associated TjMax.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/hwmon/coretemp.c |   36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)

I like the idea. Just a couple things about the implementation:

> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index 1937cd4..7107096 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -208,6 +208,23 @@ static const struct tjmax __cpuinitconst tjmax_table[] = {
>  	{ "CPU  330", 125000 },
>  };
>  
> +struct tjmax_model {
> +	u8 model;
> +	u8 mask;
> +	int tjmax;
> +};
> +
> +static const struct tjmax_model __cpuinitconst tjmax_model_table[] = {
> +	{ 0x1c, 10, 100000 },	/* D4xx, N4xx, D5xx, N5xx */
> +	{ 0x1c, 0, 90000 },	/* Z5xx, N2xx, 230, 330, others	*/

I don't think the 230 and 330 should be listed there, as they are
overridden by an earlier match in tjmax_table[].

> +	{ 0x26, 0, 90000 },	/* Atom Tunnel Creek (Exx), Lincroft (Z6xx)
> +				 * Note: TjMax for E6xxT is 110C, but CPU type
> +				 * is undetectable by software
> +				 */
> +	{ 0x27, 0, 90000 },	/* Atom Medfield (Z2460) */
> +	{ 0x36, 0, 100000 },	/* Atom Cedar Trail/Cedarview (N2xxx, D2xxx) */
> +};

Using 0 as a special value may be problematic, as I think it is a
possible stepping value:

http://openbenchmarking.org/system/1109120-IV-TSCP1539338/tscp1/cpuinfo

(I agree model name looks odd, not sure if it is a public model or some
engineering sample.)

In hwmon-vid we have:

#define ANY 0xFF

and we use that when the stepping doesn't matter. Note that we also
support stepping ranges there, maybe we'll need it for coretemp too
some day, but I guess we can add it then.

> +
>  static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
>  				  struct device *dev)
>  {
> @@ -226,20 +243,11 @@ static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
>  			return tjmax_table[i].tjmax;
>  	}
>  
> -	/* Atom CPUs */
> -
> -	if (c->x86_model = 0x1c) {
> -		/*
> -		 * TjMax for stepping 10 CPUs (N4xx, N5xx, D4xx, D5xx)
> -		 * is 100 degrees C, for all others it is 90 degrees C.
> -		 */
> -		if (c->x86_mask = 10)
> -			return 100000;
> -		return 90000;
> -	} else if (c->x86_model = 0x26 || c->x86_model = 0x27) {
> -		return 90000;
> -	} else if (c->x86_model = 0x36) {
> -		return 100000;
> +	for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) {
> +		const struct tjmax_model *tm = &tjmax_model_table[i];
> +		if (c->x86_model = tm->model &&
> +		    (!tm->mask || c->x86_mask = tm->mask))
> +			return tm->tjmax;
>  	}
>  
>  	/* Early chips have no MSR for TjMax */

Other than these implementation details:

Acked-by: Jean Delvare <khali@linux-fr.org>

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU mo
  2012-10-09 21:09 [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU models Guenter Roeck
  2012-10-12  9:12 ` [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU mo Jean Delvare
@ 2012-10-12 16:14 ` Guenter Roeck
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2012-10-12 16:14 UTC (permalink / raw)
  To: lm-sensors

On Fri, Oct 12, 2012 at 11:12:04AM +0200, Jean Delvare wrote:
> On Tue,  9 Oct 2012 14:09:00 -0700, Guenter Roeck wrote:
> > Make the code easier to extend and easier to adjust by using a model table
> > listing CPU models, stepping/mask, and associated TjMax.
> > 
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> >  drivers/hwmon/coretemp.c |   36 ++++++++++++++++++++++--------------
> >  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> I like the idea. Just a couple things about the implementation:
> 
> > diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> > index 1937cd4..7107096 100644
> > --- a/drivers/hwmon/coretemp.c
> > +++ b/drivers/hwmon/coretemp.c
> > @@ -208,6 +208,23 @@ static const struct tjmax __cpuinitconst tjmax_table[] = {
> >  	{ "CPU  330", 125000 },
> >  };
> >  
> > +struct tjmax_model {
> > +	u8 model;
> > +	u8 mask;
> > +	int tjmax;
> > +};
> > +
> > +static const struct tjmax_model __cpuinitconst tjmax_model_table[] = {
> > +	{ 0x1c, 10, 100000 },	/* D4xx, N4xx, D5xx, N5xx */
> > +	{ 0x1c, 0, 90000 },	/* Z5xx, N2xx, 230, 330, others	*/
> 
> I don't think the 230 and 330 should be listed there, as they are
> overridden by an earlier match in tjmax_table[].
> 
I rephrased it:
				/* Z5xx, N2xx, possibly others
                                 * Note: Also matches 230 and 330,
				 * which are covered by tjmax_table
				 */

> > +	{ 0x26, 0, 90000 },	/* Atom Tunnel Creek (Exx), Lincroft (Z6xx)
> > +				 * Note: TjMax for E6xxT is 110C, but CPU type
> > +				 * is undetectable by software
> > +				 */
> > +	{ 0x27, 0, 90000 },	/* Atom Medfield (Z2460) */
> > +	{ 0x36, 0, 100000 },	/* Atom Cedar Trail/Cedarview (N2xxx, D2xxx) */
> > +};
> 
> Using 0 as a special value may be problematic, as I think it is a
> possible stepping value:
> 
> http://openbenchmarking.org/system/1109120-IV-TSCP1539338/tscp1/cpuinfo
> 
> (I agree model name looks odd, not sure if it is a public model or some
> engineering sample.)
> 
> In hwmon-vid we have:
> 
> #define ANY 0xFF
> 
Good idea. I changed the code to use ANY.

> and we use that when the stepping doesn't matter. Note that we also
> support stepping ranges there, maybe we'll need it for coretemp too
> some day, but I guess we can add it then.
> 
Agreed.

> > +
> >  static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
> >  				  struct device *dev)
> >  {
> > @@ -226,20 +243,11 @@ static int __cpuinit adjust_tjmax(struct cpuinfo_x86 *c, u32 id,
> >  			return tjmax_table[i].tjmax;
> >  	}
> >  
> > -	/* Atom CPUs */
> > -
> > -	if (c->x86_model = 0x1c) {
> > -		/*
> > -		 * TjMax for stepping 10 CPUs (N4xx, N5xx, D4xx, D5xx)
> > -		 * is 100 degrees C, for all others it is 90 degrees C.
> > -		 */
> > -		if (c->x86_mask = 10)
> > -			return 100000;
> > -		return 90000;
> > -	} else if (c->x86_model = 0x26 || c->x86_model = 0x27) {
> > -		return 90000;
> > -	} else if (c->x86_model = 0x36) {
> > -		return 100000;
> > +	for (i = 0; i < ARRAY_SIZE(tjmax_model_table); i++) {
> > +		const struct tjmax_model *tm = &tjmax_model_table[i];
> > +		if (c->x86_model = tm->model &&
> > +		    (!tm->mask || c->x86_mask = tm->mask))
> > +			return tm->tjmax;
> >  	}
> >  
> >  	/* Early chips have no MSR for TjMax */
> 
> Other than these implementation details:
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>
> 
Thanks!

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-10-12 16:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-09 21:09 [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU models Guenter Roeck
2012-10-12  9:12 ` [lm-sensors] [PATCH 2/5] hwmon: (coretemp) Use model table instead of if/else to identify CPU mo Jean Delvare
2012-10-12 16:14 ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.