Add CAN driver support for Renesas RZ/G3S #83778

nhutnguyenkc · 2025-01-10T07:59:24Z

Add CAN driver support for Renesas RZ/G3S.

Since RZ/G3S does not support to calculate perfectly the sample point of 87,5%, so the test update is required.

zephyrbot · 2025-01-10T08:00:25Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff

✅ All manifest checks OK

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

henrikbrixandersen · 2025-01-10T11:23:06Z

tests/drivers/can/api/src/canfd.c

@@ -527,7 +527,11 @@ ZTEST_USER(canfd, test_set_timing_data_while_started)
 	struct can_timing timing = { 0 };
 	int err;

+#ifdef CONFIG_CAN_RENESAS_RZ_CANFD


No SoC/driver/board specific #ifdefs in generic API tests, please. You need to find another solution.

Understood. I have reverted the change. Unfortunately, RZ/G3S cannot calculate the sample point of 87.5% accurately, so the test_set_timing_data_while_started fail. Is it acceptable if we change the err check at line 531 to range check as below (in fact, 25 is enough for us)

zassert_between_inclusive(err, 0, 50, "failed to calculate data timing (err %d)", err);

@henrikbrixandersen , could you please confirm my above solution. Thanks

str4t0m

@nhutnguyenkc I am not very familiar with the renesas ecosystem, however this driver seems to use exactly the same fsp API as the existing renesas_ra driver.
In a quick glance at the hal the only difference in the header I can see are the not-/availabilty of canfd_txmb_merge_mode, e_canfd_tx_buffer, and common fifo.
And with the exception of some initialization stuff the code seems to match that driver nearly exactly.
Therefore I would suggest to keep the different compatibles, but factor out the common stuff of the two drivers.
Ideally the clock control driver would also be merged before this driver, then they would likely match even more.

str4t0m · 2025-02-09T18:35:55Z

boards/renesas/rzg3s_smarc/doc/index.rst

+| CANFD     | on-chip    | can                                 |
+-----------+------------+-------------------------------------+


This should be part of the board commit. Same for the change in the rzg3s_smarc_r9a08g045s33gbg_cm33.yaml

Thanks. I have moved them to board commit.

str4t0m · 2025-02-09T20:05:22Z

drivers/can/can_renesas_rz_canfd.c

