drivers: rtc: microchip: Add rtc driver #99144

Farsin-Nasar-Microchip · 2025-11-10T08:06:19Z

Add rtc driver for Microchip RTC G1.
Add the device tree node and the binding file for microchip RTC G1.
Add RTC node in sam_e54_xpro.dts
Update sam_e54_xpro.yaml to reflect RTC G1 support on the board.
Added sam_e54_xpro.conf file for test

bjarki-andreasen

Please add this board to the rtc test suite and run it.

bjarki-andreasen · 2025-11-10T14:40:29Z

dts/bindings/rtc/microchip,rtc-g1.yaml

+title: Microchip G1 RTC driver
+


Belongs in the previous commit

bjarki-andreasen · 2025-11-10T14:41:48Z

drivers/rtc/rtc_mchp_g1.c

+#define RTC_DATA_LOCK_INIT(p_lock)  k_mutex_init(p_lock)
+#define RTC_DATA_LOCK(p_lock)       k_mutex_lock(p_lock, K_FOREVER)
+#define RTC_DATA_UNLOCK(p_lock)     k_mutex_unlock(p_lock)


These should not be defines, its just as short and clearer to just write the actual line

Also, use semaphores for locking, mutexes are only needed if tracking thread ownership is needed, like for condvar

bjarki-andreasen · 2025-11-10T14:43:00Z

drivers/rtc/rtc_mchp_g1.c

+#endif /* CONFIG_RTC_ALARM */
+
+/* Clock configuration structure for RTC. */
+struct mchp_rtc_clock {


rtc_mchp or?

Thank you @bjarki-andreasen for your review. Do you mean the structure name should be changed to rtc_mchp_clock or something similar?

bjarki-andreasen · 2025-11-10T14:45:29Z

drivers/rtc/rtc_mchp_g1.c

+		(dataClockCalendar & RTC_MODE2_CLOCK_DAY_Msk) >> RTC_MODE2_CLOCK_DAY_Pos;
+}
+
+static bool rtc_validate_rtc_clock_time(const struct rtc_time *rtc_clock_time)


use rtc_utils_validate_rtc_time()

bjarki-andreasen · 2025-11-10T14:48:23Z

drivers/rtc/rtc_mchp_g1.c

+	}
+
+	/* Lock interrupts to ensure setting of callback */
+	unsigned int key = irq_lock();


The zephyr way of doing a critical section is with a spinlock, though since this lock looks like it is only preventing concurrent access from threads, use a mutex/sem for locking?

I have updated the code to use a semaphore

bjarki-andreasen · 2025-11-10T14:51:03Z

drivers/rtc/rtc_mchp_g1.c

+	/* Perform a software reset on the RTC peripheral */
+	rtc_swrst(cfg->regs);


Will this action reset the time of the rtc? RTCs are supposed to survive a reboot, and any other event which does not physically take away power if possible.

Thank you for the information. I have updated the code and removed the rtc_swrst

bjarki-andreasen · 2025-11-10T14:52:01Z

drivers/rtc/rtc_mchp_g1.c

+	if (ret == -EALREADY) {
+		ret = 0;
+	}


this should be moved up to right after ret is set, makes it easier to see why ret == -EALREADY is ok

nandojve · 2025-11-10T20:16:00Z

drivers/rtc/rtc_mchp_g1.c

+#ifdef CONFIG_RTC_ALARM
+#define RTC_MCHP_IRQ_HANDLER(n)                                                                    \
+	static void rtc_mchp_irq_config_##n(const struct device *dev)                              \
+	{                                                                                          \
+		RTC_MCHP_IRQ_CONNECT(n, 0);                                                        \
+	}
+#endif /* CONFIG_RTC_ALARM */
+
+/* RTC driver configuration structure for instance n */
+/* clang-format off */
+#define RTC_MCHP_CONFIG_DEFN(n)        \
+	static const struct rtc_mchp_dev_config rtc_mchp_dev_config_##n = {		\
+		.regs = (rtc_registers_t *)DT_INST_REG_ADDR(n),		\
+		.prescaler = DT_INST_ENUM_IDX(n, prescaler),		\
+	IF_ENABLED(CONFIG_RTC_ALARM, (		\
+		.alarms_count = DT_INST_PROP(n, alarms_count),	\
+		.irq_config_func = &rtc_mchp_irq_config_##n,))		\
+	IF_ENABLED(CONFIG_RTC_CALIBRATION, (		\
+		.cal_constant = DT_INST_PROP(n, cal_constant),))		\
+	RTC_MCHP_CLOCK_DEFN(n)     \
+	}
+/* clang-format on */
+/* Define and initialize the RTC device. */
+#define RTC_MCHP_DEVICE_INIT(n)                                                                    \
+	IF_ENABLED(CONFIG_RTC_ALARM, (						\
+	static void rtc_mchp_irq_config_##n(const struct device *dev);		\
+))                                                     \
+	RTC_MCHP_CONFIG_DEFN(n);                                                                   \


backslash are not align in the right in some places.

Farsin-Nasar-Microchip · 2025-11-13T07:07:21Z

Please add this board to the rtc test suite and run it.
Hi @bjarki-andreasen, Should I create a separate PR to add this board to the RTC test suite?, I have already run it locally.

bjarki-andreasen · 2025-11-13T10:45:25Z

Please add this board to the rtc test suite and run it.

Hi @bjarki-andreasen, Should I create a separate PR to add this board to the RTC test suite?, I have already run it locally.

Please add it in this PR

boards/microchip/sam/sam_e54_xpro/sam_e54_xpro.dts

dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x.dtsi

dts/bindings/rtc/microchip,rtc-g1.yaml

drivers/rtc/rtc_mchp_g1.c

nandojve · 2025-11-20T14:24:19Z

boards/microchip/sam/sam_e54_xpro/sam_e54_xpro.dts

 };

