Minimal support for RA8M1 #17

thaoluonguw · 2024-04-30T11:28:05Z

Add minimal support for RA8M1. Implementation refers from Flexible Software Package (FSP) v5.3.0

This is the initial hal support for RA MCU family series. The hal layer support for RA will base on RA FSP. Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com> Signed-off-by: The Nguyen <the.nguyen.yf@renesas.com> Signed-off-by: Quy Tran <quy.tran.pz@renesas.com> Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>

IRQ priority on Zephyr have offset, disable the priority setting for IRQ in FSP HAL layer so that priority setting will not be override when Open() API is called Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

Update RA FSP to version 5.3.0 Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

KhiemNguyenT

I reviewed the patches. It's OK to me.

soburi · 2024-05-02T22:13:18Z

ra/fsp/ra/fsp/inc/instances/r_dtc.h

@@ -0,0 +1,100 @@
+/*


I think it would be better to include all drivers licensed as BSD.

@soburi : I am sorry, but I haven't understood your comment... we add license as BSD for all RA drivers in this repository.

I think you have included only the files that will be needed in the most recent commit.
Rather than adding them to hal each time according to the main source of zephyr, I think it is a good idea to include all driver sources that may be supported in the future at this point.
(And it looks like that possible file is released as BSD.)

@soburi : (Cc @KhiemNguyenT) Adding all driver sources at once can cause adding un-used codes which are not our planning, we will support phase to phase for easy control and maintain.

As far as I know, that method is not a common management method for zephyr hals, so I cannot recommend it.
From my experience, it is better to have a management policy that does not require updating the hal too frequently.

@ydamigos we shall work on incremental update, as long as there's no violation to common policy. Some implementation takes time to get agreement but it does not block on-going activity, let's keep it separated.

@KhiemNguyenT
Regarding HAL is usually updated in bulk according to the update of the source code from which it is imported.
I expect that will be easier to maintain in the future even if it contains unused files. We will just copy/paste every new FSP release.
Regarding the "extra" HAL for covering the missing Zephyr API
The incremental updates make sense.

@ydamigos HAL updating in bulk will be applied in future. For now, we would like to focus on essential HAL drivers and address the comments on the code in-use.

@KhiemNguyenT That's fine for me.

@KhiemNguyenT
This is not blocking so let's go ahead with this.

soburi · 2024-05-02T22:17:43Z

ra/fsp/ra/fsp/src/bsp/mcu/ra8m1/bsp_feature.h

@@ -0,0 +1,448 @@
+/*


I think it would be better to include all files licensed as BSD here as well.
(I think it would be desirable to only update when a new version of fsp is released.)

@soburi : Did you mean we create only one file and describe information about license (BSD) for all?

No, it's not.

I think it is good to add other SoC files for future support, not only RA8M1.
The fsp includes files other than BSD, so I wrote it with a meaning of other than that.
(I want to use it for migrating RA4M1.)

Zephyr convention requires an SPDX identifier for all files.
So, All files probably have license notices.

@soburi : (Cc @KhiemNguyenT): We have plan to support RA4M1 in 2nd half of this year. So, drivers for RA4M1 will come soon. We are looking forward to your contribution at that time too.

Zephyr convention requires an SPDX identifier for all files.
So, All files probably have license notices.

Thanks for your reminding me. I nearly forgot.

soburi · 2024-05-02T22:24:23Z

ra/ra_cfg/fsp_cfg/r_dtc_cfg.h

@@ -0,0 +1,40 @@
+/*


I think these are based on files generated by FSP tools.
How about creating a module/hal_renesas_fsp directory on the zephyr side and placing it there?
(Also, I think it would be easier to manage if only fsp releases were included)

@soburi Thanks for your feedback.
I agree with you but a second module looks like an overkill to me. I would prefer if we followed the following structure:

. ├── fsp │ ├── ra │ └── any other family ├── smartbond └── zephyr ├── blobs ├── fsp_cfg │ ├── ra │ └── any other family └── any other folder/file that will help us use FSP with Zephyr

@thaoluonguw @KhiemNguyenT @ioannis-karachalios Your thoughts?

I also agree that a second module would be an overkill.

@thaoluonguw @ioannis-karachalios

I got it. Let's go with the method you suggested.

Update include path for mcu from CONFIG_SOC to CONFIG_SOC_SERIES because Zephyr repository changing the RA8 MCU grouping Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

Move cfg files from zephyr OS to hal_renesas Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>

ydamigos · 2024-05-27T06:45:47Z

ra/ra_cfg/fsp_cfg/bsp/ra8m1/bsp_mcu_device_pn_cfg.h

+#define BSP_MCU_FEATURE_SET       ('A')
+#define BSP_ROM_SIZE_BYTES        (2064384)
+#define BSP_RAM_SIZE_BYTES        (917504)
+#define BSP_DATA_FLASH_SIZE_BYTES (12288)


We could pull the values from DT.

@ydamigos : These values are dummy now. This file is necessary building with fsp only. We would like to keep it in this PR.

@thaoluonguw We could keep the file but please remove unused macros. We use only BSP_PACKAGE_PINS. Let's just leave this one only.

Now, We can use substring function in Kconfig.
Some definitions can be calculate from P/N.

ydamigos · 2024-05-27T07:27:19Z

CMakeLists.txt

@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: Apache-2.0

 add_subdirectory_ifdef(CONFIG_SOC_FAMILY_RENESAS_SMARTBOND smartbond)
+add_subdirectory_ifdef(CONFIG_HAS_RENESAS_RA_FSP ra)


Add an empty line at the end.

soburi · 2024-05-28T11:17:39Z

ra/fsp/ra/fsp/src/bsp/mcu/ra8m1/bsp_feature_gen.h

+/***********************************************************************************************************************
+ * Private global variables and functions
+ **********************************************************************************************************************/
+// *UNCRUSTIFY-ON*


