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 07:02:45 UTC
[GitHub] [incubator-nuttx] btashton opened a new pull request #1816: input: Add SPI mode for STMPE811 touch screen controller
btashton opened a new pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816
## Summary
Add SPI mode for STMPE811 touch screen controller. This controller can be used both in I2C and SPI modes, but only the SPI mode had been implemented.
## Impact
Nothing should change for any existing configuration that uses i2c.
## Testing
Tested with an nRF52 using the Keyboard FeatherWing. Note that this board does not have the INT line wired up so the interrupt is being faked by a worker.
I am running basic testing using the touch screen example:
```
tc_main: Reading...
stmpe811_read: len=16
tc_main: Bytes read: 16
Sample :
npoints : 1
Point 1 :
id : 215
flags : 3a
x : 0
y : 0
h : 0
w : 0
pressure : 0
tc_main: Reading...
stmpe811_read: len=16
tc_main: Bytes read: 16
Sample :
npoints : 1
Point 1 :
id : 215
flags : 3a
x : 20046
y : 26985
h : 0
w : 0
pressure : 1
tc_main: Reading...
stmpe811_read: len=16
tc_main: Bytes read: 16
Sample :
npoints : 1
Point 1 :
id : 215
flags : 3a
x : 0
y : 0
h : 0
w : 0
pressure : 0
tc_main: Reading...
stmpe811_read: len=16
tc_main: Bytes read: 16
Sample :
npoints : 1
Point 1 :
id : 215
flags : 3c
x : 0
y : 0
h : 0
w : 0
pressure : 0
tc_main: Reading...
stmpe811_read: len=16
```
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694300865
>
>
> @patacongo did you have any thoughts on the header file issue?
Appears to be a classic name collision. GPIO_PIN is too generic and should be better qualified by putting it into a name space... in both header files: Like STMPE811_GPIO_PION and NRF52_GPIO_PIN.
But stmpe811.h appears to be the only outlier:
$ 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 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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694342005
I prepared a PR to fix the name collision. See PR #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 commented on pull request #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694342005
I am preparing a PR to fix the name collision.
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694032597
Just noticed an issue with the 16bit read. Fixing 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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694006589
@patacongo I have a related question about this driver. It seems like a lot more is exposed in the `stmpe811.h` header than we should in the "public" interface. This is causing issues when I add the board integration since it defines `GPIO_PIN(n)` which is a common define for chips.
```
In file included from nrf52-feather.h:46,
from nrf52_stmpe811.c:42:
/home/bashton/nuttx/apache/incubator-nuttx/arch/arm/src/chip/nrf52_gpio.h:178: warning: "GPIO_PIN" redefined
# define GPIO_PIN(n) ((n) << GPIO_PIN_SHIFT)
In file included from nrf52_stmpe811.c:34:
/home/bashton/nuttx/apache/incubator-nuttx/include/nuttx/input/stmpe811.h:282: note: this is the location of the previous definition
#define GPIO_PIN(n) (1 << (n))
```
----------------------------------------------------------------
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 a change in pull request #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#discussion_r490291913
##########
File path: drivers/input/stmpe811_base.c
##########
@@ -422,18 +443,21 @@ uint8_t stmpe811_getreg8(FAR struct stmpe811_dev_s *priv, uint8_t regaddr)
*
****************************************************************************/
-#ifdef CONFIG_STMPE811_I2C
void stmpe811_putreg8(FAR struct stmpe811_dev_s *priv,
uint8_t regaddr, uint8_t regval)
{
/* 8-bit data read sequence:
*
- * Start - I2C_Write_Address - STMPE811_Reg_Address - STMPE811_Write_Data - STOP
+ * Start - I2C_Write_Address - STMPE811_Reg_Address -
Review comment:
```suggestion
* i2c:
* Start - I2C_Write_Address - STMPE811_Reg_Address -
```
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694300865
>
>
> @patacongo did you have any thoughts on the header file issue?
Appears to be a classic name collision. GPIO_PIN is too generic and should be better qualified by putting it into a name space... in both header files: Like STMPE811_GPIO_PION and NRF52_GPIO_PIN.
But stmpe811.h appears to be the only outlier:
$ 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))
So I think only stmpe811.h needs renaming.
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694052953
> Just noticed an issue with the 16bit read. Fixing that now.
Resolved the issue. I was marking the start read (0x80) on the LSB address as well which is not correct and would cause it to read the previous address. I updated the PR description to now show taps in 3 of the 4 screen corners.
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694300865
>
>
> @patacongo did you have any thoughts on the header file issue?
Appears to be a classic name collision. GPIO_PIN is too generic and should be better qualified by putting it into a name space... in both header files: Like STMPE811_GPIO_PION and NRF2_GPIO_PIN.
----------------------------------------------------------------
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 merged pull request #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694296090
@patacongo did you have any thoughts on the header file issue?
----------------------------------------------------------------
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 #1816: input: Add SPI mode for STMPE811 touch screen controller
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1816:
URL: https://github.com/apache/incubator-nuttx/pull/1816#issuecomment-694300865
>
>
> @patacongo did you have any thoughts on the header file issue?
Appears to be a classic name collision. GPIO_PIN is too generic and should be better qualified by putting it into a name space... in both header files: Like STMPE811_GPIO_PION and NRF52_GPIO_PIN.
----------------------------------------------------------------
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