Re: right bit shift error in ad7314.c ?

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Jeroen,

On Tue, Dec 16, 2014 at 3:21 PM, Jeroen De Wachter
<jeroen.de.wachter@xxxxxxxxxx> wrote:
> That driver reads a status from the SPI bus that contains the temperature
> read by the sensor. There's some other information in that status too, so
> some bit manipulation is done to get the temperature data:
>
> s16 data;
> int ret;
> ...
> data = (ret & AD7314_TEMP_MASK) >> AD7314_TEMP_SHIFT;
> data = (data << 6) >> 6;
>
> Before that last line is executed, the temperature data is in the lowest
> 10 bits of the data variable. To be able to handle negative temperatures,
> those 10 bits get shifted to the left (discarding higher bits) and then
> shifted back to the right, with what is assumed to be an Arithmetic Shift
> Right, which takes into account the 2's complement content of the lowest
> 10 bits.
>
> However, the implementation of right shift on a signed integer is not
> defined in the C standard and is implementation-dependent [1].

Have you tried using a signed division instead?

The following seems to work for me:

        data = (s16)(data << 6) / 64;

Without the cast, "data << 6" seems to be promoted to an unsigned type.

Alternatively, you can split it in two parts, to force a signed intermediate
result:

        data <<= 6;
        data /= 64;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html