On 10/9/07, Jean Delvare <khali at linux-fr.org> wrote:
> Hi Mark, hi Jim,
>
> On Tue, 9 Oct 2007 09:04:02 -0400, Mark M. Hoffman wrote:
> > * Jean Delvare <khali at linux-fr.org> [2007-10-02 12:50:55 +0200]:
> > > Once you send a patch that I can apply on top of Mark's tree, I'll
> > > review it.
> >
> > Here you go, I merged it for you. Or git did, but I resolved one conflict.
>
> OK, thank you. After reviewing and testing, here are the problems I've found:
>
> > @@ -218,7 +219,7 @@ static const u8 regpwm_627hf[] = { W83627HF_REG_PWM1, W83627HF_REG_PWM2 };
> > static const u8 regpwm[] = { W83627THF_REG_PWM1, W83627THF_REG_PWM2,
> > W83627THF_REG_PWM3 };
> > #define W836X7HF_REG_PWM(type, nr) (((type) == w83627hf) ? \
> > - regpwm_627hf[(nr) - 1] : regpwm[(nr) - 1])
> > + regpwm_627hf[nr] : regpwm[nr])
>
> Tabs should be used for indentation (not introduced by your patch but
> still worth fixing.)
Done.
>
> >
> > #define W83627HF_REG_PWM_FREQ 0x5C /* Only for the 627HF */
> >
> > @@ -400,72 +401,71 @@ static struct platform_driver w83627hf_driver = {
> > .remove = __devexit_p(w83627hf_remove),
> > };
> >
> > -/* following are the sysfs callback functions */
> > -#define show_in_reg(reg) \
> > -static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
> > -{ \
> > - struct w83627hf_data *data = w83627hf_update_device(dev); \
> > - return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
> > +static ssize_t
> > +show_in_input(struct device *dev, struct device_attribute *devattr, char *buf)
> > +{
> > + int nr = to_sensor_dev_attr(devattr)->index;
> > + struct w83627hf_data *data = w83627hf_update_device(dev);
> > + return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->in[nr]));
>
> Missing space after comma (not introduced by your patch but still worth
> fixing.)
done
> > @@ -1171,30 +1145,13 @@ static const struct attribute_group w83627hf_group = {
> > };
> >
> > static struct attribute *w83627hf_attributes_opt[] = {
> > - &dev_attr_in1_input.attr,
> > - &dev_attr_in1_min.attr,
> > - &dev_attr_in1_max.attr,
> > - &dev_attr_in5_input.attr,
> > - &dev_attr_in5_min.attr,
> > - &dev_attr_in5_max.attr,
> > - &dev_attr_in6_input.attr,
> > - &dev_attr_in6_min.attr,
> > - &dev_attr_in6_max.attr,
> > -
> > - &dev_attr_fan3_input.attr,
> > - &dev_attr_fan3_min.attr,
> > - &dev_attr_fan3_div.attr,
> > -
> > - &dev_attr_temp3_input.attr,
> > - &dev_attr_temp3_max.attr,
> > - &dev_attr_temp3_max_hyst.attr,
> > - &dev_attr_temp3_type.attr,
> > -
> > - &dev_attr_pwm3.attr,
> > -
> > - &dev_attr_pwm1_freq.attr,
> > - &dev_attr_pwm2_freq.attr,
> > - &dev_attr_pwm3_freq.attr,
> > + VIN_UNIT_ATTRS(1),
> > + VIN_UNIT_ATTRS(5),
> > + VIN_UNIT_ATTRS(6),
> > +
> > + FAN_UNIT_ATTRS(3),
> > + TEMP_UNIT_ATTRS(3),
> > + &sensor_dev_attr_pwm3.dev_attr.attr,
>
> Missing pwm1_freq, pwm2_freq and pwm3_freq attributes.
>
Ack. Done.
> > NULL
> > };
> >
> > @@ -1252,27 +1209,41 @@ static int __devinit w83627hf_probe(struct platform_device *pdev)
> >
> > /* Register chip-specific device attributes */
> > if (data->type == w83627hf || data->type == w83697hf)
> > - if ((err = device_create_file(dev, &dev_attr_in5_input))
> > - || (err = device_create_file(dev, &dev_attr_in5_min))
> > - || (err = device_create_file(dev, &dev_attr_in5_max))
> > - || (err = device_create_file(dev, &dev_attr_in6_input))
> > - || (err = device_create_file(dev, &dev_attr_in6_min))
> > - || (err = device_create_file(dev, &dev_attr_in6_max))
> > - || (err = device_create_file(dev, &dev_attr_pwm1_freq))
> > - || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
> > + if ((err = device_create_file(dev,
> > + &sensor_dev_attr_in5_input.dev_attr))
> > + || (err = device_create_file(dev,
> > + &sensor_dev_attr_in5_min.dev_attr))
> > + || (err = device_create_file(dev,
> > + &sensor_dev_attr_in5_max.dev_attr))
> > + || (err = device_create_file(dev,
> > + &sensor_dev_attr_in6_input.dev_attr))
> > + || (err = device_create_file(dev,
> > + &sensor_dev_attr_in6_min.dev_attr))
> > + || (err = device_create_file(dev,
> > + &sensor_dev_attr_in6_max.dev_attr)))
>
> pwm1_freq and pwm2_freq attributes are no longer created.
Ack. Done.
> > *w83627hf_update_device(struct device *dev)
> > w83627hf_read_value(data,
> > W83781D_REG_FAN_MIN(i));
> > }
> > - for (i = 1; i <= 3; i++) {
> > + for (i = 0; i <= 2; i++) {
> > u8 tmp = w83627hf_read_value(data,
> > W836X7HF_REG_PWM(data->type, i));
> > /* bits 0-3 are reserved in 627THF */
> > if (data->type == w83627thf)
> > tmp &= 0xf0;
> > - data->pwm[i - 1] = tmp;
> > - if(i == 2 &&
> > + data->pwm[i] = tmp;
> > + if(i == 1 &&
>
> Missing space before opening parenthesis.
Done.
Also fixed one other found when searching for 'if(', I hope thats ok.
>
> > (data->type == w83627hf || data->type == w83697hf))
> > break;
> > }
>
>
> --
> Jean Delvare
>
Im attaching an incremental patch (on top of Mark's),
assuming that this is easiest.
If you want it against hwmon-testing (with change-entry, diffstat, signoff)
or you want the extraneous 'if(' fix removed, let me know.
Again, thanks to both of you.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff.demacro-on-marks
Type: application/octet-stream
Size: 2149 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20071011/af393ae0/attachment.obj