If you delete it, please delete it in a balanced manner.

In the original code, this line was paired with

// *UNCRUSTIFY-OFF*

There are the following differences when compared with fsp, but is it necessary to delete them?

@@ -26,15 +26,5 @@ /*********************************************************************************************************************** * Private global variables and functions **********************************************************************************************************************/ - -// *UNCRUSTIFY-OFF* -#define BSP_FEATURE_GPT_AD_DIRECT_START_CHANNEL_MASK (0xFF) -#define BSP_FEATURE_GPT_AD_DIRECT_START_SUPPORTED (0x1) -#define BSP_FEATURE_GPT_GPTE_CHANNEL_MASK (0) -#define BSP_FEATURE_GPT_GPTE_SUPPORTED (0) -#define BSP_FEATURE_GPT_GPTEH_CHANNEL_MASK (0) -#define BSP_FEATURE_GPT_GPTEH_SUPPORTED (0) -#define BSP_FEATURE_GPT_OPS_CHANNEL_MASK (0x1) -#define BSP_FEATURE_GPT_OPS_SUPPORTED (0x1)

soburi · 2024-05-28T12:10:52Z

ra/ra_cfg/fsp_cfg/bsp/ra8m1/bsp_mcu_device_pn_cfg.h

+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef BSP_MCU_DEVICE_PN_CFG_H_


It's an incomplete guard condition.

soburi · 2024-05-28T12:17:26Z

ra/ra_cfg/fsp_cfg/bsp/ra8m1/bsp_cfg.h

+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef ZEPHYR_SOC_RENESAS_RA_COMMON_BSP_CFG_H_


Please use the same macro name policy for guard conditions.
It doesn't necessarily have to be the zephyr rules here, but
If you want to follow Zephyr's policy, please do the same for other files.

soburi · 2024-05-29T13:40:30Z

ra/CMakeLists.txt

+zephyr_include_directories(${include_dirs})
+zephyr_library_sources(${srcs})
+
+message(${CONFIG_SOC_SERIES})


I think this log is not very meaningful. It is better to delete it.

soburi · 2024-05-29T22:31:41Z

ra/ra_cfg/fsp_cfg/bsp/ra8m1/bsp_cfg.h

+#ifndef BSP_CLOCK_CFG_MAIN_OSC_POPULATED
+#define BSP_CLOCK_CFG_MAIN_OSC_POPULATED (1)
+#endif
+#ifndef BSP_CLOCK_CFG_MAIN_OSC_CLOCK_SOURCE
+#define BSP_CLOCK_CFG_MAIN_OSC_CLOCK_SOURCE (0)
+#endif
+#ifndef BSP_CLOCK_CFG_SUBCLOCK_DRIVE
+#define BSP_CLOCK_CFG_SUBCLOCK_DRIVE (0)
+#endif
+#ifndef BSP_CLOCK_CFG_SUBCLOCK_POPULATED
+#define BSP_CLOCK_CFG_SUBCLOCK_POPULATED (1)
+#endif
+#ifndef BSP_CLOCK_CFG_SUBCLOCK_STABILIZATION_MS
+#define BSP_CLOCK_CFG_SUBCLOCK_STABILIZATION_MS 1000
+#endif


These definitions are board depended.
must refer to the devicetree configuration.

duynguyenxa added 3 commits April 26, 2024 09:29

hal: renesas: ra: Disable irq priority configuration

d5f633b

IRQ priority on Zephyr have offset, disable the priority setting for IRQ in FSP HAL layer so that priority setting will not be override when Open() API is called Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

hal: renesas: ra: Update FSP version to 5.3.0

518ffac

Update RA FSP to version 5.3.0 Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

thaoluonguw mentioned this pull request Apr 30, 2024

Add minimal support for EK-RA8M1 (CM85) zephyrproject-rtos/zephyr#72150

Open

KhiemNguyenT approved these changes May 2, 2024

View reviewed changes

soburi reviewed May 2, 2024

View reviewed changes

duynguyenxa and others added 2 commits May 14, 2024 09:54

hal: ra: Update CMakeList for RA MCU grouping

0467acc

Update include path for mcu from CONFIG_SOC to CONFIG_SOC_SERIES because Zephyr repository changing the RA8 MCU grouping Signed-off-by: Duy Nguyen <duy.nguyen.xa@renesas.com>

hal: renesas: ra: Add bsp configuration files for ra8m1

6dd36ea

Move cfg files from zephyr OS to hal_renesas Signed-off-by: Thao Luong <thao.luong.uw@renesas.com>

ydamigos reviewed May 27, 2024

View reviewed changes

soburi reviewed May 28, 2024

View reviewed changes

soburi reviewed May 29, 2024

View reviewed changes

soburi suggested changes May 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minimal support for RA8M1 #17

Minimal support for RA8M1 #17

thaoluonguw commented Apr 30, 2024

KhiemNguyenT left a comment

soburi May 2, 2024

thaoluonguw May 27, 2024

soburi May 27, 2024

thaoluonguw May 27, 2024

soburi May 27, 2024 •

edited

KhiemNguyenT May 28, 2024

ydamigos May 29, 2024

KhiemNguyenT May 29, 2024

ydamigos May 29, 2024

soburi May 29, 2024

soburi May 2, 2024

thaoluonguw May 27, 2024

soburi May 27, 2024

thaoluonguw May 27, 2024

soburi May 2, 2024

ydamigos May 20, 2024

ioannis-karachalios May 20, 2024 •

edited

soburi May 20, 2024

ydamigos May 27, 2024

thaoluonguw May 28, 2024

ydamigos May 28, 2024

soburi May 29, 2024

ydamigos May 27, 2024

soburi May 28, 2024 •

edited

soburi May 28, 2024

soburi May 28, 2024

soburi May 29, 2024

soburi May 29, 2024

Minimal support for RA8M1 #17

Are you sure you want to change the base?

Minimal support for RA8M1 #17

Conversation

thaoluonguw commented Apr 30, 2024

KhiemNguyenT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi May 27, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannis-karachalios May 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi May 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi May 27, 2024 •

edited

ioannis-karachalios May 20, 2024 •

edited

soburi May 28, 2024 •

edited