Skip to content

Conversation

@soburi
Copy link
Member

@soburi soburi commented Jan 9, 2022

This PR adds watchdog drivers for GD32.
GD32 has two types of the watchdog timer, such as

  • FWDGT free watchdog timer
  • WWDGT Window watchdog timer

This PR adds support for both types of watchdogs.

I verified with Longan Nano Board with GD32VF103.

More GD32 work #38657

Copy link
Member

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Thanks @soburi for this contribution. Good to see the GD32 ecosystem support grow.

Got some rather small change requests and some questions below.

@SebastianBoe SebastianBoe removed their request for review January 11, 2022 08:42
@nandojve nandojve mentioned this pull request Jan 12, 2022
55 tasks
@nandojve
Copy link
Member

Hi @soburi ,

Rebase and squash update commits on the originals.

@soburi
Copy link
Member Author

soburi commented Jan 14, 2022

Hi @soburi ,

Rebase and squash update commits on the originals.

Hi @nandojve
I was apply review comments with [Commit Suggestion] button.
I'm now working to correct this PR.
I will soon re-push.

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 @soburi ,

A few comments.

Copy link
Member

Choose a reason for hiding this comment

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

I would like know what is the USE_GD32_DBG Kconfig used for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Ok, Is this required in production? or is this only required when debugging board?

Copy link
Member Author

Choose a reason for hiding this comment

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

Change to set the register directly not use the vendor provided function, and removing USE_GD32_DBG.

Copy link
Member

Choose a reason for hiding this comment

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

Just a remind that the USE_GD32_DBG still there.
Note, you can keep it by adding another Kconfig variable to enable it by demand with default as disabled.

Copy link
Member

Choose a reason for hiding this comment

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

Try keep short lines always possible, for instance:

uint32_t ticks = (uint64_t)CONFIG_GD32_LOW_SPEED_IRC_FREQUENCY
               * timeout / MSEC_PER_SEC;

Comment on lines 124 to 145
Copy link
Member

Choose a reason for hiding this comment

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

	ARG_UNUSED(channel_id);

	fwdgt_counter_reload();

	return 0;

Comment on lines 151 to 172
Copy link
Member

Choose a reason for hiding this comment

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

#endif

	return ret;

Comment on lines 101 to 122
Copy link
Member

Choose a reason for hiding this comment

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

	wwdgt_enable();
	wwdgt_flag_clear();
	wwdgt_interrupt_enable();

	return 0;

Comment on lines 130 to 157
Copy link
Member

Choose a reason for hiding this comment

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

	wwdgt_config(counter, window, prescaler);

	return 0;

Comment on lines 138 to 168
Copy link
Member

Choose a reason for hiding this comment

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

	ARG_UNUSED(channel_id);

	wwdgt_counter_update(data->counter);

	return 0;

Comment on lines 148 to 180
Copy link
Member

Choose a reason for hiding this comment

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

		wwdgt_flag_clear();

		if (data->callback) {
			data->callback(dev, 0);
		}

Comment on lines 151 to 154
Copy link
Member

Choose a reason for hiding this comment

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

You are doing more than simple add overlay files. I think you should explain better at commit message why you are changing the example or split the changes.

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 got it. Splitting this commit.

@soburi soburi force-pushed the gd32_watchdog branch 2 times, most recently from f61ac4a to 3f6b5d8 Compare September 17, 2022 03:49
@soburi soburi requested review from gmarull and nandojve September 17, 2022 10:20
@gmarull gmarull added this to the v3.3.0 milestone Sep 19, 2022
@fkokosinski
Copy link
Member

Hi @soburi, you seem to have introduced undefined references to log_output_flush in your code and some minor style issues, which is why the CI now fails.

Add options about Internal RC(IRC) oscillator.

- GD32_HAS_IRC_32K/40K indicates IRC types.
- GD32_LOW_SPEED_IRC_FREQUENCY is the numeric value of frequency

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add GD32_DBG_SUPPORT options to enable HAL debug functions.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add support for GD32 Free watchdog timer.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi soburi force-pushed the gd32_watchdog branch 3 times, most recently from 9fffdb5 to a648877 Compare September 23, 2022 11:05
@soburi
Copy link
Member Author

soburi commented Sep 24, 2022

Hi, @fkokosinski

Hi @soburi, you seem to have introduced undefined references to log_output_flush in your code and some minor style issues, which is why the CI now fails.

Rebase and fix CI failure done.

fkokosinski
fkokosinski previously approved these changes Sep 26, 2022
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

one last nitpick, thanks for this work

Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

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

nit: resets required true missing

Copy link
Member Author

Choose a reason for hiding this comment

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

It is significant issue!
Fixed it.

soburi added 5 commits October 2, 2022 19:56
Add support for GD32 Window watchdog timer.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add watchdog node for GD32 series SoC's dts files.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Enable watchdog in each GD32 implemented board.
Add the watchdog function as a supported function in each yaml files.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Add support for boards that implement GD32 SoC.

Overlay file for longan_nano_lite enables FWDGT.

Add WDT_FEED_INTERVAL to make adjustable
the execution cycle of wdt_feed().

And also, make waiting for the WDT_FEED_INTERVAL period
for the watchdog window opens at the start of the test.

Signed-off-by: TOKITA Hiroshi <[email protected]>
Enable watchdog tests for boards that implement GD32 SoC.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@gmarull gmarull requested a review from fkokosinski October 3, 2022 06:37
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.

Thank you @soburi !

@carlescufi carlescufi merged commit 6bbd2b2 into zephyrproject-rtos:main Oct 3, 2022
@soburi soburi deleted the gd32_watchdog branch October 3, 2022 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards area: Build System area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Modules area: RISCV RISCV Architecture (32-bit & 64-bit) area: Samples Samples area: Tests Issues related to a particular existing or missing test area: Watchdog Watchdog manifest-hal_gigadevice platform: GD32 GigaDevice

Projects

None yet

Development

Successfully merging this pull request may close these issues.