Re: [PATCH 8/8] thinkpad-acpi: log temperatures on termal alarm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 06 Dec 2009, Pavel Machek wrote:
> > Log temperatures on any of the EC thermal alarms.  It could be
> > useful to help tracking down what is happening...
> 
> Thanks, I applied it locally.
> 
> >  static bool hotkey_notify_thermal(const u32 hkey,
> >  				 bool *send_acpi_ev,
> >  				 bool *ignore_acpi_ev)
> >  {
> > +	int known = true;
> > +
> 
> Oops?

Yeah, will fix and resend the entire stack, as it has grown a few more
patches during the weekend :)

> > +	for (i = 0; i < n; i++)
> > +		t.temp[i] = t.temp[i] / 1000;
> > +
> > +	/* Fill missing sensors with N/A marker */
> > +	for (i = n; i < TPACPI_MAX_THERMAL_SENSORS; i++)
> > +		t.temp[i] = -128;
> 
> -273 would be better "N/A" marker :-).

No can do.  The firmware uses -128 (it is a signed 8-bit value), so there is
an internal driver ABI *and* an userspace ABI since 2.6.13 or thereabouts
that forces N/A sensors in thinkpad-acpi to return -128...

In sysfs, I return an error code instead, which is arguably a much better
way of doing that.

But I do agree -273 would be a cooler value to return for N/A :-)

> > +	/* FIXME: it is ugly */
> > +	printk(TPACPI_NOTICE
> > +		"temperatures (Celsius): "
> > +		"%d %d %d %d %d %d %d %d %d %d %d %d %d %d %d %d\n",
> 
> But this indeed is ugly. Why not sometihng like
> 
> 	printk(TPACPI_NOTICE "temperatures (Celsius): ");
> 	for (i = 0; i < n; i++)
> 		printk(KERN_CONT "%d ", t.temp[i]);
> 	printk(KERN_CONT "\n");
> 
> ...you'd get rid of #ifdef above, ugly -128 markers, and nasty %d series...

Can't get rid of -128, unless I print N/A instead, which your for() loop
would allow.

However, while using KERN_CONT is nicer, that printk is important and needs
to get out to the logs with no reordering, no long delay, and preferably, no
other printks interleaved with it.  Please excuse my lack of knowledge on
this, but wouldn't KERN_CONT make it far more likely that a problem happens
to the printk that will make it hard to read/useless (if reordering
happens)?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux