Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
On Thu, 2014-11-06 at 08:15 -0600, Corey Minyard wrote: On 11/05/2014 09:38 PM, Jeremy Kerr wrote: Corey Michael: if this is acceptable, it may be mergable as two separate patches - one for the IPMI subsystem, one for the powernv platform. However, we'd need to preserve their order: patch 2/2 depends on 1/2, which provides the structure function definitions. This'll break the build if only 2/2 is in the tree, and CONFIG_IPMI_POWERNV is set. Alternatively, they could be merged by one maintainer, pending an ack from the other. I'm fine either way. How about the third option? :) I've put patch 1 in a topic branch: https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/log/?h=topic/opal-ipmi And will merge that into my next, probably by tomorrow. If you merge the topic branch and then apply patch 2/2, then it should all go in without any hiccups. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
Hi Corey, Alternatively, they could be merged by one maintainer, pending an ack from the other. I'm fine either way. How about the third option? :) I've put patch 1 in a topic branch: https://git.kernel.org/cgit/linux/kernel/git/mpe/linux.git/log/?h=topic/opal-ipmi And will merge that into my next, probably by tomorrow. If you merge the topic branch and then apply patch 2/2, then it should all go in without any hiccups. OK, and I have an updated IPMI driver patch (ie, 2/2) that will apply on top of the new ipmi/for-next tree (best done after that merge). Patch coming shortly... Cheers, Jeremy ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
On Mon, 2014-11-10 at 11:26 +0800, Jeremy Kerr wrote: 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. Our OPAL interface can only do one at a time ? Because our underlying FW driver already has a queue .. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
Hi Ben, Our OPAL interface can only do one at a time ? Because our underlying FW driver already has a queue .. The Linux driver needs to match responses to their requests, so the driver needs to keep a reference to the requests that we've sent. The current implementation is very simple: we just keep one pointer, and require that there's only one outstanding request. To have multiple outstanding requests, our options here are that we either: 1) duplicate the si_intf's queuing code in the powernv driver; or 2) move the queuing code from the si_intf component to the generic IPMI msghander, and use that (so all drivers can assume a single outstanding request) Corey has suggested the latter, which will only require a minimal change to the powerpv IPMI driver. Cheers, Jeremy ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] Add IPMI support for powernv powerpc machines
On Mon, 2014-11-10 at 17:01 +1100, Alistair Popple wrote: Ben, Our OPAL interface can only do one at a time ? Because our underlying FW driver already has a queue .. The OPAL interface supports sending more than one message at a time using the underlying FW queue as you suggest. However in theory the interface doesn't make any response order guarantees, but in practice they should be ordered as our BMC doesn't send out-of-order responses. Can't we put some ID in the response message ? OPAL messages have room... Cheers, Ben. Regards, Alistair 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/2] Add IPMI support for powernv powerpc machines
This series adds IPMI driver and arch glue for OPAL-firmware-based powernv machines. The first change exposes the firmware's IPMI API, and the second adds an actual driver. 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. Corey Michael: if this is acceptable, it may be mergable as two separate patches - one for the IPMI subsystem, one for the powernv platform. However, we'd need to preserve their order: patch 2/2 depends on 1/2, which provides the structure function definitions. This'll break the build if only 2/2 is in the tree, and CONFIG_IPMI_POWERNV is set. Alternatively, they could be merged by one maintainer, pending an ack from the other. Comments / queries / etc welcome. Cheers, Jeremy --- Jeremy Kerr (2): powerpc/powernv: Add OPAL IPMI interface drivers/char/ipmi: Add powernv IPMI driver ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