+&rtc {
+	compatible = "microchip,rtc-g1";


Why are you defining again the compatible inside the board ?
Usually in the board, the user should define the values that are necessary. The compatible should be in the dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x.dtsi.

I have removed the redundant compatible string from the board DTS.

nandojve · 2025-11-20T14:24:59Z

boards/microchip/sam/sam_e54_xpro/sam_e54_xpro.dts

+	compatible = "microchip,rtc-g1";
+	prescaler = <1024>;
+	alarms-count = <2>;
+	cal-constant = <1048576>;  /* (8192 * 128 = 1048576) */


Same with this constant. User should not know about this if it never changes. This means that is should be defined at dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x.dtsi.

Updated accordingly

Add the device tree node and the binding file for microchip RTC G1 IP. Signed-off-by: farsin NASAR V A <[email protected]>

Add rtc driver for Microchip RTC G1. Signed-off-by: Farsin Nasar V A <[email protected]>

Add RTC node in sam_e54_xpro.dts Update sam_e54_xpro.yaml to reflect RTC G1 support on the board. Signed-off-by: farsin NASAR V A <[email protected]>

Added sam_e54_xpro.conf file Signed-off-by: farsin NASAR V A <[email protected]>

sonarqubecloud · 2025-11-21T09:43:19Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Farsin-Nasar-Microchip · 2025-11-21T12:19:15Z

The error comes from the CI script. Other recent PRs also show the same issue. eg : #99799

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch 2 times, most recently from ed10fea to 5ebd07e Compare November 10, 2025 09:13

Farsin-Nasar-Microchip marked this pull request as ready for review November 10, 2025 09:46

zephyrbot added area: RTC Real Time Clock platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: Boards/SoCs area: Devicetree Bindings platform: Microchip MEC Microchip MEC Platform labels Nov 10, 2025

zephyrbot requested review from ArunMCHP, VenkatKotakonda, albertofloyd, bjarki-andreasen, jvasanth1, nandojve, pdgendt and stephanosio November 10, 2025 09:48

zephyrbot assigned bjarki-andreasen Nov 10, 2025

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from 5ebd07e to 5ec84ac Compare November 10, 2025 10:47

bjarki-andreasen requested changes Nov 10, 2025

View reviewed changes

albertofloyd removed their request for review November 10, 2025 18:33

nandojve added this to the v4.4.0 milestone Nov 10, 2025

nandojve requested changes Nov 10, 2025

View reviewed changes

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from 5ec84ac to c5a914e Compare November 17, 2025 06:17

zephyrbot added the area: Tests Issues related to a particular existing or missing test label Nov 17, 2025

zephyrbot requested a review from nashif November 17, 2025 06:18

Farsin-Nasar-Microchip requested review from bjarki-andreasen and nandojve November 17, 2025 06:59

NhMchp previously approved these changes Nov 17, 2025

View reviewed changes

ArunMCHP requested changes Nov 17, 2025

View reviewed changes

boards/microchip/sam/sam_e54_xpro/sam_e54_xpro.dts Outdated Show resolved Hide resolved

dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x.dtsi Show resolved Hide resolved

dts/bindings/rtc/microchip,rtc-g1.yaml Show resolved Hide resolved

Farsin-Nasar-Microchip dismissed NhMchp’s stale review via 6c90730 November 18, 2025 11:17

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from c5a914e to 6c90730 Compare November 18, 2025 11:17

Farsin-Nasar-Microchip requested review from ArunMCHP and NhMchp November 18, 2025 11:22

NhMchp requested changes Nov 19, 2025

View reviewed changes

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from 6c90730 to 3bf448c Compare November 19, 2025 12:39

Farsin-Nasar-Microchip requested a review from NhMchp November 20, 2025 04:30

ArunMCHP previously approved these changes Nov 20, 2025

View reviewed changes

NhMchp previously approved these changes Nov 20, 2025

View reviewed changes

nandojve requested changes Nov 20, 2025

View reviewed changes

Farsin-Nasar-Microchip added 3 commits November 21, 2025 14:18

dts: arm: microchip: add RTC node and binding for G1 IP

cd38535

Add the device tree node and the binding file for microchip RTC G1 IP. Signed-off-by: farsin NASAR V A <[email protected]>

drivers: rtc: microchip: Add rtc driver

c19103f

Add rtc driver for Microchip RTC G1. Signed-off-by: Farsin Nasar V A <[email protected]>

boards: microchip: sam: Add RTC node and enable RTC G1 support

921d422

Add RTC node in sam_e54_xpro.dts Update sam_e54_xpro.yaml to reflect RTC G1 support on the board. Signed-off-by: farsin NASAR V A <[email protected]>

Farsin-Nasar-Microchip dismissed stale reviews from NhMchp and ArunMCHP via 5bbb6c8 November 21, 2025 08:53

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from 3bf448c to 5bbb6c8 Compare November 21, 2025 08:53

tests: drivers: rtc: Add support for the sam_e54_xpro board

9f35618

Added sam_e54_xpro.conf file Signed-off-by: farsin NASAR V A <[email protected]>

Farsin-Nasar-Microchip force-pushed the g1_rtc_driver_upstream branch from 5bbb6c8 to 9f35618 Compare November 21, 2025 09:15

Farsin-Nasar-Microchip requested review from ArunMCHP, NhMchp and nandojve November 21, 2025 12:19

		/* Perform a software reset on the RTC peripheral */
		rtc_swrst(cfg->regs);

drivers: rtc: microchip: Add rtc driver #99144

Are you sure you want to change the base?

drivers: rtc: microchip: Add rtc driver #99144

Conversation

Farsin-Nasar-Microchip commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjarki-andreasen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Farsin-Nasar-Microchip commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjarki-andreasen commented Nov 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 21, 2025

Quality Gate passed

Uh oh!

Farsin-Nasar-Microchip commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Farsin-Nasar-Microchip commented Nov 10, 2025 •

edited

Loading

Farsin-Nasar-Microchip commented Nov 13, 2025 •

edited

Loading