-
Notifications
You must be signed in to change notification settings - Fork 8.1k
boards: add Arduino UNO Q #97118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
boards: add Arduino UNO Q #97118
Conversation
status = "okay"; | ||
pinctrl-0 = <&i2c4_scl_pd12 &i2c4_sda_pd13>; | ||
pinctrl-1 = <&i2c4_scl_pf14 &i2c4_sda_pf15>; | ||
pinctrl-names = "default", "sleep"; /* TODO: sleep is jmisc mux */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be worth explanding the comment, not very clear at the moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have connected the same CPU peripheral to multiple sets of pins and need a solution to choose which pinmux must be applied at runtime; the choice of sleep
was just a helpful placeholder.
Converted to regular comments and will submit a separate PR to provide this functionality.
pinctrl-0 = <&spi2_sck_pb13 | ||
&spi2_miso_pb14 &spi2_mosi_pb15 &spi2_nss_pb9>; | ||
pinctrl-1 = <&spi2_sck_pd1 | ||
&spi2_miso_pc2 &spi2_mosi_pc3>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: that's a weird way to indent this, maybe do two per line? or use #92334 if you feel like trying it, worked well from what I've seen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried downloading the latest available dts-linter
(0.3.0-beta.5), but it did not flag or correct those. Fixed them by hand 👍
|
||
&gpiog { | ||
status = "okay"; | ||
}; No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing eof newline, fix your editor
boards/arduino/uno_q/board.cmake
Outdated
# Flash merged TF-M + Zephyr binary | ||
set_property(TARGET runners_yaml_props_target PROPERTY hex_file tfm_merged.hex) | ||
|
||
if (CONFIG_HAS_FLASH_LOAD_OFFSET) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check the compliance error, this one is very uninsightful for some reasons but I think it's complaining about the space after the if
@rugeGerritsen can you take a look? The error only shows
boards/arduino/uno_q/board.cmake:8 CMakeStyle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has to be under 100k or somehing, crop the whitespace and pass it through https://tinywebp.app/ or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has to be under 100k or somehing, crop the whitespace and pass it through tinywebp.app or something
maybe use this one? It has transparent background and has the merit of being an actual picture, not a 3d render with lots of stuff apparenlty being edited out
|
||
.. code-block:: console | ||
|
||
adb forward tcp:3333 tcp:3333 && adb shell arduino-debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fancy, I presume this has to be run from the application processor? May be worth expanding a bit on it, or maybe just like the official programming documentation
I wonder what the "Q" stands for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fabiobaltieri for the review, comments should be addressed.
pinctrl-0 = <&spi2_sck_pb13 | ||
&spi2_miso_pb14 &spi2_mosi_pb15 &spi2_nss_pb9>; | ||
pinctrl-1 = <&spi2_sck_pd1 | ||
&spi2_miso_pc2 &spi2_mosi_pc3>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried downloading the latest available dts-linter
(0.3.0-beta.5), but it did not flag or correct those. Fixed them by hand 👍
status = "okay"; | ||
pinctrl-0 = <&i2c4_scl_pd12 &i2c4_sda_pd13>; | ||
pinctrl-1 = <&i2c4_scl_pf14 &i2c4_sda_pf15>; | ||
pinctrl-names = "default", "sleep"; /* TODO: sleep is jmisc mux */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have connected the same CPU peripheral to multiple sets of pins and need a solution to choose which pinmux must be applied at runtime; the choice of sleep
was just a helpful placeholder.
Converted to regular comments and will submit a separate PR to provide this functionality.
Sorry for the noise, did a pretty bad rebase 👎 |
I can confirm the the current sha 94c74f9 produced the right formatting
I have tried the linter on c080801 and I got the diff see attached, can you share how you ran it so I can fix any issues present. |
Thanks for confirming! Then there is no issue. I guess we were both surprised that the lines Fabio highlighted : zephyr/boards/arduino/uno_q/arduino_uno_q-common.dtsi Lines 133 to 136 in c080801
did not trigger any rewrite even if they definitely look ugly, but you confirmed they don't breach any existing rules. Also, I'm not sure we want to enforce a reformat for those: it will be a pretty invasive change whatever direction is chosen (one per line, reflow to some width etc). |
No there are no such rules in place at the moment. The only rules I am planning to add in the near future is to reflow when we exceed 100 char and only if we can physically split the line. |
scratch_partition: partition@e0000 { | ||
label = "image-scratch"; | ||
reg = <0x000e0000 DT_SIZE_K(64)>; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a change request but swap using scratch should only be used if you have different sector sizes in one slot or between both slots
@@ -0,0 +1,11 @@ | |||
# SPDX-License-Identifier: Apache-2.0 | |||
|
|||
# enable uart driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capitalise first letters of comments and acronyms like UART
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking: Use of -common.dtsi file feels useless now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally left this to simplify porting changes from the ST eval board, but yes, it has no other purpose now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2:
- removed support for TF/M
- fixed Twister CI issues
- all comments should be addressed
@nordicjm please update your review, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally left this to simplify porting changes from the ST eval board, but yes, it has no other purpose now.
Uses an STM32U585 microcontroller. Some drivers will need to be developed (led matrix, internal RPC) Signed-off-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
Enable relevant Twister tests for the Arduino Uno Q board. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
f7a59af
|
Uses an STM32U585 microcontroller.
Some drivers will need to be developed (led matrix, internal RPC)