On Thu, 25 Jan 2007 01:24:01 +0100
Segher Boessenkool [off-list ref] wrote:

quoted

This patch adds support for of_platform_driver to the ipmi_si module.
When loading the module, the driver will be registered to of_platform.
The driver will be probed for all devices with the type ipmi. It's =20
supporting
devices with compatible settings ipmi-kcs, ipmi-smic and ipmi-bt.
Only ipmi-kcs could be tested.

=20
I'm still saying that because of this, and because they might
never be used and as such be unnecessary baggage, you shouldn't
add SMIC and BT support.

Well, i left it in because there is no baggage except the few bytes in the =
match array.
This way the driver gets loaded if there is such a device.
It looks better to me to add generic support for these devices,
instead of adding code every time a specific device becomes available.
But actually I don't care too much.=20
So if you have another argument than the few bytes baggage, I'll remove it.

=20

quoted

Signed-off-by: Christian Krafft <redacted>
Acked-by: Heiko J Schick <redacted>

=20

quoted

 #define DEFAULT_REGSPACING	1
+#define DEFAULT_REGSIZE		DEFAULT_REGSPACING

=20
Just #define it as 1 I'd say.  Esp. for KCS interfaces, it can't
ever be anything else there.

fixed

=20

quoted

+	if (regsize && proplen!=3D4) {

=20
Whitespace problem (a few times in this file).

fixed

=20

quoted

+	info->si_type		=3D (enum si_type) match->data;

=20
Do you need the cast here?  Oh I suppose you do, why else
did you add it (and it teaches the world as a whole once
again that enums in C are bloody useless almost always).

yep, I also feel sorry for that.

=20

quoted

+static int __devexit ipmi_of_remove(struct of_device *dev)
+{
+	/* should call
+	 * cleanup_one_si(dev->dev.driver_data); */
+	return 0;
+}

=20
While I know this isn't really your problem, no one who
isn't touching the IPMI code will ever fix it, so...  nudge
nudge, wink wink.

fixed, 2.6.20 will contain the forward declaration,=20
so the cleanup code can be called there.

=20

quoted

(void *)(unsigned long) SI_KCS

=20
Yes I do hate enums.

Why ?

=20

quoted

+	.name		=3D "ipmi",

=20
Shouldn't this name be "ipmi-kcs" etc.?  Just asking :-)

You just wanna confuse me, right ?

=20
Cheers,
=20
=20
Segher
=20

See my next mail for patch.

--=20
Mit freundlichen Gr=FCssen,
kind regards,

Christian Krafft
IBM Systems & Technology Group,=20
Linux Kernel Development
IT Specialist