Hi Hans,
On Mon, 24 Sep 2007 16:28:16 +0200, Jean Delvare wrote:
> Hi Hans,
>
> On Fri, 21 Sep 2007 17:03:32 +0200, Hans de Goede wrote:
> > Here is a patch adding some text to the sysfs interface documentation on how
> > settings written to sysfs attributes should be handled, focussing mainly on
> > error handling. This version incorperates Jean's latest comments.
> >
> > signed-off-by: j.w.r.degoede at hhs.nl
>
> Thanks for doing this.
>
> Acked-by: Jean Delvare <khali at linux-fr.org>
While reviewing your new fschmd driver, I found some mistakes which
also appear in the code examples of this documentation update:
> +--- begin code ---
> +long v = simple_strtol(buf, NULL, 10) / 1000;
> +SENSORS_LIMIT(v, -128, 127);
This is a no-op, should be v = ...
> +/* write v to register */
> +--- end code ---
> +--- begin code ---
> +unsigned long v = simple_strtoul(buf, NULL, 10);
> +
> +switch (v) {
> + case 2: v = 1; break;
> + case 4: v = 2; break;
> + case 8: v = 3; break;
> + default:
> + return -EINVAL;
> +}
Indentation doesn't comply with CodingStyle.
> +/* write v to register */
> +--- end code ---
Additionally, I think that the examples are a bit difficult to read,
one level of indentation would probably be better than the begin/end
code markers. Thus, I propose the following incremental patch:
Documentation/hwmon/sysfs-interface | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
--- linux-2.6.23-rc9.orig/Documentation/hwmon/sysfs-interface 2007-10-07 10:12:33.000000000 +0200
+++ linux-2.6.23-rc9/Documentation/hwmon/sysfs-interface 2007-10-07 10:13:43.000000000 +0200
@@ -450,22 +450,20 @@ continuous like for example a tempX_type
written, -EINVAL should be returned.
Example1, temp1_max, register is a signed 8 bit value (-128 - 127 degrees):
---- begin code ---
-long v = simple_strtol(buf, NULL, 10) / 1000;
-SENSORS_LIMIT(v, -128, 127);
-/* write v to register */
---- end code ---
+
+ long v = simple_strtol(buf, NULL, 10) / 1000;
+ v = SENSORS_LIMIT(v, -128, 127);
+ /* write v to register */
Example2, fan divider setting, valid values 2, 4 and 8:
---- begin code ---
-unsigned long v = simple_strtoul(buf, NULL, 10);
-switch (v) {
- case 2: v = 1; break;
- case 4: v = 2; break;
- case 8: v = 3; break;
- default:
- return -EINVAL;
-}
-/* write v to register */
---- end code ---
+ unsigned long v = simple_strtoul(buf, NULL, 10);
+
+ switch (v) {
+ case 2: v = 1; break;
+ case 4: v = 2; break;
+ case 8: v = 3; break;
+ default:
+ return -EINVAL;
+ }
+ /* write v to register */
--
Jean Delvare
