drivers: cop: add NXP COP driver

[alxrey]

Port NXP cop driver to Zephyr.

[alxrey]

The implementation as pushed here is functional, but IMO not ideal.
To use the watchdog, the following configuration is currently required:

CONFIG_WATCHDOG=y  
CONFIG_WDOG_ENABLE_AT_BOOT=y 

to prevent the watchdog from being disabled by SystemInit() (DISABLE_WDOG set to 0 via zephyr_compile_definitions_ifdef(CONFIG_WDOG_ENABLE_AT_BOOT DISABLE_WDOG=0)).

This is absolutely necessary since the COP watchdog control register is write-once after reset. In reality, the macro name ENABLE_AT_BOOT is therefore misleading, it should be more like "DO_NOT_DISABLE_AT_BOOT," since the actual configuration is done later by the application using the watchdog API.

A cleaner solution could be to initialize the watchdog directly in SystemInit(), and only implement mcux_cop_feed() in the driver. However, this would require accessing the configuration in the devicetree from soc.c, which I'm not sure is recommended...

[dleach02]

This doesn't need any actual code?

[decsny]

I don't understand why there is a giant comment saying not to single step through an empty function

[dleach02]

Let's put in the initial configuration. Note that this soc.c is a reference for folks who make custom boards and if they need this they will configure it the way the need to.

If we deem it useful, we can have some additional Kconfig for custom configuring the parameters.

[decsny]

@michal-smola @NeilChen93 as this is on MCXC, can you please help to review this PR

[michal-smola]

As @alxrey suggests, it would be probably better to use different Kconfig symbol to disable the watchdog. Symbol WDT_DISABLE_AT_BOOT which is defined in watchdog driver could be used. The symbol is also used in the driver test.
If WDT_DISABLE_AT_BOOT is used instead of WDOG_ENABLE_AT_BOOT, the empty z_arm_watchdog_init function definition could be avoided.

[alxrey]

The thing is, since the watchdog register is write-once and can’t be disabled and re-enabled later, I’m not too comfortable leaving it up to the application to configure it early enough before it triggers a reset. I think the cleanest solution would be to configure the watchdog entirely during system init, without requiring the user to call anything manually.
The problem then is how to let the user define the watchdog settings in that case.

[dleach02]

I'll suggest that we don't have too much concern over counting on the application to configure the wdog early enough. Remember that they will be making their custom board and application so using z_arm_watchdog_init() at boot time is there to basically handle this situation.

[michal-smola]

@alxrey , configuration of wdog could be done in soc.c using soc_late_init_hook function. When the function is called, devices are already initialized and wdog driver can be used for configuration. I believe it is what you want to achieve. I tried to implement it and it works, but I am not sure if it is good practice. @dleach02 @decsny , could you comment, please?

Second point is, that I was not able to disable wdog using the driver. The reason is that there is a check in mcux_cop_disable function if wdog is enabled, but it can be enabled only by calling mcux_cop_setup function which makes the first write to COPC register. After the first write the second write to disable wdog is ignored. @alxrey , could you try reproduce and fix it, please?

[alxrey]

@alxrey , could you try reproduce and fix it, please?

Yes, you were right, thanks. I fixed it.

[alxrey]

@alxrey , configuration of wdog could be done in soc.c using soc_late_init_hook function. [...]

Sounds like a good idea, + use of WDT_DISABLE_AT_BOOT symbol rather than WDOG_ENABLE_AT_BOOT.
I wait for the review of @dleach02 and @decsny, and if they agree, I will implement that.

[sonarqubecloud]

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

[dleach02]

soc_late_init_hook

I think you can use either the soc_late_init_hook, soc_early_init_hook, or z_arm_watchdog_init hook. It just depends on how early or late it is needed to ensure it doesn't fire improperly.

[butok]

Should it return an error here?