+static int can_renesas_rz_get_core_clock(const struct device *dev, uint32_t *rate)
+{
+	const struct can_renesas_rz_cfg *cfg = dev->config;


Clock control driver not yet implemented, would make commonality between with the ra series even bigger.

Thanks. I have implemented clock control driver for can. It took a while to get clock control merged.

Add CAN driver support for Renesas RZ/G3S Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

Add CAN nodes to Renesas RZ/G3S devicetree Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

henrikbrixandersen

Did you consider the refactoring proposed in #83778 (review) ?

henrikbrixandersen · 2025-02-17T22:02:39Z

tests/drivers/can/api/src/canfd.c

@@ -528,7 +528,7 @@ ZTEST_USER(canfd, test_set_timing_data_while_started)
 	int err;

 	err = can_calc_timing_data(can_dev, &timing, TEST_BITRATE_3, TEST_SAMPLE_POINT);
-	zassert_ok(err, "failed to calculate data timing (err %d)", err);
+	zassert_between_inclusive(err, 0, 50, "failed to calculate data timing (err %d)", err);


This needs to be split out into it's own PR with proper description instead of being somewhat "hidden" in this larger PR.

Understood. I have reverted this change, and will update it in a separate PR.

henrikbrixandersen · 2025-02-17T22:03:19Z

tests/drivers/can/api/boards/rzg3s_smarc_r9a08g045s33gbg_cm33.overlay

How come CAN is not enabled on the board level?

Thanks. I moved it to the board commit and dropped the test commit.

henrikbrixandersen · 2025-02-17T22:03:26Z

tests/drivers/can/timing/boards/rzg3s_smarc_r9a08g045s33gbg_cm33.overlay

How come CAN is not enabled on the board level?

Thanks. I moved it to the board commit and dropped the test commit.

Add CAN support for board RZ/G3S-SMARC Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

nhutnguyenkc · 2025-02-18T04:06:09Z

Thanks for your feedback. For the coexistence of CAN driver of renesas_rz and renesas_ra, let me explain a bit. They are two different product lines (RA is MCU, RZ is MPU) having two different FSP code base (hal_renesas). You may see small differences in case of RZ/G3S, but when other RZ devices come, the difference will be bigger, and the code would look too messy if it is shared between RZ and RA. In addition to the points that you have listed out (canfd_txmb_merge_mode, e_canfd_tx_buffer, and common fifo), clock, hardware configuration, direct register accesses are the points keeping them separate. These were also considered internally before concluding implementing a different code base.

zephyrbot added area: CAN platforms: Renesas RZ labels Jan 10, 2025

zephyrbot requested review from alexanderwachter, henrikbrixandersen, martinjaeger, str4t0m and tgorochowik January 10, 2025 08:00

zephyrbot assigned henrikbrixandersen Jan 10, 2025

zephyrbot added manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Jan 10, 2025

henrikbrixandersen requested changes Jan 10, 2025

View reviewed changes

nhutnguyenkc force-pushed the rzg3s_support_can branch from b37f36e to ac70044 Compare January 14, 2025 02:25

nhutnguyenkc force-pushed the rzg3s_support_can branch from ac70044 to f8912e1 Compare February 3, 2025 09:07

zephyrbot removed manifest manifest-hal_renesas DNM This PR should not be merged (Do Not Merge) labels Feb 3, 2025

nhutnguyenkc force-pushed the rzg3s_support_can branch from f8912e1 to 2774cef Compare February 6, 2025 07:09

str4t0m requested changes Feb 11, 2025

View reviewed changes

nhutnguyenkc force-pushed the rzg3s_support_can branch from 2774cef to d12e6e0 Compare February 17, 2025 08:19

zephyrbot requested a review from binhnguyen2434 February 17, 2025 08:19

Hieu Nguyen added 2 commits February 17, 2025 16:40

drivers: can: Initial support for RZ/G3S

263bbdc

Add CAN driver support for Renesas RZ/G3S Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

dts: renesas: Add CAN support for RZ/G3S

1cce763

Add CAN nodes to Renesas RZ/G3S devicetree Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

nhutnguyenkc force-pushed the rzg3s_support_can branch 2 times, most recently from 98d9143 to c93a24d Compare February 17, 2025 10:39

henrikbrixandersen requested changes Feb 17, 2025

View reviewed changes

boards: renesas: rzg3s_smarc: Add CAN support

35f2406

Add CAN support for board RZ/G3S-SMARC Signed-off-by: Hieu Nguyen <[email protected]> Signed-off-by: Nhut Nguyen <[email protected]>

nhutnguyenkc force-pushed the rzg3s_support_can branch from c93a24d to 35f2406 Compare February 18, 2025 03:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CAN driver support for Renesas RZ/G3S #83778

Add CAN driver support for Renesas RZ/G3S #83778

nhutnguyenkc commented Jan 10, 2025 •

edited

Loading

zephyrbot commented Jan 10, 2025 •

edited

Loading

henrikbrixandersen Jan 10, 2025

nhutnguyenkc Jan 14, 2025 •

edited

Loading

nhutnguyenkc Jan 21, 2025

str4t0m left a comment

str4t0m Feb 9, 2025

nhutnguyenkc Feb 17, 2025

str4t0m Feb 9, 2025

nhutnguyenkc Feb 17, 2025

henrikbrixandersen left a comment

henrikbrixandersen Feb 17, 2025

nhutnguyenkc Feb 18, 2025

henrikbrixandersen Feb 17, 2025

nhutnguyenkc Feb 18, 2025

henrikbrixandersen Feb 17, 2025

nhutnguyenkc Feb 18, 2025

nhutnguyenkc commented Feb 18, 2025 •

edited

Loading

		\| CANFD \| on-chip \| can \|
		+-----------+------------+-------------------------------------+

Add CAN driver support for Renesas RZ/G3S #83778

Are you sure you want to change the base?

Add CAN driver support for Renesas RZ/G3S #83778

Conversation

nhutnguyenkc commented Jan 10, 2025 • edited Loading

zephyrbot commented Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

nhutnguyenkc Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

str4t0m 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

henrikbrixandersen 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhutnguyenkc commented Feb 18, 2025 • edited Loading

nhutnguyenkc commented Jan 10, 2025 •

edited

Loading

zephyrbot commented Jan 10, 2025 •

edited

Loading

nhutnguyenkc Jan 14, 2025 •

edited

Loading

nhutnguyenkc commented Feb 18, 2025 •

edited

Loading