Re: [PATCH v4] hwmon: Add support for MAX6642

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

 



> On Wed, Apr 06, 2011 at 03:06:13PM -0400, Per Dalén wrote:
>> On 04/06/2011 08:58 PM, Guenter Roeck wrote:
>> > Hi Per,
>> >
>> > On Wed, Apr 06, 2011 at 02:29:44PM -0400, Per Dalén wrote:
>> >> First a big thanks to Guenter Roeck for helping me (allot) to cleanup
>> >> this driver
>> >>
>> >> v3 -> v4:
>> >>  * Removed "data->valid = 0;" (missed that in v3 ;)
>> >>  * only read the limits once
>> >>  * changed the temp_input and temp_high as suggested by Guenter, also
>> >> changed the functions/macros related to that change
>> >
>> >> MAX6642 is a SMBus-Compatible Remote/Local Temperature Sensor with
>> Overtemperature Alarm from Maxim.
>> >>
>> >> Signed-off-by: Per Dalen <per.dalen@xxxxxxxxxxxx>
>> >>
>> > Sorry, one more detail I just noticed. More of a question.
>> >
>> > [ ... ]
>> >> +
>> >> +static struct max6642_data *max6642_update_device(struct device
>> *dev)
>> >> +{
>> >> +	struct i2c_client *client = to_i2c_client(dev);
>> >> +	struct max6642_data *data = i2c_get_clientdata(client);
>> >> +	u16 val, tmp;
>> >> +
>> >> +	mutex_lock(&data->update_lock);
>> >> +
>> >> +	if (time_after(jiffies, data->last_updated + HZ * 8) ||
>> !data->valid) {
>> >
>> > Are you sure you only want to update readings every 8 seconds ?
>>
>> No, this is wrong from me. Every second, or what do you think?
>>
> Something like that, or HZ + HZ/<fraction> if you want to be fancy.
>
>> >
>> > Also, after spending all this time on this driver (and after falling
>> into
>> > the same trap last night with another driver ;), it looks like this is
>> really
>> > just another variant of the lm90, and adding support for max6642 to
>> the lm90
>> > driver might be quite trivial. Have you considered this ?
>>
>> Hmmm, ;)
>>
>> "Re:  [PATCH] hwmon: (lm90) Add support for max6642
>>
>> On 03/03/2011 08:55 PM, Guenter Roeck wrote:
>> With all the differences and lacking registers, I wonder if it would be
>> better to write a new driver, either based on this driver of based on
>> the max1619 driver.
>>
>> Guenter"
>>
> Guess you got me there. But then this was more than a month ago, and it
> was
> my birthday, so who knows how much blood was left in my alcohol ;).

Ok, that is something I can relate to ;)

Do you think I should do the last fix on the max6642 driver or should I
add it to lm90?

>
> Thanks,
> Guenter
>

/Per


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



[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux