[Skiboot] [RFC PATCH 1/6] ipmi: Introduce registration for SEL command handlers

[Stewart Smith]

Andrew Jeffery <andrew at aj.id.au> writes:
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  hw/ipmi/ipmi-sel.c | 110 +++++++++++++++++++++++++++++++++------------
>  include/ipmi.h     |   5 +++
>  2 files changed, 86 insertions(+), 29 deletions(-)
>
> diff --git a/hw/ipmi/ipmi-sel.c b/hw/ipmi/ipmi-sel.c
> index eb63147bdc52..c85cd6fc781b 100644
> --- a/hw/ipmi/ipmi-sel.c
> +++ b/hw/ipmi/ipmi-sel.c
> @@ -15,6 +15,10 @@
>   */
>  
>  #define pr_fmt(fmt) "IPMI: " fmt
> +#include <ccan/list/list.h>
> +#include <ccan/str/str.h>
> +#include <compiler.h>
> +#include <errno.h>
>  #include <skiboot.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -37,10 +41,14 @@
>  #define SEL_NETFN_IBM		0x3a
>  
>  /* OEM SEL Commands */
> +/* TODO: Move these to their respective source files */

FWIW I think it's okay to leave that to the future.

> +int ipmi_sel_register(uint8_t oem_cmd,
> +		      void (*fn)(uint8_t data, void *context),
> +		      void *context)
> +{
> +	struct ipmi_sel_handler *handler;
> +
> +	handler = malloc(sizeof(*handler));
> +	if (!handler)
> +		return -ENOMEM;
> +
> +	handler->oem_cmd = oem_cmd;
> +	handler->fn = fn;
> +	handler->context = context;
> +
> +	list_add(&sel_handlers, &handler->node);

I'm tempted to say we should check that nothing in the list has the
same oem_cmd as one already there to prevent subtle bugs.

otherwise looks ok.

-- 
Stewart Smith
OPAL Architect, IBM.