Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hal: ti: HwiP_zephyr.c blocks generic AUX interrupt (plus several others) #57713

Closed
ghost opened this issue May 9, 2023 · 8 comments
Closed
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug platform: TI Texas Instruments priority: low Low impact/importance bug Stale

Comments

@ghost
Copy link

ghost commented May 9, 2023

Describe the bug

Zephyr's platform adaptation layer for TI's CC13xx/CC26xx MCU assigns an exclusive interrupt handler for the combined "AUX_COMB" interrupt. It seems that this excludes custom drivers from most of the MCU's low-power peripherals (low power comparators, timers, time-to-digital converter, etc.).

I'm not very deep into the CC13xx/CC26xx adaptation layer yet so maybe I'm just overlooking something? But if it turns out to be a real problem I'll be able to provide a patch once we settle on a solution.

See the following code snippet
/hal_ti/blob/4e0e053f5ea413ef0cb20000729038d921ac2746/simplelink/kernel/zephyr/dpl/HwiP_zephyr.c#L223

	case INT_AUX_COMB:
		sl_AUX_COMB_cb.cb = hwiFxn;
		sl_AUX_COMB_cb.arg = arg;
		obj->cb = &sl_AUX_COMB_cb;
		irq_connect_dynamic(INT_AUX_COMB - 16, priority, sl_isr, &sl_AUX_COMB_cb, 0);
		break;

The affected peripherals can be seen here:

dl.ti.com/simplelink/esd/simplelink_cc13xx_cc26xx_sdk/6.41.00.17/exports/docs/driverlib_cc13xx_cc26xx/cc13x2_cc26x2/register_descriptions/CPU_MMAP/AUX_EVCTL.html#EVTOMCUFLAGS)

To Reproduce

Assigning a custom interrupt to the AUX_COMB IRQ will break zephyr's PM adaptation layer for CC13xx/CC26xx.

See https://github.com/zephyrproject-rtos/hal_ti/blob/b6ca1ace9024afa072c0ff8294d04b8d9fe4aede/simplelink/source/ti/drivers/power/PowerCC26X2.c#L343

Expected behavior

It should be possible to wrap or extend the platform adaption layer's ISR. As other IRQ numbers are also being shadowed by the same method it might be a good idea to find a solution that can be extended to those IRQ numbers if needed.

Impact

Most low power peripherals cannot be used as intended (except for the ADC which has its own IRQ number). Actively polling the corresponding event flags instead does not seem like a good option as the MCU should be able to sleep while low-power peripherals are doing their thing.

Logs and console output

n/a

Environment (please complete the following information):

current Zephyr mainline

Any ideas how this could be fixed?

@ghost ghost added the bug The issue is a bug, or the PR is fixing a bug label May 9, 2023
@ghost
Copy link
Author

ghost commented May 9, 2023

See #25216 for a similar problem / some additional context.

@ghost
Copy link
Author

ghost commented May 9, 2023

Some ideas for discussion:

  • HwiP_construct/destruct/setFunc() might check whether an interrupt has already been assigned before and (un)wrap it. HwiP_setPriority() may be changed to only keep the highest requested priority for all ISRs.
  • It seems that Power_init() is only called once during the initialization of the PM. The AUX_COMB interrupt is used there to calibrate RC oscillators. It could release the interrupt handler once calibration is done. I've no idea how this could be synchronized with other drivers' access to the same IRQ, though.
  • The shared-irq driver could somehow be used internally.
  • Implement some custom interrupt controller.
  • We could reserve the programmable interrupt (IRQ30) or one of the AON programmable interrupts (AON_PROGx) for AUX events.

@ghost
Copy link
Author

ghost commented May 10, 2023

@vaishnavachath maybe?

@cfriedt cfriedt added priority: low Low impact/importance bug platform: TI Texas Instruments labels May 10, 2023
@vaishnavachath
Copy link
Collaborator

@fgrandel Thanks for reporting the issue, I think the concerns you mentioned are genuine and needs to be fixed, I will try to reproduce this issue locally and come up with some inputs for the discussion over the weekend, as of now I do not have a clear idea on what should be right fix.

The following approach you mentioned can be used to workaround the situation for AUX events if I understand correctly?

  • We could reserve the programmable interrupt (IRQ30) or one of the AON programmable interrupts (AON_PROGx) for AUX events.

@ghost
Copy link
Author

ghost commented May 10, 2023

The following approach you mentioned can be used to workaround the situation for AUX events if I understand correctly?

  • We could reserve the programmable interrupt (IRQ30) or one of the AON programmable interrupts (AON_PROGx) for AUX events.

@vaishnavachath Yes, absolutely. This is how we're trying to work around the issue for now.

@ghost
Copy link
Author

ghost commented May 16, 2023

@vaishnavachath I managed to build a working PoC on our side that works around the issue by routing TDC_DONE event via AON event fabric to AON_PROG0 and from there via MCU event subscriber to Interrupt 29 which is connected to that event. Works perfectly.

The question remains, though, how interrupts could be shared as there are many more event lines than available interrupt lines on the CC13/26xx platform - so being able to demux those interrupts somehow would be great if more drivers will be added. But no hurry - we have a working solution on our side and no need to rush.

@ghost
Copy link
Author

ghost commented May 23, 2023

See #49379 for a possible upcoming fix.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Jul 23, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug platform: TI Texas Instruments priority: low Low impact/importance bug Stale
Projects
None yet
Development

No branches or pull requests

2 participants