I2C and External Interrupt Controller Drivers for Atmel's SAMD21 MCU #9347
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9347 +/- ##
==========================================
- Coverage 52.83% 49.72% -3.12%
==========================================
Files 310 292 -18
Lines 45311 42425 -2886
Branches 10468 9968 -500
==========================================
- Hits 23942 21096 -2846
- Misses 16584 17178 +594
+ Partials 4785 4151 -634
Continue to review full report at Codecov.
|
|
... but you do have lots of issues with coding style noted in the shippable details above :( |
madlain
force-pushed
the
master
branch
5 times, most recently
from
806602b to
665a752
Compare
|
I'm not sure why the shippable run successive to the merge conflict resolution is failing. Can you please help me out? |
|
Is there anything needed from my side to move forward with the merge? |
galak
added
area: I2C
platform: Microchip SAM
|
@anangl can you review the i2c driver. |
MaureenHelm
left a comment
MaureenHelm
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Will you please add the new board to samples/sensor/fxas21002/sample.yaml? This will ensure that the i2c driver gets built in CI.
There was a problem hiding this comment.
Move the sercom compatibles to samd21.dtsi
There was a problem hiding this comment.
I've adhered to the exact same approach taken for the arduino_zero also based on Atmel SAMD21: the SoC device tree file only declares SERCOM devices while the board device tree file specifies how a given SERCOM is configured (USART, SPI or I2C) for that board. Is there anything wrong with this approach?
drivers/gpio/gpio_sam0.c
Outdated
There was a problem hiding this comment.
These should be part of the const config struct
There was a problem hiding this comment.
The porta_extints array allows using designated initializers for both the
mapping table and its length in the const config struct. Moving the definition
of the table to the const config struct prevents using a designated initializer
for the length and defers its computation to run time, using pointer arithmetic
in the device init function.
drivers/gpio/gpio_sam0.c
Outdated
There was a problem hiding this comment.
Instead of doing this, you can get the device struct using the CONTAINER_OF macro.
There was a problem hiding this comment.
Not sure how this macro could help here. This code is in the Interrupt Service
Routine for the External Interrupt Controller where the only way to get a
reference to the structure for the GPIO ports is to use their respective device
names with DEVICE_GET.
madlain
force-pushed
the
master
branch
5 times, most recently
from
6babb30 to
73f122b
Compare
|
All checks are passing now. Review history of this comment for details about previous failed status. |
madlain
force-pushed
the
master
branch
2 times, most recently
from
dd86549 to
d3274b0
Compare
|
Hi, Thanks for the review and the valuable comments. I’ve addressed and resolved all but three of them for which I’m expecting your feedback. Also as clarification, and to answer your top-level comment, the board doesn't carry the FXAS21002 gyro. At the time I implemented the I2C and EIC drivers, it was possible to build the sample app for the sensor with an external gyro board hooked up to the MKRZero. This set-up provided a convenient way to exercise the newly developed I2C and EIC drivers all together with the already existing drivers for the SoC (UART, GPIO, PINMUX). |
madlain
force-pushed
the
master
branch
2 times, most recently
from
bd4fe8a to
03e5345
Compare
Only the most basic operation has been tested. Current limitations include: - the use of polling - the rudimentary handling of concurrency - support for standard mode only Signed-off-by: Madani Lainani <[email protected]>
This extends the existing SAMD21 GPIO driver functionnality with the possibility to configure I/O pins as interrupt lines. Signed-off-by: Madani Lainani <[email protected]>
Built upon the existing SoC for Atmel's SAMD21 MCU but with support enabled for two newly added peripherals: - I2C port (SERCOM) - External Interrupt Controller Signed-off-by: Madani Lainani <[email protected]>
|
Closing PR as its been marked stale for several months. Feel free to re-open and update if desired. |
7 participants
Hi,
I've implemented a basic I2C driver and an external interrupt controller driver for Atmel's SAMD21 MCU. I've performed functional testing for both by exercising the sample application for the FXAS21002 gyroscope running on an Arduino MKRZero board connecting to Adafruits NXP Precision 9DoF breakout board.
Madani.