Skip to content

Conversation

@iperry
Copy link
Contributor

@iperry iperry commented Jun 8, 2022

This patchset implements support for the Atmel SAM V70(b) series devices.

Just the basics, soc/ and DTS bindings.

Requires zephyrproject-rtos/hal_atmel#23

Signed-off-by: Perry Hung [email protected]

@zephyrbot
Copy link

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_atmel zephyrproject-rtos/hal_atmel@78c5567 zephyrproject-rtos/hal_atmel#23 zephyrproject-rtos/hal_atmel#23/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added the platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) label Jun 8, 2022
@nandojve
Copy link
Member

nandojve commented Jun 8, 2022

Hi @iperry ,

To move with this we need a board to build at CI at least. As far I remember, there is no official board available from MCHP that uses SAMV70. Once SAMV70 it is a subset from SAMV71 (less Ethernet) I`ll suggest to add entries at boards/arm/sam_v71_xult for that purpose. In this case, there will be have 4 targets inside sam_v71_xult board directory.

@iperry
Copy link
Contributor Author

iperry commented Jun 8, 2022

Hi @iperry ,

To move with this we need a board to build at CI at least. As far I remember, there is no official board available from MCHP that uses SAMV70. Once SAMV70 it is a subset from SAMV71 (less Ethernet) I`ll suggest to add entries at boards/arm/sam_v71_xult for that purpose. In this case, there will be have 4 targets inside sam_v71_xult board directory.

Hello @nandojve , thank you for the review. I don't fully understand what you would like me to do. Can you clarify what you mean?

Should I copy the board overlay for the sam_v71_xult and make a new, fake one for an imaginary board? In particular, I am not sure about:

  1. What to name it
  2. Where to put the Kconfig options
  3. Do I make a new board name, defconfig, dts, pin-ctrl, and yaml file for a imaginary board?

@nandojve
Copy link
Member

nandojve commented Jun 9, 2022

Hi @iperry ,

Should I copy the board overlay for the sam_v71_xult and make a new, fake one for an imaginary board? In particular, I am not sure about:

  1. What to name it
  2. Where to put the Kconfig options
  3. Do I make a new board name, defconfig, dts, pin-ctrl, and yaml file for a imaginary board?
  1. Zephyr identify boards by content inside a board directory, not necessarily the directory name itself. When you run west boards you already see two boards for sam_v71_xult: sam_v71_xult and sam_v71b_xult. Now you need add two other boards: sam_v70_xult and sam_v70b_xult.

2-3. There is no necessity to create a new board directory. You need just create sam_v70[b]xult.dts, sam_v70[b]xult.yaml and sam_v7[b]xult_defconfig files to make a new board available to Zephyr. You need edit Kconfig.foo, openocd.cfg and index.rst files. In the case of documentation you should mention the support of SAMV70 and clarify that only network is not available in this specific Zephyr board. You should rename sam_v71_xult-pinctrl.dtsi as sam_v7x_xult-pinctrl-common.dtsi to be included by two new files sam_v71_xult-pinctrl.dtsi and sam_v70_xult-pinctrl.dtsi. We need to do this to make sure we are including correct dt-bindings pinctrl file. You should do a similar process with sam_v71_xult-common.dtsi and move gmac node to a new sam_v71_xult-common.dtsi file. I think it may not be necessary create a sam_v70_xult-common.dtsi file.

Note: It is possible to use Kconfig variables to instruct OpenOCD, see below as reference:

if(CONFIG_X86 OR CONFIG_ARC)
set_ifndef(OPENOCD_USE_LOAD_IMAGE YES)
endif()

iperry added 3 commits June 10, 2022 02:35
Import upstream ASF support pack for the SAM V70(b) series devices.

Signed-off-by: Perry Hung <[email protected]>
Add DTS bindings for the SAM V70(b) device family.

Signed-off-by: Perry Hung <[email protected]>
Implement soc files for the samv70(b) family devices.

Signed-off-by: Perry Hung <[email protected]>
@iperry
Copy link
Contributor Author

iperry commented Jun 10, 2022

Hi @nandojve , thanks for the clarification.

I've prepared two test targets that I believe sort of implements what you are going for. I introduced two boards, ran twister tests, and fixed any Kconfig errors that I could find.

Introduce two new variations (sam_v70_xult and sam_v70b_xult) of the SAM
V71 Xplained Ultra development board. While these are not actually
manufactured variations, they exist to test the SAM V70 support. These
boards are configured with an ATSAMV70Q20(b) processor and are
functionally identical to the SAM V71 versions, minus Ethernet.

Signed-off-by: Perry Hung <[email protected]>
@nandojve nandojve changed the title soc: dts: implement SAM V70 support soc: arm: atmel: implement SAM V70 support Jun 11, 2022
@nandojve nandojve added this to the v3.2.0 milestone Jun 11, 2022
Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iperry,

Almost there. See my comments.

Comment on lines +11 to +22

if CAN_SAM

config CAN_MAX_FILTER
int "Maximum number of concurrent active filters"
default 5
range 1 32
help
Defines the array size of the callback/msgq pointers.
Must be at least the size of concurrent reads.

endif # CAN_SAM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a separated commit or could even be on a different PR.
Could you clarify?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Kconfig is not needed by the Bosch M_CAN driver backend. Please remove it.

Comment on lines 12 to +13
depends on SOC_SERIES_SAME70 || \
SOC_SERIES_SAMV71
SOC_SERIES_SAMV71 || SOC_SERIES_SAMV70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep pattern of one per line were V70 came before V71

# are used).
#
config NUM_IRQS
default 74 if SOC_ATMEL_SAMV70_REVB
Copy link
Member

@nandojve nandojve Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

71 instead 74, check typedef enum IRQn

#
config NUM_IRQS
default 74 if SOC_ATMEL_SAMV70_REVB
default 71
Copy link
Member

@nandojve nandojve Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

69 instead 71, check typedef enum IRQn

Comment on lines +47 to +53

if BOARD_SAM_V70_XULT

config BOARD
default "sam_v70_xult"

endif # BOARD_SAM_V70_XULT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try keep less specialized always first.

Comment on lines +10 to +13

config BOARD_SAM_V70_XULT
bool "Atmel SMART SAM V70 Xplained Ultra Board"
depends on SOC_PART_NUMBER_SAMV70Q20 || SOC_PART_NUMBER_SAMV70Q20B
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try keep less specialized always first.

Comment on lines +3 to +4
SAM V70/V71 (B) Xplained Ultra
##############################
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V70/V71(B)


#include <atmel/same70.dtsi>

#include "dma_atmel_samv71.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should exclude gmac node here.


#include <atmel/same70b.dtsi>

#include "dma_atmel_samv71.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should exclude gmac node here.

Comment on lines +11 to +22

if CAN_SAM

config CAN_MAX_FILTER
int "Maximum number of concurrent active filters"
default 5
range 1 32
help
Defines the array size of the callback/msgq pointers.
Must be at least the size of concurrent reads.

endif # CAN_SAM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Kconfig is not needed by the Bosch M_CAN driver backend. Please remove it.

@nandojve
Copy link
Member

nandojve commented Aug 1, 2022

Hi @iperry ,

I was wondering if you have time to address our concerns. It will be nice have it for 3.2

@nandojve nandojve modified the milestones: v3.2.0, future Aug 28, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Oct 28, 2022
@github-actions github-actions bot closed this Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants