boards: Introduce Google Quincy Development Board

[davidcross-chromium]

Quincy is a board created by Google for fingerprint-related functionality development.

[github-actions]

Hello @davidcross-chromium, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[nordicjm]

change commit title to remove "arm:" and have a space after the :'s

[kartben]

@davidcross-chromium will you be coming back to this PR?

[davidcross-chromium]

@davidcross-chromium will you be coming back to this PR?

Yes, apologies for the delay. I've been otherwise occupied lately and am still coming up to speed on some of the GitHub CLI tools needed to modify the PR. I've made many of the updates locally but am having trouble authenticating my CLI to submit. Reviewing docs when bandwidth permits and I expect to resolve soon. Thanks for your patience.

[kartben]

CI failures also need to be addressed. Thanks!

[fabiobaltieri]

I think this wants a CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC here for some of the drivers to work, all other nuvoton boards have it, load of CI builds fail because it's unset https://github.com/zephyrproject-rtos/zephyr/pull/81548/checks?check_run_id=35000988928

[nordicjm]

Should be set in SoC Kconfig defconfig file e.g. https://github.com/zephyrproject-rtos/zephyr/blob/main/soc/adi/max32/Kconfig.defconfig.max32662#L9

[fabiobaltieri]

@MulinChao @ChiHuaL want to follow up on this for NPCX platforms? ^^

[ChiHuaL]

@fabiobaltieri
Sure. To make sure no misunderstanding.
You like a default value of CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC in Nuvoton SoC's Kconfig.default file.
So a board won't fail to build if it doesn't add it in the board default config file explicitly, right?

[fabiobaltieri]

based on @nordicjm comment I think that the idea was to have a default in npcx soc Kconfig that points at a value in devicetree, then the actual value I suppose would be in the soc dtsi files and can be overridden by a board if needed

@nordicjm on a separate note, how feasible do you think it would be to eliminate board defconfig entirely? the current trends seems to suggest that they've been largely abused for stuff that should have been deps, or hardware settings that should arguably have been in dt the whole time (and the migration from a Kconfig to a dt prop is fairly straightforward). One could imagine a long term target of getting rid of those files entirely.

[davidcross-chromium]

I've verified locally that there are no issues removing all of the CONFIGs from google_quincy_defconfig and placing them in the app's prj.conf. There is a lot of precedent in the upstream boards directory for similar default board configs so I'd like to make sure I understand which CONFIGs from google_quincy_defconfig should be moved to the app for this PR?
On a related note from this thread, adding CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=96000000 to google_quincy_defconfig causes the app to break. My understanding is that this is the correct value, but will defer to @MulinChao @ChiHuaL on this issue if we still need this CONIFG added.

[davidcross-chromium]

Please note that I do understand the suggestion to move many of these default CONFIGs to the SOC (not the app) and remove them from the boards. However, this seems like a broader discussion with larger impacts across many boards and potentially outside the scope of this PR. As such, trying to make sure I understand narrowly what specifically should be included in this PR in order to get it approved and am OK to park CONFIGs in the app as a temporary solution if needed.

[fabiobaltieri]

@davidcross-chromium sorry I reused the thread for comments beyond this PR, moving the config is for the Nuvoton folks really since that affects all SoC, removing more stuff is more of a curiosity of what we could further optimize, this board I think at this stage just needs that kconfig I pointed out, I think it's meant to be set at 15000000 no npcx? it's fine to add it here for now, that's where it's at on, for example, boards/nuvoton/npcx9m6f_evb/npcx9m6f_evb_defconfig, we can remove it with the soc change down the road

[davidcross-chromium]

Thanks for clarifying, Fabio. I've added CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=15000000 to the google_quincy_defconfig and left the rest of the defines as is for now but can move if needed. I've tested that 15 MHz works with the app and hardware, but it woud be great if @MulinChao or @ChiHuaL could confirm that this is the optimal value for the NPCX99FPA0BX since latency is important for our application. This is the same value used by other NPCX9 SOCs and EVBs so I guess this should be right/consistent but optimizations welcome. Note that I'd set initially to 96 MHz based on the CPU clock rate used, but it looks like this is more of a HW peripheral clock setting.

[ChiHuaL]

