quoted

+	__u8  led_state;
+	struct led_classdev *led[5];

Why not put this under the CONFIG_LEDS_CLASS condition as well?

Can do, I don't think these are referenced anywhere else.

quoted

+	/* register led subsystem - G27 only */
+	entry->led_state = 0;
+	for (j = 0; j < 5; j++)
+		entry->led[j] = NULL;
+

The same here.

OK.

quoted

+			if (error) {
+				hid_err(hid, "failed to register LED %d. Aborting.\n", j);
+err:
+				/* Deregister LEDs (if any) but let the driver continue */
+				for (j = 0; j < 5; j++) {
+					led = entry->led[j];
+					entry->led[j] = NULL;
+					if (!led)
+						continue;
+					led_classdev_unregister(led);
+				kfree(led);

Indentation seems to be wrong here.

Bugger...

quoted

+				}
+				break;	/* Don't create any more LEDs */

Putting the whole err: label outside the outer for cycle would make the
whole thing much more readable (and would save you the break). Could you
please redo this?

The 'break' is actually not needed as 'j' has reached maximum value and
the outer for loop would be at completion. I put the 'break' there just to
highlight this fact.

Maybe just a comment would be OK, such as 'on error fall though to driver
completion'.

The reason I'd like this code here is that it seems traditional to output
the following 'hid_info' line once the driver is fully active.

quoted

 	hid_info(hid, "Force feedback for Logitech Speed Force Wireless by
Simon Wood [off-list ref]\n");

Moving the 'err:' code would have to:
1). Jump back to hit this hid_info line
2). Duplicate the hid_info line
3). Have err code called as a function

Any major objections to keeping it as is?


Man, it seems to be taking me ages to get this code right....
Simon