Re: [PATCH] ipmi: add SET_SENSOR_READING command

[Corey Minyard]

On Mon, Nov 18, 2019 at 10:24:29AM +0100, Cédric Le Goater wrote:
> SET_SENSOR_READING is a complex IPMI command (see IPMI spec 35.17)
> which enables the host software to set the reading value and the event
> status of sensors supporting it.
> 
> Below is a proposal for all the operations (reading, assert, deassert,
> event data) with the following limitations :
> 
>  - No event are generated for threshold-based sensors.
>  - The case in which the BMC needs to generate its own events is not
>    supported.

Ok, I've included this in my tree.  I made one small change mentioned
below.  Beyond that, I think you could make this function shorter, but I
think that would actually make it harder to understand.  Breaking it
into multiple functions doesn't make sense to me, either.

If you are including this in the ppc tree:

Acked-by: Corey Minyard <address@hidden>

with the change below and I can remove it from mine.

> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> Reviewed-by: Corey Minyard <address@hidden>
> ---
> +
> +    switch (do_gen_event) {
> +    case SENSOR_GEN_EVENT_DATA: {
> +        unsigned int bit = evd1 & 0xf;
> +        uint16_t mask = (1 << bit);
> +
> +        if (sens->assert_states & mask & sens->assert_enable) {
> +            gen_event(ibs, cmd[2], 0, evd1, evd2, evd3);
> +        }
> +
> +        if (sens->deassert_states & mask & sens->deassert_enable) {
> +            gen_event(ibs, cmd[2], 1, evd1, evd2, evd3);
> +        }
> +    }
> +        break;

I moved this break statement above the brace before it to keep the
indention consistent.  It just screwed with my brain too much :).

I looked and there is nothing in the coding style about this, and I
found this done in three different ways:

  case x: {  /* in vl.c */
      ....
      break;
  }
  case y: /* in thunk.c */
      {
           ....
      }
      break;
  case z: /* In vl.c */
  {
      ....
      break;
  }

Oddly enough, I didn't find anything about this in the Linux coding
style document, either (I was curious).  One could argue, I suppose,
that according to the "Block structure" section in the qemu style and
the similar section in the Linux style that the first is correct,
but then case statements violate the "Every indented statement is
braced" statement in the qemu style.  This has always bugged me in
C, sorry for the diatribe on this.

-corey

> +    case SENSOR_GEN_EVENT_BMC: