You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/09/17 16:29:30 UTC
[GitHub] [incubator-nuttx] patacongo opened a new pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
patacongo opened a new pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832
## Summary
The macro name GPIO_PIN collides with naming used by many architectures as noted in PR #1816:
$ find . -name "*.h" | xargs grep "define GPIO_PIN[(]"
./arch/arm/src/am335x/hardware/am335x_gpio.h:#define GPIO_PIN(n) (1 << ((n) & 0x1f)) /* Bit n: Pin n, n=0-31 */
./arch/arm/src/imx6/hardware/imx_gpio.h:#define GPIO_PIN(n) (1 << (n)) /* Bit n: Pin n, n=0-31 */
./arch/arm/src/imxrt/hardware/imxrt_gpio.h:#define GPIO_PIN(n) (1 << (n)) /* Bit n: Pin n, n=0-31 */
./arch/arm/src/lpc43xx/hardware/lpc43_gpio.h:#define GPIO_PIN(p) (1 << (p)) /* Bits 0-31: Read/write pin state */
./arch/arm/src/nrf52/nrf52_gpio.h:# define GPIO_PIN(n) ((n) << GPIO_PIN_SHIFT)
./arch/arm/src/nuc1xx/hardware/nuc_gpio.h:#define GPIO_PIN(n) (1 << (n)) /* Bit n: GPIOx Pin[n] pin value */
./arch/avr/src/at32uc3/at32uc3_gpio.h:#define GPIO_PIN(n) (1 << (n))
./include/nuttx/input/stmpe811.h:#define GPIO_PIN(n) (1 << (n))
This commit changes the name used by the STMPE811 driver to STMPE11_GPIO_PIN
## Impact
No impact is anticipated. This is a name change only. No functionality change.
## Testing
PR check only. There is no functional change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694395999
@patacongo does this definition make sense for `STMPE811_GPIO_PIN(n)` I would expect to do something like this to define the pin config:
```
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
```
But that sets the bit shift in the register not pin config. Instead I have to do something like this:
```
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
```
That seems inconsistent to me with how we normally set GPIO pincfg.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694431378
>
>
> @patacongo does this definition make sense for `STMPE811_GPIO_PIN(n)` I would expect to do something like this to define the pin config:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
> ```
>
> But that sets the bit shift in the register not pin config. Instead I have to do something like this:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
> ```
>
> That seems inconsistent to me with how we normally set GPIO pincfg.
What is there seems correct to me. The encoding is "Bits 0-2 is the pin number". For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
That would be Bits 0-2 == 0b011. For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
STMPE811_GPIO_PIN(3) ix 0b1000 sowhich would not fit into Bits 0-2 and would clobber Bit 3: "Bit 3 is the initial value for output pins"
This is the full encoding:
389 /* The STMPE811 GPIO interfaces take an 8-bit bit-encoded parameter to describe the GPIO pin.
390 * The following definitions describe the bit-encoding of that parameter.
391 *
392 * 7654 3210
393 * ---- ----
394 * DIRF VPPP
395 *
396 * Input Pins: 1IRF .PPP
397 *
398 * Output Pins: 0... VPPP
399 *
400 * Bits 7 is the pin direction.
401 */
407 /* Bit 6 indicates that the pin will generate an interrupt (inputs only) */
411 /* The bits 4-5 select the rising and/or the falling edge detection. */
416 /* Bit 3 is the initial value for output pins */
422 /* Bits 0-2 is the pin number */
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694431378
>
>
> @patacongo does this definition make sense for `STMPE811_GPIO_PIN(n)` I would expect to do something like this to define the pin config:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
> ```
>
> But that sets the bit shift in the register not pin config. Instead I have to do something like this:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
> ```
>
> That seems inconsistent to me with how we normally set GPIO pincfg.
What is there seems correct to me. The encoding is "Bits 0-2 is the pin number". For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
That would be Bits 0-2 == 0b011. For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
STMPE811_GPIO_PIN(3) ix 0b1000 sowhich would not fit into Bits 0-2 and would clobber Bit 3: "Bit 3 is the initial value for output pins"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694440871
Yeah I understand why, just the naming through me off, but there is not really a "standard" here and this is documented, so I say we just merge this in.
My input issue is not related to the driver it is related to the hardware. There is no pull-up on the SD Card detect pin so it goes between floating and ground which is not useful since this chip cannot set an internal pull-up. I verified the reads is correct.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton merged pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
btashton merged pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694431378
>
>
> @patacongo does this definition make sense for `STMPE811_GPIO_PIN(n)` I would expect to do something like this to define the pin config:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
> ```
>
> But that sets the bit shift in the register not pin config. Instead I have to do something like this:
>
> ```
> #define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
> ```
>
> That seems inconsistent to me with how we normally set GPIO pincfg.
What is there seems correct to me. The encoding is "Bits 0-2 is the pin number". For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN3)
That would be Bits 0-2 == 0b011. For:
#define KB_FW_BL (STMPE811_GPIO_OUTPUT | STMPE811_GPIO_PIN(3))
STMPE811_GPIO_PIN(3) ix 0b1000 which would not fit into Bits 0-2 and would clobber Bit 3: "Bit 3 is the initial value for output pins"
This is the full encoding:
389 /* The STMPE811 GPIO interfaces take an 8-bit bit-encoded parameter to describe the GPIO pin.
390 * The following definitions describe the bit-encoding of that parameter.
391 *
392 * 7654 3210
393 * ---- ----
394 * DIRF VPPP
395 *
396 * Input Pins: 1IRF .PPP
397 *
398 * Output Pins: 0... VPPP
399 *
400 * Bits 7 is the pin direction.
401 */
407 /* Bit 6 indicates that the pin will generate an interrupt (inputs only) */
411 /* The bits 4-5 select the rising and/or the falling edge detection. */
416 /* Bit 3 is the initial value for output pins */
422 /* Bits 0-2 is the pin number */
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694437954
STMPE811_GPIO_PIN() is an internal driver definition and not part of the user interface. If you want, you can just close this PR and move the definition to drivers/input/stmpe811.h
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694406946
Other than that I have verified that I am able to write to the GPIO pins with this. Something is not working with reading, but I am looking into that now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #1832: STMPE811: Fix name collision in include/nuttx/input/stmpe811.h
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1832:
URL: https://github.com/apache/incubator-nuttx/pull/1832#issuecomment-694375840
Thanks Greg, I'll give it a quick test in my config just for sanity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org