Hi @benys and thanks a lot for your contribution.

Your PR contains lots of added features in a single commit. In order to make the review easier you will have to split it up a little bit.

I suggest to open a dedicated PR for:

• Board and SOC definitions
• CAN driver addition
• The fix for the ADC driver

Also the commit messages have to follow the guidelines stated here in order to make it through the CI.

Hi @martinjaeger it is no problem to split changes. I would like to clarify:
a) create soc/board without CAN & ADC
b) create standalone changes to ADC (to fix compliation) and add ADC definition
c) after merge a) & b) add CAN driver?

No problem for me, but I need wait for HAL approval - becouse many problems with compliation is caused by no HAL files.

OK?

Yes, looks like a good approach. Whether the SOC and board stuff need to be split further (I've seen some new clock-control bindings, but no driver implementation) would be up to the ATSAM maintainer @nandojve.

Please note that we are currently in feature freeze for v3.2 release, so the new additions can only be merged after the release. Assuming the ADC fix is a bug, it could be merged before the release already.

@martinjaeger ok I created #50388.
I am waiting for approve HAL and #50388 to make future PR (with only soc/board and CAN)

a) create soc/board without CAN & ADC
b) create standalone changes to ADC (to fix compliation) and add ADC definition
c) after merge a) & b) add CAN driver?

Sounds good. The main reason for splitting into several PRs, as pointed out by others, is to keep the size of the PR down - but you also need to consider that different features like these are likely to be reviewed by different groups of people. By splitting the PR into logically separate changes you also allow for the PRs to have the reviewers assigned that really care for and know the details of the topic in question.

@nandojve I think that I have last problem with this PR. HAL defines C20, C20N, C21, C21N folders (series).

So I discovered that this implicates that I need make C2x and C2x soc series.

/zephyrproject/zephyr/soc/arm/atmel_sam0/samc21/soc.h:48:10: fatal error: samc21n18a.h: No such file or directory
   48 | #include <samc21n18a.h>
      |          ^~~~~~~~~~~~~~

becouse atmel hal searches includes like this:

zephyr_include_directories(${CONFIG_SOC_SERIES})

So I tried

config SOC_SERIES
	default "samc21n" if SOC_PART_NUMBER_SAMC21N17A
	default "samc21n" if SOC_PART_NUMBER_SAMC21N18A
	default "samc21"

so this implicates configuration SOC_SERIES_SAMC21 and SOC_SERIES_SAMC21N. Even I discovered that maybe I need create separated samc21 samc21n folder in soc/atmel/atmel_sam0

But easier way maybe is change at atmel hal include for something like

if c21xxn or c20xxn
zephyr_include_directories(${CONFIG_SOC_SERIES}n)
else 
zephyr_include_directories(${CONFIG_SOC_SERIES})

Yes, there is this implication. However, we need add a solution that may solve future problems without give us more work.
Please add a commit (after the update manifest) with following content and I as co-author:

diff --git a/soc/arm/atmel_sam0/Kconfig b/soc/arm/atmel_sam0/Kconfig
index ebda1a5179..a11cf9b95a 100644
--- a/soc/arm/atmel_sam0/Kconfig
+++ b/soc/arm/atmel_sam0/Kconfig
@@ -18,5 +18,6 @@ source "soc/arm/atmel_sam0/common/Kconfig.saml2x"
 source "soc/arm/atmel_sam0/common/Kconfig.samd2x"
 source "soc/arm/atmel_sam0/common/Kconfig.samd5x"
 source "soc/arm/atmel_sam0/*/Kconfig.soc"
+source "soc/arm/atmel_sam0/Kconfig.soc.revisions"
 
 endif
diff --git a/soc/arm/atmel_sam0/Kconfig.soc.revisions b/soc/arm/atmel_sam0/Kconfig.soc.revisions
new file mode 100644
index 0000000000..a152528ac7
--- /dev/null
+++ b/soc/arm/atmel_sam0/Kconfig.soc.revisions
@@ -0,0 +1,10 @@
+# Copyright (c) 2022 Gerson Fernando Budke <[email protected]>
+# SPDX-License-Identifier: Apache-2.0
+
+config SOC_SERIES_REVISION_N
+       bool
+
+config SOC_SERIES_REVISION
+       string
+       default "n" if SOC_SERIES_REVISION_N
+       default ""

Add this patch at hal_atmel:

diff --git a/asf/sam0/include/CMakeLists.txt b/asf/sam0/include/CMakeLists.txt
index d273adb..77e86df 100644
--- a/asf/sam0/include/CMakeLists.txt
+++ b/asf/sam0/include/CMakeLists.txt
@@ -1 +1 @@
-zephyr_include_directories(${CONFIG_SOC_SERIES})
+zephyr_include_directories(${CONFIG_SOC_SERIES}${CONFIG_SOC_SERIES_REVISION})

Then, on your commits you can just add:

diff --git a/soc/arm/atmel_sam0/samc21/Kconfig.soc b/soc/arm/atmel_sam0/samc21/Kconfig.soc
index 3f82572539..f3f324abf6 100644
--- a/soc/arm/atmel_sam0/samc21/Kconfig.soc
+++ b/soc/arm/atmel_sam0/samc21/Kconfig.soc
@@ -51,8 +51,10 @@ config SOC_PART_NUMBER_SAMC21J18AU
 
 config SOC_PART_NUMBER_SAMC21N17A
        bool "SAMC21N17A"
+       select SOC_SERIES_REVISION_N
 
 config SOC_PART_NUMBER_SAMC21N18A
        bool "SAMC21N18A"
+       select SOC_SERIES_REVISION_N
 
 endchoice

@nandojve nice solution. I am still learning zephyr philosophy :)

I made all changes.

(Edit: I hope that I fix lint problems)

Hi @nandojve I think you can merge it ;)

After merge I want to extend my PR with CAN support (#50408) with this SOC:)

Thanks a lot!

I see that PR wit ADC is completed. Maybe I will add devicetrees related with ADC in this PR in my third commit :)?

I see that PR wit ADC is completed. Maybe I will add devicetrees related with ADC in this PR in my third commit :)?

Yes, please do it. Make sure that tests are running using twister as I already mentioned for CI side and run a test on board to make sure everything is in place.

Yes, please do it. Make sure that tests are running using twister as I already mentioned for CI side and run a test on board to make sure everything is in place.

@nandojve twister works fine, only some tests present "test mismatch", but if I check some of them that it present 0 failed.

Almost there... : -)

No problem, I am patient :-)