GPIO protocol, Maaxboard support - #619
Conversation
Do you mind just adding the build time tests at least so the CI checks it compiles until we get run-time tests working. |
midnightveil
left a comment
There was a problem hiding this comment.
Ignoring the protocol design and some of the pointless comments, this seems mostly OK.
| /* | ||
| * We have two potential interrupts to deal with, one for the lower 16 pins, and another for the | ||
| * the upper 16 pins. | ||
| * There exists single interrupt lines for the device, however they do not appear to leave the GPIO |
There was a problem hiding this comment.
| * There exists single interrupt lines for the device, however they do not appear to leave the GPIO | |
| * There exists a single interrupt line for the device, however the interrupts do not appear to leave the GPIO |
I think that's what you meant?
There was a problem hiding this comment.
note that I didn't write any of this! this was Tristan's work that I am picking up. But yeah probably what he meant.
|
LGTM, left some nitpicks, mostly grammar and some minor adjustments. |
052f023 to
b3b15b0
Compare
|
Blocked on me releasing a new sdfgen version with the GPIO changes, but it seems like the latest commit broke it and the driver no longer compiles. |
5c95811 to
04119a4
Compare
2f0309a to
fa1e780
Compare
|
NOTE: is correct as it is now. Just pending on sdfgen release. |
fa1e780 to
7cc7a2c
Compare
Signed-off-by: Lesley Rossouw <lesley.rossouw@unsw.edu.au> Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Lesley Rossouw <lesley.rossouw@unsw.edu.au> Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Lesley Rossouw <lesley.rossouw@unsw.edu.au> Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Lesley Rossouw <lesley.rossouw@unsw.edu.au> Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
…dated GPIO to use board.py Signed-off-by: Lesley Rossouw <lesley.rossouw@unsw.edu.au> Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
7cc7a2c to
3622efe
Compare
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
Signed-off-by: Ivan Velickovic <i.velickovic@unsw.edu.au>
| ) | ||
| from board import BOARDS | ||
|
|
||
| assert version("sdfgen").split(".")[1] == "29", "Unexpected sdfgen version" |
This PR re-introduces Tristan's GPIO changes for the Maaxboard. Original PR message below. I've also updated the sdf_gen PR accordingly au-ts/microkit_sdf_gen#20. Several (philosophical / design) issues exist with this version of the protocol, but we have discussed just merging this in and fixing them later as it's good enough for now. This is needed for several projects we are working on right now, especially the LionsOS robot. Currently we don't have CI tests for this protocol because there's not an easy way to test without a dedicated external tester circuit - see #618.
Includes:
GPIO protocol
Client
IMX GPIO driver
NOTE:
The GPIO sdfgen branch must be merged as well.
Current Design
When a digtial I/O pin is not being used for any specific purpose (i2c, uart) it is implicitly converted to a GPIO pin.
There is a single driver
No virtualiser but instead everything is assigned in the system file and a config file containing gpio_driver_channel_mappings.
These mappings determine which GPIO or IRQ is assigned to which driver channel which thus means GPIOs and IRQs can only be configured and interacted with through those channels at runtime.
Due to there only being 62 channels in the driver, num_gpios + num_device_irqs <= 62
Driver
Controls both basic GPIO functionality and IRQ control through hardware interaction.
Direction: Configure the GPIO as input or output.
Value: Read or write the logical level (high or low).
Interrupts: Configure edge detection (rising/falling/both) for GPIO-based IRQs.
Receives interupts from the device and forwards them to the corresponding client based on gpio_driver_channel_mappings.
Receives requests in the form of PPCs from the Clients, performs request then returns the outcome.
Clients
Clients only interact with the driver through PPC requests which is defined in sddf/include/gpio/gpio.h and under the PPC heading below
Although currently not permitted if we see the need in the future there are other platform specific configurations the client potentially can use.
There is a client library as well which abstract the PPC mechanism away as well.
PPC
This is the only way clients interact with the driver
Specifc implementation details are in sddf/include/gpio/protocol.h (see below)
Interupts
The driver implements a fast lookup table mechanism to forward IRQs recieved from the device to the appropriate client.
Protocol.h
GPIO DRIVER CLASS PROTOCOL
This is modelled as similiar as possible to:
include/linux/gpio/driver.h
Part of the gpio_chip also is an irq_chip so that functionality is modelled as simliar as possible to:
include/linux/irq.h
DESIGN :
For the GPIO driver we never need to transfer that much infomation. Thus for now our protocol tries to condense all the infomation into 20 bits. The choice for 20 bits comes from the fact that the amount of words in the sel4 IPC fastpath is 1 word in the worst case architecure (x86). This first word is always reserved for the message tag thus we try to fit protocol data in that. Now the tag contains label, caps unwrapped, #caps, msg length where the label is for us to define semantics for. The other fields not including label add up to 12 bits.
field capsUnwrapped 3
field extraCaps 2
field length 7
So to account for the worst case which is a 32 bit system means we only get 20 bits in the label to do what we need. NOTE: all the current platforms are little endian as well
REQUESTS:
We currently are splitting the fields up like this
Bits [0 : 9]
will contain the interface enum SDDF_GPIO_interface_t;
Bits [10 : 19]
will contain the config, value or type
MR(0)
In some cases can be used for a config argument as well
RESPONSES:
can currently all fit into the label
the twentieth bit marks if its an error or not (1 is an error)
the other bits marks the return value
gpio_config.h
Contains which pins (and irq) are to be assigned to which channels at build time.
Looks something like this
static const GPIO_driver_channel_t gpio_driver_channel_mappings[NUM_DRIVER_CHANNELS] = {
[0] = { PIN_1, IRQ_UNUSED },
[1] = { PIN_2, IRQ_1 },
UNUSED_CH(2),
[3] = { PIN_3, IRQ_UNUSED },
}