Thanks for clarifying, Fabio. I've added CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=15000000 to the google_quincy_defconfig and left the rest of the defines as is for now but can move if needed. I've tested that 15 MHz works with the app and hardware, but it woud be great if @MulinChao or @ChiHuaL could confirm that this is the optimal value for the NPCX99FPA0BX since latency is important for our application.

CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC should match the frequency of APB2 clock.
The APB2 clock frequency is according to the DTS pcc node and can be calculated by clock-frequency/apb2-prescaler

pcc: clock-controller@4000d000 {
                        clock-frequency = <DT_FREQ_M(90)>; /* OFMCLK runs at 90MHz */
                        core-prescaler = <6>; /* CORE_CLK runs at 15MHz */
                        apb1-prescaler = <6>; /* APB1_CLK runs at 15MHz */
                        apb2-prescaler = <6>; /* APB2_CLK runs at 15MHz */
                        ...
        };

[nordicjm]

HW cycles per sec to be fixed by SoC vendor in a future PR for all in-tree boards

[duda-patryk]

This PR looks good, but sample.net.prometheus fails to compile due to IS_BIT_SET macro collision

/__w/zephyr/zephyr/subsys/net/lib/shell/http.c:16: error: "IS_BIT_SET" redefined [-Werror]
   16 | #define IS_BIT_SET(val, bit) (((val >> bit) & (0x1)) != 0)
      | 
In file included from /__w/zephyr/zephyr/soc/nuvoton/npcx/npcx9/./soc.h:50,
                 from /__w/zephyr/zephyr/modules/cmsis/./cmsis_core_m.h:24,
                 from /__w/zephyr/zephyr/modules/cmsis/./cmsis_core.h:10,
                 from /__w/zephyr/zephyr/include/zephyr/arch/arm/asm_inline_gcc.h:24,
                 from /__w/zephyr/zephyr/include/zephyr/arch/arm/asm_inline.h:18,
                 from /__w/zephyr/zephyr/include/zephyr/arch/arm/arch.h:30,
                 from /__w/zephyr/zephyr/include/zephyr/arch/cpu.h:19,
                 from /__w/zephyr/zephyr/include/zephyr/sys/cbprintf_internal.h:17,
                 from /__w/zephyr/zephyr/include/zephyr/sys/cbprintf.h:124,
                 from /__w/zephyr/zephyr/include/zephyr/logging/log_msg.h:11,
                 from /__w/zephyr/zephyr/include/zephyr/logging/log_core.h:9,
                 from /__w/zephyr/zephyr/include/zephyr/logging/log.h:11,
                 from /__w/zephyr/zephyr/subsys/net/lib/shell/http.c:7:
/__w/zephyr/zephyr/soc/nuvoton/npcx/common/./reg/reg_access.h:13: note: this is the location of the previous definition
   13 | #define IS_BIT_SET(reg, bit)        (((reg >> bit) & (0x1)) != 0)
      | 

soc/nuvoton/npcx/common/reg/reg_access.h:#define IS_BIT_SET(reg, bit)        (((reg >> bit) & (0x1)) != 0)
subsys/net/lib/shell/http.c:#define IS_BIT_SET(val, bit) (((val >> bit) & (0x1)) != 0)

[duda-patryk]

There are 4 definitions of IS_BIT_SET() macro. Maybe it would be good to replace all definitions with one in include/zephyr/sys/util_macro.h or include/zephyr/sys/util.h?

[fabiobaltieri]

There are 4 definitions of IS_BIT_SET() macro. Maybe it would be good to replace all definitions with one in include/zephyr/sys/util_macro.h or include/zephyr/sys/util.h?

not a bad idea, want to open a PR?

[duda-patryk]

There are 4 definitions of IS_BIT_SET() macro. Maybe it would be good to replace all definitions with one in include/zephyr/sys/util_macro.h or include/zephyr/sys/util.h?

not a bad idea, want to open a PR?

Sure. I'll take a look tomorrow.

[duda-patryk]

There are 4 definitions of IS_BIT_SET() macro. Maybe it would be good to replace all definitions with one in include/zephyr/sys/util_macro.h or include/zephyr/sys/util.h?

not a bad idea, want to open a PR?

Sure. I'll take a look tomorrow.

Prepared #83683

[github-actions]

Hi @davidcross-chromium!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