[lm-sensors] [RFC PATCH] hwmon sysfs class, again

[lm-sensors] [RFC PATCH] hwmon sysfs class, again

[Mark M. Hoffman]

Hi all:

Here are the "new and improved" hwmon sysfs class patches:

http://members.dca.net/mhoffman/sensors/temp/20050627/

Jean Delvare's recent patches[1] must be applied first.  I plan to submit
the hwmon/sysfs patches for inclusion into -mm shortly after Jean's are
submitted.

[1] http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012850.html

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

[lm-sensors] Re: [RFC PATCH] hwmon sysfs class, again

[Greg KH]

On Tue, Jun 28, 2005 at 12:14:14AM -0400, Mark M. Hoffman wrote:
> Hi all:
> 
> Here are the "new and improved" hwmon sysfs class patches:
> 
> http://members.dca.net/mhoffman/sensors/temp/20050627/
> 
> Jean Delvare's recent patches[1] must be applied first.  I plan to submit
> the hwmon/sysfs patches for inclusion into -mm shortly after Jean's are
> submitted.
> 
> [1] http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012850.html

Looks good to me.

thanks,

greg k-h

[lm-sensors] Re: [RFC PATCH] hwmon sysfs class, again

[Jean Delvare]

Hi Mark,

> Here are the "new and improved" hwmon sysfs class patches:
>
> http://members.dca.net/mhoffman/sensors/temp/20050627/

Looks all OK to me, except for:

> --- linux-2.6.12-git5.orig/drivers/hwmon/adm1026.c
> +++ linux-2.6.12-git5/drivers/hwmon/adm1026.c
> (...)
> @@ -1690,8 +1701,10 @@ int adm1026_detect(struct i2c_adapter *a
> (...)
>  exitfree:
> -	kfree(new_client);
> +	kfree(data);

Although this fix is correct, it would belong to
hwmon-chips-kfree-fix.patch, not hwmon-class-chips.patch.

Also, it looks to me like adm9240, smsc47m1 and smsc47b397 need the same
fix.

> --- linux-2.6.12-git5.orig/drivers/hwmon/asb100.c
> +++ linux-2.6.12-git5/drivers/hwmon/asb100.c
> (..)
> +        data->class_dev = hwmon_device_register(&new_client->dev);
> +        if (IS_ERR(data->class_dev)) {
> +                err = PTR_ERR(data->class_dev);
> +                goto ERROR3;
> +        }

Beware you are using spaces instead of tabulations here.

> --- linux-2.6.12-git5.orig/drivers/hwmon/w83781d.c
> +++ linux-2.6.12-git5/drivers/hwmon/w83781d.c
> (...)
> +ERROR4:
> +	if (NULL != data->lm75[1]) {
> +		i2c_detach_client(data->lm75[1]);
> +		kfree(data->lm75[1]);
> +	}
> +	if (NULL != data->lm75[0]) {
> +		i2c_detach_client(data->lm75[0]);
> +		kfree(data->lm75[0]);
> +	}

Please express the comparisons the other way around. I can guess why you
did it this way, but the usual style in the kernel is different and gcc
would warn you anyway.

Thanks,
--
Jean Delvare

[lm-sensors] Re: [RFC PATCH] hwmon sysfs class, again

[Mark M. Hoffman]

Hi Jean:

Thanks for the review.

* Jean Delvare <khali@linux-fr.org> [2005-06-28 12:44:07 +0200]:
> 
> Hi Mark,
> 
> > Here are the "new and improved" hwmon sysfs class patches:
> >
> > http://members.dca.net/mhoffman/sensors/temp/20050627/
> 
> Looks all OK to me, except for:
> 
> > --- linux-2.6.12-git5.orig/drivers/hwmon/adm1026.c
> > +++ linux-2.6.12-git5/drivers/hwmon/adm1026.c
> > (...)
> > @@ -1690,8 +1701,10 @@ int adm1026_detect(struct i2c_adapter *a
> > (...)
> >  exitfree:
> > -	kfree(new_client);
> > +	kfree(data);
> 
> Although this fix is correct, it would belong to
> hwmon-chips-kfree-fix.patch, not hwmon-class-chips.patch.
> 
> Also, it looks to me like adm9240, smsc47m1 and smsc47b397 need the same
> fix.

right - fixed all of that

> > --- linux-2.6.12-git5.orig/drivers/hwmon/asb100.c
> > +++ linux-2.6.12-git5/drivers/hwmon/asb100.c
> > (..)
> > +        data->class_dev = hwmon_device_register(&new_client->dev);
> > +        if (IS_ERR(data->class_dev)) {
> > +                err = PTR_ERR(data->class_dev);
> > +                goto ERROR3;
> > +        }
> 
> Beware you are using spaces instead of tabulations here.

oops - fixed

> > --- linux-2.6.12-git5.orig/drivers/hwmon/w83781d.c
> > +++ linux-2.6.12-git5/drivers/hwmon/w83781d.c
> > (...)
> > +ERROR4:
> > +	if (NULL != data->lm75[1]) {
> > +		i2c_detach_client(data->lm75[1]);
> > +		kfree(data->lm75[1]);
> > +	}
> > +	if (NULL != data->lm75[0]) {
> > +		i2c_detach_client(data->lm75[0]);
> > +		kfree(data->lm75[0]);
> > +	}
> 
> Please express the comparisons the other way around. I can guess why you
> did it this way, but the usual style in the kernel is different and gcc
> would warn you anyway.

Actually, there were two of those already in the file (probably by me though).
I prefer 'if (p)' myself, but the day job code style is stamped on my brain,
heh.  Anyway, they're fixed.

New patches here:

http://members.dca.net/mhoffman/sensors/temp/20050703/

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

[lm-sensors] Re: [RFC PATCH] hwmon sysfs class, again

[Jean Delvare]

Hi Mark,

> New patches here:
> 
> http://members.dca.net/mhoffman/sensors/temp/20050703/

Works all OK for me :)

-- 
Jean Delvare

[lm-sensors] Re: [RFC PATCH] hwmon sysfs class, again

[Jean Delvare]

Hi Mark,

> New patches here:
> 
> http://members.dca.net/mhoffman/sensors/temp/20050703/

Now that my patches moving all the drivers to the new place have been
accepted, could you please post these three patches here so that Greg
can pick them up?

(You can mark them as signed-off by me if you wish.)

Thanks,
-- 
Jean Delvare