Hi Wei,
On Mon, 29 Jul 2013 19:15:12 +0800, Wei Ni wrote:
> On 07/27/2013 11:38 PM, Jean Delvare wrote:
> > On Fri, 12 Jul 2013 15:48:07 +0800, Wei Ni wrote:
> >> +/*
> >> + * TEMP11 register index
> >> + */
> >> +enum lm90_temp11_reg_index {
> >> + TEMP11_REMOTE_TEMP = 0, /* 0: remote input */
> >> + TEMP11_REMOTE_LOW, /* 1: remote low limit */
> >> + TEMP11_REMOTE_HIGH, /* 2: remote high limit */
> >> + TEMP11_REMOTE_OFFSET, /* 3: remote offset
> >> + * (except max6646, max6657/58/59,
> >> + * and max6695/96)
> >> + */
> >> + TEMP11_LOCAL_TEMP, /* 4: local input */
> >> + TEMP11_REMOTE2_TEMP, /* 5: remote 2 input (max6695/96 only) */
> >> + TEMP11_REMOTE2_LOW, /* 6: remote 2 low limit (max6695/96 only) */
> >> + TEMP11_REMOTE2_HIGH, /* 7: remote 2 high limit (max6695/96 only) */
> >> + TEMP11_REG_NUM
> >> +};
> > (...)
> > Also, the comments are mostly useless now, they were there to document
> > what each number was referring to, but now this is exactly what the new
> > constants are doing.
>
> Yes, we can remove these comments, but I think it's better to remain
> those exception and only things.
Yes, I agree.
> >> +
> >> +/*
> >> + * TEMP11 register NR
> >> + */
> >> +enum lm90_temp11_reg_nr {
> >> + NR_CHAN_0_REMOTE_LOW = 0, /* 0: channel 0, remote low limit */
> >> + NR_CHAN_0_REMOTE_HIGH, /* 1: channel 0, remote high limit */
> >> + NR_CHAN_0_REMOTE_OFFSET, /* 2: channel 0, remote offset */
> >> + NR_CHAN_1_REMOTE_LOW, /* 3: channel 1, remote low limit */
> >> + NR_CHAN_1_REMOTE_HIGH, /* 4: channel 1, remote high limit */
> >
> > The conventions used in the descriptions diverge from the ones used
> > above. "channel 0 remote" here is just "remove" above, and "channel 1
> > remote" is "remote 2" above. This is quite confusing.
> >
> >> + NR_NUM /* number of the NRs for temp11 */
> >
> > The fact that you were unable to come up with a proper name for this
> > number is a clear indication that this enum should not exist in the
> > first place.
> >
> > These numbers are used only once, to pass specific information to
> > set_temp11. This was easy enough when these were just numbers and I
> > really had no reason not to do that.
>
> Ok, so how about to remove these changes, and keep the original codes to
> use numbers.
Fine with me. We can always change the code later to use the TEMP11
index values instead if anyone cares, this can be done separately.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
