cpu/msp430fxyz: Fixed input sanitizing in GPIO_PIN

[maribu]

In cpu/msp430fxyz/include/periph_cpu.h the type gpio_t is defined as:

typedef uint16_t gpio_t;

The old version of the macro is:

#define GPIO_PIN(x, y)      ((gpio_t)(((x & 0xff) << 8) | (1 << (y & 0xff))))

Both x and y are sanitized by using bit-wise and. But the y sanitizing is not correct:
1 can be shifted at most 15 times to the left in an uint16_t. But the upper 8 bits are determined by the port x, so only the lower 8 bit should be determined by y. Thus, the correct bitmask is 0x07 to sanitize y.

Update: My original patch and statement was strictly wrong. It should be fixed now.

[maribu]

@haukepetersen: As you are the author of cpu/msp430fxyz/include/periph_cpu.h, could you review this one-liner PR?

[maribu]

Sadly this issue is blocking another pull request, which I'm really interested to get it merged. Maybe @cladmi also has a an msp430 bases board and is able to review and test this one-line PR?

[PeterKietzmann]

@maribu I can give it a look this evening. Can you add a short description how to test this PR in order to see that it fixes the bug?

[maribu]

@PeterKietzmann: Thank you very much. The bug is that the compilation with specific invalid values for x and/or y in GPIO_PIN(x, y) fails. In this PR a test application for the SHT10/SHT11/SHT15 driver is compiled by Murdock against all platforms. If a board does not specify on which pins the SHT1x is connected, the default configuration is used - which happens to be invalid on msp430fxyz based boards. Of course, it does not make much sense to compile the test application and the driver for a board not having such a sensor. But as the GPIO_PIN macro does perform sanitizing of the input already, it should do that imho correctly.

So, I already know that this PR fixes the compilation Bug. I only don't know if it has any regressions, as I don't have the correct hardware to test. So basically, does the GPIO_PIN() still allow to access all pins on all ports? (I would just test the pin with the highest and lowest number on each port and three randomly chosen pins with an LED to reduce the testing effort. If there is a bug in the new sanitizing, it should affect pins with high port or pin numbers. Maybe using the SAUL interface to GPIO controlled LEDs and the saul CLI is an easy way for testing?)

[PeterKietzmann]

@maribu I'm fine with the y operation but I don't understand how 0x0f & x "clears all but the lower 8 bits". Can you investigate on that? It shouldn't hurt though as the MCU has max. 8 pins per port anyway. I ran some tests on the Z1 board but unfortunately the driver doesn't seem to work for all port/pins, neither on master nor with your PR. If we agree on the x sanitation this general" bug shouldn't block your PR.

[maribu]

You're totally right about the x sanitizing. I guess I should stop creating pull requests after long days at work. I feel quite a bit ashamed now of me writing such bullshit. I fixed both the PR description and the PR

[PeterKietzmann]

@maribu no worries, we all know that.

[PeterKietzmann]

ACK. I wasn't able to completely test this PR as the driver seems (partly) broken. However, the change (i) is correct by the description and (ii) does not break the Z1/MSP430 GPIO pins that work on current master. Furthermore, this PR blocks progress in other PRs so I decide for merging it know.