drv: ft8xx: new coprocessor commands #82376
Conversation
The ft8xx driver reported a compile-time error caused by an invalid logging configuration of this module. This patch intorduces FT800-specific logging configuration in Kconfig that is used by the driver. This way the error is fixed. Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
56cab5d to
6837dda
Compare
| @@ -73,6 +78,183 @@ void ft8xx_copro_cmd_swap(void) | |||
| ft8xx_copro_cmd(CMD_SWAP); | |||
| } | |||
|
|
|||
| void ft8xx_copro_cmd_fgcolor(uint32_t c) | |||
There was a problem hiding this comment.
is c for color ? would use the full term :)
There was a problem hiding this comment.
I think so. It's how the programming guide defines it:
I would prefer to keep functions prototypes like in the programming guide.
| const uint16_t cmd_size = sizeof(CMD_FGCOLOR) + | ||
| sizeof(c); | ||
|
|
||
| while (ram_cmd_freespace() < cmd_size) { |
There was a problem hiding this comment.
Is this function a blocking call? Probably it should be refelected in documentation as well :) same goes for other functions as well.
There was a problem hiding this comment.
In an RTOS I expect a function to be blocking unless it's explicitly stated that it's asynchronous and described how the caller is notified when the operation ends.
Implement more co-processor commands in the FT8xx display driver. The list of the new implemented commands consists of: * CMD_FGCOLOR * CMD_BGCOLOR * CMD_SLIDER * CMD_TOGGLE * CMD_TRACK Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
6837dda to
8b4766e
Compare
| int16_t w, | ||
| int16_t h, |
There was a problem hiding this comment.
single letter arguments are ok for local variables, bad idea for external functions, x and y are obviously ok "w" "h" "s" should really be expanded, I imagine you did it because that's how the existing apis are declared but I'd suggest changing them all to something meaningful, it's not like it would break existing code anyway
| } | ||
|
|
||
| ft8xx_wr32(FT800_RAM_CMD + reg_cmd_write, CMD_SLIDER); | ||
| increase_reg_cmd_write(sizeof(CMD_SLIDER)); |
There was a problem hiding this comment.
Since every instance of ft8xx_wr is always followed by increase_reg_cmd_write maybe it makes sense to combine them? The separation into more lines does not add anything of value imo.
|
I've decided to add more improvement to the drivers in smaller steps. Let's start with less controversial one: #83851 |
|
@hubertmis please amend your commits instead of closing and opening new PRs. |
Implement more co-processor commands in the FT8xx display driver. The list of the new implemented commands consists of: