Skip to content

Conversation

@soburi
Copy link
Member

@soburi soburi commented Aug 7, 2024

Change to use FSP to integrate with other Renesas RA series.

@soburi soburi added the DNM This PR should not be merged (Do Not Merge) label Aug 7, 2024
@zephyrbot
Copy link

zephyrbot commented Aug 7, 2024

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

Name Old Revision New Revision Diff

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

Copy link

@KhiemNguyenT KhiemNguyenT left a comment

Choose a reason for hiding this comment

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

@soburi , I think you need to brush up the PR. There are some changes for RA2A1 are mixed into.

@soburi soburi force-pushed the ra4m1_migrate_to_fsp branch from 78cb002 to 6da34c3 Compare August 21, 2024 15:49
@KhiemNguyenT KhiemNguyenT self-assigned this Aug 21, 2024
@soburi soburi force-pushed the ra4m1_migrate_to_fsp branch 4 times, most recently from 0708bdb to b58b9d0 Compare September 1, 2024 08:26
@soburi
Copy link
Member Author

soburi commented Sep 2, 2024

@KhiemNguyenT

@soburi , I think you need to brush up the PR. There are some changes for RA2A1 are mixed into.

Update done.
I will remove DNM and proceed once the following PR in hal_renesas is approved.

zephyrproject-rtos/hal_renesas#31

@soburi soburi marked this pull request as ready for review September 2, 2024 05:43
@zephyrbot zephyrbot added area: GPIO area: Pinctrl platform: Renesas RA Renesas Electronics Corporation, RA area: Clock Control area: UART Universal Asynchronous Receiver-Transmitter labels Sep 2, 2024
@soburi soburi requested a review from KhiemNguyenT September 4, 2024 12:08
Comment on lines 66 to 67
Copy link
Member

Choose a reason for hiding this comment

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

wrong:

  • source: define another clock and make it source using eg clocks = <&main_osc>;
  • divider should be given as integer div = <2>;.

Copy link
Member Author

@soburi soburi Sep 6, 2024

Choose a reason for hiding this comment

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

@duynguyenxa @quytranpzz
Are these properties actually used?

Copy link
Member

Choose a reason for hiding this comment

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

@soburi , these properties are used to provide information for the bsp_clock_cfg.h in the hal layer:

#define BSP_CFG_PLL_SOURCE                                                                         \
	BSP_CLOCK_PROP_HAS_STATUS_OKAY_OR(DT_NODELABEL(pll), source, RA_PLL_SOURCE_DISABLE)
#define BSP_CFG_PLL_DIV BSP_CLOCK_PROP_HAS_STATUS_OKAY_OR(DT_NODELABEL(pll), div, RA_PLL_DIV_2)
#if DT_NODE_HAS_STATUS(DT_NODELABEL(pll), okay)

The BSP_CFG_PLL_SOURCE, and BSP_CFG_PLL_DIV BSP will be used in the bsp_clock_init() to configure PLL clock.

Copy link
Member

Choose a reason for hiding this comment

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

FYI: We can't have HALisms in DT.

Copy link
Member

Choose a reason for hiding this comment

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

@gmarull , the macro is defined in zephyr main repo, zephyr/include/zephyr/dt-bindings/clock/ra_clock.h

Copy link
Member

Choose a reason for hiding this comment

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

it still looks wrong, why isn't div = <2>? Are such values encoding what HAL expects?

Copy link
Member

Choose a reason for hiding this comment

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

@gmarull , sorry for late response, it's expect the value map with HWM here for the pll.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

@duynguyenxa

The root of this problem is that the DeviceTree is a hardware description language,
and how values ​​are handled should not be mixed in here.
In other words, the DeviceTree should be able to describe semantic values; for example, if the division factor is 2, then it should be describable as "2", and it should be up to the implementation side, i.e., the .h or .c file, to convert this to bit notation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I splitted out this issue to #78365.

Copy link
Member Author

Choose a reason for hiding this comment

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

#78365 is already merged.

Copy link
Member

Choose a reason for hiding this comment

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

no underscores in properties please.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have created #78097.
I will fix it after this PR is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

#78097 is already merged.

duynguyenxa
duynguyenxa previously approved these changes Oct 29, 2024
@soburi
Copy link
Member Author

soburi commented Oct 31, 2024

@KhiemNguyenT
Please approve if it is good for you.

tejlmand
tejlmand previously approved these changes Nov 1, 2024
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm, a minot nit observed but not blocking.

Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit, not blocking.

No need for linewrap anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for your feedback.
I fixed it.

Change to use FSP to integrate with other Renesas RA series.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Since the Option Setting Memory area is set in FSP, the Kconfig value
switches between using the FSP implementation or the existing
Option Setting Memory implementation.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Switch the pinctrl driver to renesas,ra-pinctrl-pfs which can be
used with FSP.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Switch the clock controller driver to renesas,ra-cgc-pclkblock
which can be used with FSP.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Update configuration for migrate to FSP

Signed-off-by: TOKITA Hiroshi <[email protected]>
Update configuration for migrate to FSP

Signed-off-by: TOKITA Hiroshi <[email protected]>
This PR fixes zephyrproject-rtos#78619 for the Arduino UNO R4 Minima/Wifi board.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Remove the renesas,ra-pinctrl driver, which is no longer
needed after migrating to the FSP-based implementation.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Remove the renesas,ra-clock-generation-circuit driver, which is no longer
needed after migrating to the FSP-based implementation.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi soburi modified the milestones: v4.0.0, v4.1.0 Nov 4, 2024
@soburi
Copy link
Member Author

soburi commented Nov 16, 2024

@KhiemNguyenT
Please take a look.

Copy link

@KhiemNguyenT KhiemNguyenT left a comment

Choose a reason for hiding this comment

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

Thank you for this action.
LGTM.

@fabiobaltieri fabiobaltieri merged commit 43db55a into zephyrproject-rtos:main Nov 20, 2024
26 checks passed
@soburi soburi deleted the ra4m1_migrate_to_fsp branch November 20, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants