On Fri, Apr 17, 2020 at 01:40:19PM +0200, Linus Walleij wrote:
> This breaks out the measurement code to its own function
> so we can handle this without swirling it up with the
> big switch() statement inside ak8974_read_raw().
>
> Keep a local s16 helper variable for the signed value
> coming out of the measurement before assigning it to the
> integer *val. The local variable makes the code easier
> to read and the compiler will optimize it if possible.
[...]
> +static int ak8974_measure_channel(struct ak8974 *ak8974, unsigned long address,
> + int *val)
> +{
> + __le16 hw_values[3];
> + int ret;
> + s16 outval;
[...]
> + /*
> + * This explicit cast to (s16) is necessary as the measurement
> + * is done in 2's complement with positive and negative values.
> + * The follwing assignment to *val will then convert the signed
> + * s16 value to a signed int value.
> + */
> + outval = (s16)le16_to_cpu(hw_values[address]);
> + *val = outval;
The intermediate 'outval' is not needed. What you describe in the
comment is a normal C integer promotion rule, so I would leave the
comment out, too. IOW, this is equivalent to:
*val = (s16)le16_to_cpu(...);
Otherwise:
Reviewed-by: Michał Mirosław <mirq-linux@xxxxxxxxxxxx>
