Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines

[Jeremy Kerr <jk@ozlabs.org>]

Hi Corey,

Thanks for the review.

>> IPMI folks: the IPMI driver could do with a little review, as it's
>> not a conventional BT/KCS/SMI SI, in that the low-level send/recv
>> interface will handle the entire message at once.
> 
> Handling the entire message at once should be fine, as that's what
> this driver level is designed to do for the message handler.  That
> part all looks correct.  The code itself looks good, but I have a
> couple of high-level comments.
> 
> The driver at this level can receive more than one message to handle
> at a time, so it needs some sort of queue.  This is to allow multiple
> users and to allow the message handler to send its own commands while
> other commands are going on.  You might argue that the queuing should
> be done in ipmi_msghandler, and you would probably be right.

Ah, that's what I'd been assuming was being done - I missed the
xmit_list in the si_intf code. It'd be great if this could be in the
generic msghandler code, otherwise I'd just be duplicating the si_intf
logic.

> I'll look at doing that.  If that is the case, then your NULL check
> for current message should probably be a BUG_ON().

OK, I'll update this when the msghandler bit is implemented.

> Do you need to handle any BMC flags?  Particularly incoming events?

Not at this stage - we may in future though.

Cheers,


Jeremy