Re: [PATCH v7 5/8] Loongson: YeeLoong: add hardware monitoring driver

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

 



On Fri, 2009-12-04 at 09:08 +0100, Pavel Machek wrote:
> Hi!
> 
> > +static int get_cpu_temp(void)
> > +{
> > +	int value;
> > +
> > +	value = ec_read(REG_TEMPERATURE_VALUE);
> > +
> > +	if (value & (1 << 7))
> > +		value = (value & 0x7f) - 128;
> > +	else
> > +		value = value & 0xff;
> 
> wtf?
> 
> Maybe value should be 's8'?
> 
> > +static int get_battery_current(void)
> > +{
> > +	int value;
> > +
> > +	value = (ec_read(REG_BAT_CURRENT_HIGH) << 8) |
> > +		(ec_read(REG_BAT_CURRENT_LOW));
> > +
> > +	if (value & 0x8000)
> > +		value = 0xffff - value;
> 
> Another version of  pair-complement conversion; this one is broken --
> off by 1.
> 
> > +static int parse_arg(const char *buf, unsigned long count, int *val)
> > +{
> > +	if (!count)
> > +		return 0;
> > +	if (sscanf(buf, "%i", val) != 1)
> > +		return -EINVAL;
> > +	return count;
> > +}
> 
> We have strict_strtoul for a reason...
> 

Done, thanks!

Regards,
	Wu Zhangjin

--
To unsubscribe from this list: send the line "unsubscribe linux-laptop" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux ACPI]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]
  Powered by Linux