soc: nxp: imx95: Refine condition to enable MCUX_LPTMR_TIMER

[dbaluta]

For i.MX95 M7 MCUX_LPTMR_TIMER defaults to y if COUNTER_MCUX_LPTMR not enabled.

This prevents us on enabling CORTEX_M_SYSTICK. So refine the condition so that MCUX_LPTMR_TIMER only defaults to enabled if COUNTER_MCUX_LPTMR not set (as it is now) and also add condition that zephyr,system-timer is actually set.

This also moves DT_CHOSEN_ZEPHYR_SYSTEM_TIMER, DT_CHOSEN_ZEPHYR_SYSTEM_TIMER_PATH definitions at the top of the file.

Fixes: 0c4382a ("drivers: timer: keep mcux_lptmr policy explicit")

This also fixes SOF build for i.MX95 thesofproject/sof#10765

[zejiang0jason]

suggest using configdefault

[dbaluta]

@zejiang0jason this an enhancement. I prefer to fix the issue first.

[LaurentiuM1234]

this looks wrong. why are we using the same compatible for two different programming models? what if someone enables CONFIG_COUNTER to use LPIT for example? looks like we'll end up with two different drivers operating on the same IP.

[dbaluta]

@LaurentiuM1234 not sure I understand your comment. The code in this patch adds a stricter condition on enabling MCUX_LPTMR_TIMER.

So, if someone enables CONFIG_COUNTER then MCUX_LPTMR_TIMER won't be selected by default.

[LaurentiuM1234]

@LaurentiuM1234 not sure I understand your comment. The code in this patch adds a stricter condition on enabling MCUX_LPTMR_TIMER.

what I'm trying to point out here is that the way these 2 drivers are handled doesn't seem correct due to the fact that we're trying to use the same compatible for two different programming models. What I'd like to see here is the core problem being fixed. Adding more Kconfig stitching is not it, IMO.

So, if someone enables CONFIG_COUNTER then MCUX_LPTMR_TIMER won't be selected by default.

this only seems to work if you set CONFIG_COUNTER=y in the defconfig file. If you set it manually via menuconfig then you'll end up with both drivers enabled.

[dbaluta]

@LaurentiuM1234 the entire design looks fishy but at least it looks like both drivers were designed to acknowledge each other.

So for example, here is a comment from drivers/counter/counter_mcux_lptmr.c:

 * Skip the instance reserved as the system timer via zephyr,system-timer.
 * When both drivers are enabled, they must operate on separate hardware.
 */
#define COUNTER_MCUX_LPTMR_IS_SYSTEM_TIMER(n)»  »       »       »       \
»       COND_CODE_1(DT_HAS_CHOSEN(zephyr_system_timer),»»       »       \
»       »       (DT_SAME_NODE(DT_INST(n, nxp_lptmr),»   »       »       \
»       »       »             DT_CHOSEN(zephyr_system_timer))),»»       \
»       »       (0))

@Holt-Sun can you please comment on this?

@LaurentiuM1234 my fix is indeed not fixing the root cause of the issue but I also don't think it is worse either. It just partially fixes 0c4382a so that we can enable CORTEX_M_SYSTICK.

I wonder if it is not a better idea to just revert 0c4382a

[Holt-Sun]

@LaurentiuM1234 the entire design looks fishy but at least it looks like both drivers were designed to acknowledge each other.

So for example, here is a comment from drivers/counter/counter_mcux_lptmr.c:

 * Skip the instance reserved as the system timer via zephyr,system-timer.
 * When both drivers are enabled, they must operate on separate hardware.
 */
#define COUNTER_MCUX_LPTMR_IS_SYSTEM_TIMER(n)»  »       »       »       \
»       COND_CODE_1(DT_HAS_CHOSEN(zephyr_system_timer),»»       »       \
»       »       (DT_SAME_NODE(DT_INST(n, nxp_lptmr),»   »       »       \
»       »       »             DT_CHOSEN(zephyr_system_timer))),»»       \
»       »       (0))

@Holt-Sun can you please comment on this?

@LaurentiuM1234 my fix is indeed not fixing the root cause of the issue but I also don't think it is worse either. It just partially fixes 0c4382a so that we can enable CORTEX_M_SYSTICK.

I wonder if it is not a better idea to just revert 0c4382a

@LaurentiuM1234 and @dbaluta , for LPTMR specifically, I don't think using the same nxp,lptmr compatible for both Zephyr APIs is the problem here.

The separation is done at the instance-selection level. MCUX_LPTMR_TIMER is for the singleton system timer selected by /chosen/zephyr,system-timer, and counter_mcux_lptmr.c explicitly skips that same instance:

/ *
  * Skip the instance reserved as the system timer via zephyr,system-timer.
  * When both drivers are enabled, they must operate on separate hardware.
  */

So even if CONFIG_COUNTER=y and both Kconfig symbols are enabled, the counter driver should not instantiate a device for the LPTMR node reserved as the system timer. If there are additional LPTMR instances, using them through the counter API is intentional.

This PR only narrows the i.MX95 M7 default so MCUX_LPTMR_TIMER is selected by default only when an LPTMR is actually selected as the system timer. It does not change the existing timer/counter instance separation.

[LaurentiuM1234]

@dbaluta please do a rebase so we can re-run the CI - issue is unrelated to this.

[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